Re: [pve-devel] [PATCH v3 qemu-server 06/10] convert drive device to json format

[Fiona Ebner <f.ebner@proxmox.com>]

Am 22.08.25 um 4:18 PM schrieb Alexandre Derumier via pve-devel:
> @@ -52,14 +52,18 @@ sub print_drivedevice_full {
>  
>      my $drive_id = PVE::QemuServer::Drive::get_drive_id($drive);
>      if ($drive->{interface} eq 'virtio') {
> -        my $pciaddr = print_pci_addr("$drive_id", $bridges, $arch);
> -        $device = 'virtio-blk-pci';
> +        my $pciaddr = get_pci_addr("$drive_id", $bridges, $arch);
> +        $device = {
> +            id => $drive_id,
> +            driver => 'virtio-blk-pci',
> +            bus => $pciaddr->{bus},
> +            addr => $pciaddr->{addr},

Doesn't the value need to be a coerced into a string?
# kvm -device virtio-blk-pci,help | grep addr
addr=<str> ...

> +        };
>          # for the switch to -blockdev, there is no blockdev for 'none'
>          if (!min_version($machine_version, 10, 0) || $drive->{file} ne 'none') {
> -            $device .= ",drive=drive-$drive_id";
> +            $device->{drive} = "drive-$drive_id";
>          }
> -        $device .= ",id=${drive_id}${pciaddr}";
> -        $device .= ",iothread=iothread-$drive_id" if $drive->{iothread};
> +        $device->{iothread} = "iothread-$drive_id" if $drive->{iothread};
>      } elsif ($drive->{interface} eq 'scsi') {
>  
>          my ($maxdev, $controller, $controller_prefix) = scsihw_infos($conf, $drive);
> @@ -68,39 +72,44 @@ sub print_drivedevice_full {
>          my $device_type =
>              PVE::QemuServer::Drive::get_scsi_device_type($drive, $storecfg, $machine_version);
>  
> +        $device = {
> +            id => $drive_id,
> +            driver => "scsi-$device_type",
> +            bus => "$controller_prefix$controller.0",
> +        };
> +
>          if (!$conf->{scsihw} || $conf->{scsihw} =~ m/^lsi/ || $conf->{scsihw} eq 'pvscsi') {
> -            $device = "scsi-$device_type,bus=$controller_prefix$controller.0,scsi-id=$unit";
> +            $device->{'scsi-id'} = $unit;

Please coerce into an int, just to be sure

>          } else {
> -            $device = "scsi-$device_type,bus=$controller_prefix$controller.0,channel=0,scsi-id=0"
> -                . ",lun=$drive->{index}";
> +            $device->{'scsi-id'} = 0;
> +            $device->{channel} = 0;
> +            $device->{lun} = int($drive->{index});
>          }
>          # for the switch to -blockdev, there is no blockdev for 'none'
>          if (!min_version($machine_version, 10, 0) || $drive->{file} ne 'none') {
> -            $device .= ",drive=drive-$drive_id";
> +            $device->{drive} = "drive-$drive_id";
>          }
> -        $device .= ",id=$drive_id";
>  
>          # For the switch to -blockdev, the SCSI device ID needs to be explicitly specified. Note
>          # that only ide-cd and ide-hd have a 'device_id' option.
>          if (
>              min_version($machine_version, 10, 0) && ($device_type eq 'cd' || $device_type eq 'hd')
>          ) {
> -            $device .= ",device_id=drive-${drive_id}";
> +            $device->{'device_id'} = "drive-${drive_id}";
>          }
>  
>          if ($drive->{ssd} && ($device_type eq 'block' || $device_type eq 'hd')) {
> -            $device .= ",rotation_rate=1";
> +            $device->{'rotation_rate'} = 1;
>          }
> -        $device .= ",wwn=$drive->{wwn}" if $drive->{wwn};
> -
> +        $device->{wwn} = $drive->{wwn} if $drive->{wwn};

Needs to be coerced to int (we have string in our schema)

# kvm -device scsi-hd,help | grep wwn
  port_wwn=<uint64>      -  (default: 0)
  wwn=<uint64>           -  (default: 0)


>         # only scsi-hd and scsi-cd support passing vendor and product information and have a
>          # 'write-cache' option
>          if ($device_type eq 'hd' || $device_type eq 'cd') {
>              if (my $vendor = $drive->{vendor}) {
> -                $device .= ",vendor=$vendor";
> +                $device->{vendor} = $vendor;

Please coerce into a string by surrounding with ""

>              }
>              if (my $product = $drive->{product}) {
> -                $device .= ",product=$product";
> +                $device->{product} = $product;

Please coerce into a string by surrounding with ""

>              }
>  
>              $has_write_cache = 1;
> @@ -134,27 +143,26 @@ sub print_drivedevice_full {
>          # backup if the type is 'ide-cd' instead.
>          $device_type = 'cd' if $conf->{template};
>  
> -        $device = "ide-$device_type";
> +        $device = { id => $drive_id, driver => "ide-$device_type" };
>          if ($drive->{interface} eq 'ide') {
> -            $device .= ",bus=ide.$controller,unit=$unit";
> +            $device->{bus} = "ide.$controller";
> +            $device->{unit} = $unit;

Please coerce into an int, just to be sure

>          } else {
> -            $device .= ",bus=ahci$controller.$unit";
> +            $device->{bus} = "ahci$controller.$unit";
>          }
>          if (!min_version($machine_version, 10, 0) || $drive->{file} ne 'none') {
> -            $device .= ",drive=drive-$drive_id";
> +            $device->{drive} = "drive-$drive_id";
>          }
> -        $device .= ",id=$drive_id";
>  
>          if ($device_type eq 'hd') {
>              if (my $model = $drive->{model}) {
> -                $model = URI::Escape::uri_unescape($model);
> -                $device .= ",model=$model";
> +                $device->{model} = URI::Escape::uri_unescape($model);
>              }
>              if ($drive->{ssd}) {
> -                $device .= ",rotation_rate=1";
> +                $device->{'rotation_rate'} = 1;
>              }
>          }
> -        $device .= ",wwn=$drive->{wwn}" if $drive->{wwn};
> +        $device->{wwn} = $drive->{wwn} if $drive->{wwn};

Needs to be coerced to int (we have string in our schema)

[I] root@pve9a1 ~# kvm -device ide-hd,help | grep wwn
  wwn=<uint64>           -  (default: 0)


>      } elsif ($drive->{interface} eq 'usb') {
>          die "implement me";
>          #  -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0
> @@ -162,11 +170,10 @@ sub print_drivedevice_full {
>          die "unsupported interface type";
>      }
>  
> -    $device .= ",bootindex=$drive->{bootindex}" if $drive->{bootindex};
> +    $device->{bootindex} = $drive->{bootindex} if $drive->{bootindex};

Please coerce into an int, just to be sure

>  
>      if (my $serial = $drive->{serial}) {
> -        $serial = URI::Escape::uri_unescape($serial);
> -        $device .= ",serial=$serial";
> +        $device->{serial} = URI::Escape::uri_unescape($serial);
>      }
>  
>      if (min_version($machine_version, 10, 0)) { # for the switch to -blockdev
> @@ -175,10 +182,10 @@ sub print_drivedevice_full {
>              if (my $cache = $drive->{cache}) {
>                  $write_cache = 'off' if $cache eq 'writethrough' || $cache eq 'directsync';
>              }
> -            $device .= ",write-cache=$write_cache";
> +            $device->{'write-cache'} = $write_cache;
>          }
>          for my $o (qw(rerror werror)) {
> -            $device .= ",$o=$drive->{$o}" if defined($drive->{$o});
> +            $device->{$o} = $drive->{$o} if defined($drive->{$o});
>          }
>      }
>  
> -- 
> 2.47.2
> 
> 



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