From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 7E1EB84DCA for ; Thu, 16 Dec 2021 09:16:14 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5C4A41B7CA for ; Thu, 16 Dec 2021 09:15:44 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id B27C21B7BB for ; Thu, 16 Dec 2021 09:15:43 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 6CE7745270; Thu, 16 Dec 2021 09:15:43 +0100 (CET) Message-ID: Date: Thu, 16 Dec 2021 09:15:42 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:96.0) Gecko/20100101 Thunderbird/96.0 Content-Language: en-US To: Thomas Lamprecht , Proxmox Backup Server development discussion References: <20211215091952.1490583-1-d.csapak@proxmox.com> <20211215091952.1490583-3-d.csapak@proxmox.com> From: Dominik Csapak In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.190 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.034 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [mod.rs] Subject: Re: [pbs-devel] [PATCH proxmox v2 2/3] proxmox-sys: add DiskUsage struct and helper X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Dec 2021 08:16:14 -0000 On 12/15/21 13:41, Thomas Lamprecht wrote: > On 15.12.21 10:19, Dominik Csapak wrote: >> copied from proxmox-backup >> > > It's copied I know, but maybe we can use the need of touching this to improve it > slightly, so please read my comments inline as I would make then on this being a > clean slate addition. > sure, makes sense >> Signed-off-by: Dominik Csapak >> --- >> proxmox-sys/src/fs/mod.rs | 26 ++++++++++++++++++++++++++ >> 1 file changed, 26 insertions(+) >> >> diff --git a/proxmox-sys/src/fs/mod.rs b/proxmox-sys/src/fs/mod.rs >> index 9fe333b..5935b1a 100644 >> --- a/proxmox-sys/src/fs/mod.rs >> +++ b/proxmox-sys/src/fs/mod.rs >> @@ -3,6 +3,7 @@ use std::fs::File; >> use std::path::Path; >> >> use anyhow::{bail, Error}; >> +use serde::Serialize; >> >> use std::os::unix::io::{AsRawFd, RawFd}; >> use nix::unistd::{Gid, Uid}; >> @@ -102,3 +103,28 @@ impl CreateOptions { >> */ >> } >> >> +/// Basic disk usage information > > would add the source, e.g.: ... derived from what statfs64 reports > >> +#[derive(Serialize)] >> +pub struct DiskUsage { > > Is DiskUsage really a good name here? As with "Disk" I thing block level, but > this is filesystem level.. Maybe FileSystemUsage (shorteting to FSUsage could > be done, but that's not helping that much.. how about FileSystemInformation (or FSInfo if the length is a problem)? IMHO "usage" is wrong if we include things like the type or id > > can we get short doc comments here about what unit this is, IMO some slight > redundancy here never hurt anyone or made it harder to understand. > > /// total bytes available in a mounted filesystem > >> + pub total: u64, >> + pub used: u64, > > Available is derived from f_bavail, which is the available to the unprivileged user, > maybe note that here, as with ext4 default that means that you get about 5% less > here than `total - used` which may be good to know for users, that would then also > explain why we even have `available` as else, `total` and `used` would be enough > info. > > general notes, in general a file system usage could be improved with inode total and > free count and mayb the FSID for mapping such a struct back to the physical FS? yeah sounds good, i'd also include the fs type, too, could be useful sometimes. > >> + pub available: u64, >> +} >> + >> +/// Get disk usage information from path >> +pub fn disk_usage(path: &std::path::Path) -> Result { >> + let mut stat: libc::statfs64 = unsafe { std::mem::zeroed() }; >> + >> + use nix::NixPath; >> + >> + let res = path.with_nix_path(|cstr| unsafe { libc::statfs64(cstr.as_ptr(), &mut stat) })?; >> + nix::errno::Errno::result(res)?; >> + >> + let bsize = stat.f_bsize as u64; >> + > > > nit but can you rust format new entries, it's a bit a PITA as the test-range operating > feature is only available in nightly IIRC, but proxmox-sys should be relatively clean so > doing it on the whole file shouldn't that big of a nuisance. yeah sorry, i still do not do that automatically .... > > >> + Ok(DiskUsage{ > > missing space before { > >> + total: stat.f_blocks*bsize, > > space between operands of a math evaluation > > >> + used: (stat.f_blocks-stat.f_bfree)*bsize, > > same here > >> + available: stat.f_bavail*bsize, > > same here >> + }) >> +} >