From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id DAFAF1FF0E5 for ; Wed, 29 Jul 2026 14:08:35 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id ADB25213E5; Wed, 29 Jul 2026 14:08:35 +0200 (CEST) Message-ID: Date: Wed, 29 Jul 2026 14:08:05 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH proxmox 1/3] system-report: add crate for shared report generation To: Erik Fastermann , pbs-devel@lists.proxmox.com, pdm-devel@lists.proxmox.com References: <20260714083156.99794-1-e.fastermann@proxmox.com> <20260714083156.99794-2-e.fastermann@proxmox.com> <1219b0fa-c7c8-4749-9069-9cb440338e91@proxmox.com> <39404168-b9e6-4321-9250-7cd0e6bc6362@proxmox.com> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: <39404168-b9e6-4321-9250-7cd0e6bc6362@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1785326878774 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.141 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: VA3GU2KFPYPW4VYLLWHLBCNHDMNC34CM X-Message-ID-Hash: VA3GU2KFPYPW4VYLLWHLBCNHDMNC34CM X-MailFrom: c.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 7/29/26 1:55 PM, Erik Fastermann wrote: > Thank you for the review. Some notes/questions below. I will send the v2 > ASAP. > > [snip] > >>> diff --git a/proxmox-system-report/debian/copyright b/proxmox-system- >>> report/debian/copyright >>> new file mode 100644 >>> index 00000000..77952eba >>> --- /dev/null >>> +++ b/proxmox-system-report/debian/copyright >>> @@ -0,0 +1,18 @@ >>> +Format: https://www.debian.org/doc/packaging-manuals/copyright- >>> format/1.0/ >>> + >>> +Files: >>> + * >>> +Copyright: 2019 - 2026 Proxmox Server Solutions GmbH >>> >> >> question: not sure about the copyright timespan here, but since the >> factored out code lived in proxmox-backup and that has 2019-2026 this >> is fine I guess? > > I checked and for the recently added `proxmox-disks` only 'Copyright: > 2026' is used and that crate was also factored out from PBS, so I will > do the same. > > [snip] > >>> +    vec![ >>> +        ("proxmox-boot-tool", vec!["status"]), >>> +        ("df", vec!["-h", "-T"]), >>> +        ( >>> +            "lsblk", >>> +            vec![ >>> +                "--ascii", >>> +                "-M", >>> +                "-o", >>> +                "+HOTPLUG,ROTA,PHY-SEC,FSTYPE,MODEL,TRAN", >>> +            ], >>> +        ), >>> +        ("bash", vec!["-c", "ls -l /dev/disk/by-*/"]), >>> +        ("zpool", vec!["status"]), >>> +        ("zfs", vec!["list"]), >>> +        ("zarcstat", vec![]), >>> +        ("dmidecode", vec!["-t", "bios"]), >>> +        ("lscpu", vec![]), >>> +        ("lspci", vec!["-nnk"]), >>> +        ("ip", vec!["-details", "-statistics", "a"]), >>> +        ("ip", vec!["-4", "route", "show"]), >>> +        ("ip", vec!["-6", "route", "show"]), >> >> comment: these commands depend on external executables. So there >> should be a dependency in debian/control as well. > > E.g. `proxmox-disks` calls `zpool` and `sgdisk`, which are also not in > the `Depends` section. In fact all crates I checked in this repository > only have other Rust related `Depends`. It looks like either PBS or PDM > depend on them directly or they are pulled in by the installer. Right, since this is a lib only and does not produce any executable by itself, it is not correct to pull these in. Disregard my comment with respect to that. > The `Depends` section of PDM or PBS might need some work in general. > E.g. `iproute2` is missing from PDM `Depends`, but is given for PBS. I > saw there was a patch which added some to PBS [0], but that was not > applied to PDM. It would probably be good to check what dependencies > should be added. Should this be part of this series? What do you think? Yes, adding a patch to assure such dependencies is especially important if PBS or PDM are installed on top of a vanilla Debian and not via the installer.