public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH pve-storage] dismanagement: account for leading white space in serial number
@ 2025-03-12  8:38 Shannon Sterz
  2025-04-06 19:19 ` [pve-devel] applied: " Thomas Lamprecht
  0 siblings, 1 reply; 4+ messages in thread
From: Shannon Sterz @ 2025-03-12  8:38 UTC (permalink / raw)
  To: pve-devel

some manufacturer seem to report leading white space in the
`ID_SERIAL_SHORT` field. the regex failed here, as it just didn't
match the whitespace at all.

reported on the forum:
https://forum.proxmox.com/threads/nvme-drive-serial-unknown.163480/#post-754953

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---

not sure this is the ideal fix, but i tried to stay on the more
conservative side here. alternatively the regex could be:

^E: ID_SERIAL_SHORT=(.+)$

but then the whitespace would be considered as part of the serial, not
sure this is intended or could have negative side effects.

 src/PVE/Diskmanage.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/Diskmanage.pm b/src/PVE/Diskmanage.pm
index 0217c75..4272668 100644
--- a/src/PVE/Diskmanage.pm
+++ b/src/PVE/Diskmanage.pm
@@ -328,7 +328,7 @@ sub get_udev_info {
     return if !defined($data->{devpath});

     $data->{serial} = 'unknown';
-    $data->{serial} = $1 if $info =~ m/^E: ID_SERIAL_SHORT=(\S+)$/m;
+    $data->{serial} = $1 if $info =~ m/^E: ID_SERIAL_SHORT=\s*(\S+)$/m;

     $data->{gpt} = $info =~ m/^E: ID_PART_TABLE_TYPE=gpt$/m ? 1 : 0;

--
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pve-devel] applied: [PATCH pve-storage] dismanagement: account for leading white space in serial number
  2025-03-12  8:38 [pve-devel] [PATCH pve-storage] dismanagement: account for leading white space in serial number Shannon Sterz
@ 2025-04-06 19:19 ` Thomas Lamprecht
  2025-04-07  8:00   ` Fiona Ebner
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Lamprecht @ 2025-04-06 19:19 UTC (permalink / raw)
  To: Proxmox VE development discussion, Shannon Sterz

did a s/dismanagement/disk management/ for the subject on applying this.

Am 12.03.25 um 09:38 schrieb Shannon Sterz:
> some manufacturer seem to report leading white space in the
> `ID_SERIAL_SHORT` field. the regex failed here, as it just didn't
> match the whitespace at all.
> 
> reported on the forum:
> https://forum.proxmox.com/threads/nvme-drive-serial-unknown.163480/#post-754953
> 
> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
> ---
> 
> not sure this is the ideal fix, but i tried to stay on the more
> conservative side here. alternatively the regex could be:
> 
> ^E: ID_SERIAL_SHORT=(.+)$
> 
> but then the whitespace would be considered as part of the serial, not
> sure this is intended or could have negative side effects.

As serials are often printed on labels it would be really error
prone to include spaces in them.

> 
>  src/PVE/Diskmanage.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks! What about spaces at the end though, are they already
parsed out and thus allowed?


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pve-devel] applied: [PATCH pve-storage] dismanagement: account for leading white space in serial number
  2025-04-06 19:19 ` [pve-devel] applied: " Thomas Lamprecht
@ 2025-04-07  8:00   ` Fiona Ebner
  2025-04-07 10:51     ` Thomas Lamprecht
  0 siblings, 1 reply; 4+ messages in thread
From: Fiona Ebner @ 2025-04-07  8:00 UTC (permalink / raw)
  To: Proxmox VE development discussion, Thomas Lamprecht, Shannon Sterz

Am 06.04.25 um 21:19 schrieb Thomas Lamprecht:
> did a s/dismanagement/disk management/ for the subject on applying this.
> 
> Am 12.03.25 um 09:38 schrieb Shannon Sterz:
>> some manufacturer seem to report leading white space in the
>> `ID_SERIAL_SHORT` field. the regex failed here, as it just didn't
>> match the whitespace at all.
>>
>> reported on the forum:
>> https://forum.proxmox.com/threads/nvme-drive-serial-unknown.163480/#post-754953
>>
>> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
>> ---
>>
>> not sure this is the ideal fix, but i tried to stay on the more
>> conservative side here. alternatively the regex could be:
>>
>> ^E: ID_SERIAL_SHORT=(.+)$
>>
>> but then the whitespace would be considered as part of the serial, not
>> sure this is intended or could have negative side effects.
> 
> As serials are often printed on labels it would be really error
> prone to include spaces in them.
> 
>>
>>  src/PVE/Diskmanage.pm | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>>
> 
> applied, thanks! What about spaces at the end though, are they already
> parsed out and thus allowed?

And I suppose we should make the behavior in PBS consistent with this
too? There, we get the value via udev_device_get_property_value() via
the udev crate. Haven't checked, but I'd be surprised if that wouldn't
pass along the value verbatim. OTOH, one could also argue that the
correct behavior is that, i.e. be transparent between user and udev.

We could also ask upstream if udev considers including the whitespace
correct behavior in the first place?


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pve-devel] applied: [PATCH pve-storage] dismanagement: account for leading white space in serial number
  2025-04-07  8:00   ` Fiona Ebner
@ 2025-04-07 10:51     ` Thomas Lamprecht
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2025-04-07 10:51 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fiona Ebner, Shannon Sterz

Am 07.04.25 um 10:00 schrieb Fiona Ebner:
> We could also ask upstream if udev considers including the whitespace
> correct behavior in the first place?

If there isn't anything documented or discussed already it might be
indeed worth to ask. Albeit I wouldn't be totally surprised if they do
not care much about what's the value here... OTOH, matching in udev
rules might actually need some defined/deterministic behavior here.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-04-07 10:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-03-12  8:38 [pve-devel] [PATCH pve-storage] dismanagement: account for leading white space in serial number Shannon Sterz
2025-04-06 19:19 ` [pve-devel] applied: " Thomas Lamprecht
2025-04-07  8:00   ` Fiona Ebner
2025-04-07 10:51     ` Thomas Lamprecht

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