public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH storage] disk manage: pass full NVMe device path to smartctl
Date: Mon, 12 Dec 2022 13:33:09 +0100	[thread overview]
Message-ID: <20221212123309.109693-1-f.ebner@proxmox.com> (raw)

This essentially reverts commit c9bd3d2 ("fix #1123: modify NVME
device path for SMART support").

The man page for smartctl states
> Use the forms "/dev/nvme[0-9]" (broadcast namespace) or
> "/dev/nvme[0-9]n[1-9]" (specific  namespace 1-9) for NVMe devices.
so it should be fine to pass the path with the specific namespace to
smartctl.

But that text was already present in the man page of version 6.5,
which is the version the commit c9bd3d2 talks about. It might be that
it was necessary to drop the specific namespace for the version
backported from Stretch to Jessie (the bug report mentions that that
version was used[0]), but it's not quite clear.

With current versions, passing in the path with the specific namespace
did work as expected[1], even on a device with multiple namespaces set
up tested locally. In PBS, the path queried via
udev::Device::from_syspath("/sys/block/{name}") is passed to smartctl
and that also included the specific namespace on the systems I tested
with a short script.

So pass the full path to make things a little bit simpler and to avoid
potential future issues like bug #2020[2].

[0]: https://bugzilla.proxmox.com/show_bug.cgi?id=1123#c3
[1]: https://forum.proxmox.com/threads/113962/post-493185
[2]: https://bugzilla.proxmox.com/show_bug.cgi?id=2020

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/Diskmanage.pm                                         | 2 --
 test/disk_tests/nvme_smart/{nvme0_smart => nvme0n1_smart} | 0
 2 files changed, 2 deletions(-)
 rename test/disk_tests/nvme_smart/{nvme0_smart => nvme0n1_smart} (100%)

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index f682e59..a311ffd 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -93,8 +93,6 @@ sub get_smart_data {
     my $smartdata = {};
     my $type;
 
-    $disk =~ s/n\d+$// if $disk =~ m!^/dev/nvme\d+n\d+$!;
-
     my $cmd = [$SMARTCTL, '-H'];
     push @$cmd, '-A', '-f', 'brief' if !$healthonly;
     push @$cmd, $disk;
diff --git a/test/disk_tests/nvme_smart/nvme0_smart b/test/disk_tests/nvme_smart/nvme0n1_smart
similarity index 100%
rename from test/disk_tests/nvme_smart/nvme0_smart
rename to test/disk_tests/nvme_smart/nvme0n1_smart
-- 
2.30.2





             reply	other threads:[~2022-12-12 12:33 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-12 12:33 Fiona Ebner [this message]
2022-12-13 12:28 ` [pve-devel] applied: " Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221212123309.109693-1-f.ebner@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal