From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <d.csapak@proxmox.com>
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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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: <ea59e379-7e89-a5d1-a70e-563dcac2f786@proxmox.com>
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
<pbs-devel@lists.proxmox.com>, Hannes Laimer <h.laimer@proxmox.com>
References: <20210830111505.38694-1-h.laimer@proxmox.com>
<20210830111505.38694-2-h.laimer@proxmox.com>
From: Dominik Csapak <d.csapak@proxmox.com>
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
<pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=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<String, Error> {
> 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<String, Error> {
> + 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<String, Error> {
> 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<String, Error> {
>
> bail!("get_fs_uuid failed - missing UUID");
> }
> +
> +/// Get mount point by UUID
> +pub fn get_mount_point_by_uuid(uuid: &str) -> Result<String, Error> {
> + 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(())
> +}
>