Re: [PATCH pve-stoage 3/4] iscsiPlugin: add wwid

["Daniel Kral" <d.kral@proxmox.com>]

Thanks for the patch!

Some notes inline

On Fri Mar 27, 2026 at 11:14 AM CET, Felix Driessler wrote:
> Signed-off-by: Felix Driessler <fdriessler@inett.de>
> ---
>  src/PVE/Storage/ISCSIPlugin.pm | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm
> index 30f4178..bb5576b 100644
> --- a/src/PVE/Storage/ISCSIPlugin.pm
> +++ b/src/PVE/Storage/ISCSIPlugin.pm
> @@ -305,13 +305,21 @@ sub iscsi_device_list {
>  
>                      my $volid = "$channel.$id.$lun.$blockdev";
>  
> +                    my $wwid = '';
> +                    my $dev = "/dev/$bdev";
> +                    $dev =~ m|^(/dev/[A-Za-z0-9_\-./]+)$|
> +                        or die "Invalid device path";
> +                    $dev = $1;
> +                    run_command(['/lib/udev/scsi_id', '-g', '-u', '-d', $dev], outfunc => sub { $wwid .= "$_[0]" });

Only a suggestion and haven't tested this extensively yet: Since this
code block already enumerates the iSCSI devices through the relevant
sysfs, we can also fetch the wwid with:

    my $wwid = file_read_firstline("/sys/bus/scsi/devices/$tmp/wwid");

Though this does have a different output depending on the WWN type [0]
and this should probably also check whether the device actually has a
WWID (just a check whether the file 'wwid' exists).

[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/scsi/scsi_lib.c?h=v7.0#n3348

> +
>                      $res->{$target}->{$volid} = {
> -                        'format' => 'raw',
> -                        'size' => int($size * 512),
> -                        'vmid' => 0, # not assigned to any vm
> -                        'channel' => int($channel),
> -                        'id' => int($id),
> -                        'lun' => int($lun),
> +                        'format'    => 'raw',
> +                        'size'      => int($size * 512),
> +                        'vmid'      => 0, # not assigned to any vm
> +                        'channel'   => int($channel),
> +                        'id'        => int($id),
> +                        'lun'       => int($lun),

Aligning the indentation of the other existing properties should be done
in a separate patch before this.

Though our `make tidy` target will revert this back to the previous
formatting.

> +                        'wwid'      => $wwid

nit: add a , at the end of the new property

>                      };
>  
>                      #print "TEST: $target $session $host,$bus,$tg,$lun $blockdev\n";