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 B5FD56FDD9 for ; Wed, 1 Sep 2021 16:48:57 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 96BE49D6B for ; Wed, 1 Sep 2021 16:48:57 +0200 (CEST) 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 93EEE9D2E for ; Wed, 1 Sep 2021 16:48:53 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 67324444B1; Wed, 1 Sep 2021 16:48:53 +0200 (CEST) Message-ID: Date: Wed, 1 Sep 2021 16:48:52 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:92.0) Gecko/20100101 Thunderbird/92.0 Content-Language: en-US To: Proxmox Backup Server development discussion , Hannes Laimer References: <20210830111505.38694-1-h.laimer@proxmox.com> <20210830111505.38694-2-h.laimer@proxmox.com> From: Dominik Csapak In-Reply-To: <20210830111505.38694-2-h.laimer@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.921 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.932 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 v2 proxmox-backup 01/15] tools: add disks utility functions 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: Wed, 01 Sep 2021 14:48:57 -0000 comments inline On 8/30/21 13:14, Hannes Laimer wrote: > adds: > - get_fs_uuid_by_disk_path: taken out of the already existing > get_fs_uuid function > - get_mount_point_by_uuid: returns the mount-point of a disk by a given > uuid, fails if the disk is not mounted > - get_fs_uuid_by_path: finds the uuid of the disk that the given path > is on, can fail if there is no uuid for the disk > - mount_by_uuid: mounts a disk identified by uuid to a given path > - unmount_by_mount_point: basically linux 'umount -l', justification > for the lazy option in code > --- > src/tools/disks/mod.rs | 53 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 53 insertions(+) > > diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs > index af7f7fe1..ec4d8742 100644 > --- a/src/tools/disks/mod.rs > +++ b/src/tools/disks/mod.rs > @@ -1009,6 +1009,22 @@ pub fn get_fs_uuid(disk: &Disk) -> Result { > None => bail!("disk {:?} has no node in /dev", disk.syspath()), > }; > > + get_fs_uuid_by_disk_path(&disk_path) > +} > + > +/// Get the FS UUID of the disk that the given path is on. > +pub fn get_fs_uuid_by_path(path: &Path) -> Result { > + let mut command = std::process::Command::new("df"); > + command.args(&["--output=source"]); > + command.arg(path); > + > + let output = crate::tools::run_command(command, None)?; AFAICS, we already have that information in DiskManage::find_mounted_device ? would it not make sense to add the uuid information there? this would save us a fork > + > + get_fs_uuid_by_disk_path(Path::new(output.lines().skip(1).next().unwrap())) although it seems unlikely to make a problem (for now), please do not use unwrap in the middle of code. when df sometime decides to change output (or some other unpredictable thing happens) this will panic, and depending where that code is executed, it can have bad effects on the rest of the code (e.g. when holding a mutex) instead use e.g. '.ok_or_else' or use something like if let Some(foo) = output....next() { ... } else { bail!(...) } > +} > + > +/// Get the FS UUID of a disk with a given disk path. > +pub fn get_fs_uuid_by_disk_path(disk_path: &Path) -> Result { > let mut command = std::process::Command::new("blkid"); > command.args(&["-o", "export"]); > command.arg(disk_path); > @@ -1023,3 +1039,40 @@ pub fn get_fs_uuid(disk: &Disk) -> Result { > > bail!("get_fs_uuid failed - missing UUID"); > } > + > +/// Get mount point by UUID > +pub fn get_mount_point_by_uuid(uuid: &str) -> Result { > + let mut command = std::process::Command::new("findmnt"); > + command.args(&["-rn", "-oTARGET", "-S"]); > + command.arg(&format!("UUID={}", uuid)); > + > + let output = crate::tools::run_command(command, None)?; > + > + if !output.is_empty() { > + return Ok(String::from(output.trim())); > + } > + > + bail!("get_mount_point_by_uuid failed - device with uuid: {} is not mounted", uuid); > +} > + > +/// Mount a disk by its UUID and the mount point. > +pub fn mount_by_uuid(uuid: &str, mount_point: &Path) -> Result<(), Error> { > + let mut command = std::process::Command::new("mount"); > + command.args(&[&format!("UUID={}", uuid)]); > + command.arg(mount_point); > + > + crate::tools::run_command(command, None)?; > + Ok(()) > +} > + > +/// Unmount a disk by its mount point. > +pub fn unmount_by_mount_point(mount_point: &Path) -> Result<(), Error> { > + let mut command = std::process::Command::new("umount"); > + // Datastores are only unmounted after a check for running jobs, '-l' needed due to some weird > + // bug where some tokio-threads keep the .lock-file open (sometimes) > + command.args(&["-l"]); > + command.arg(mount_point); > + > + crate::tools::run_command(command, None)?; > + Ok(()) > +} >