all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH v2 qemu-server] fix #4957: add vendor and product information passthrough for SCSI-Disks
@ 2023-11-08  8:51 Hannes Duerr
  2023-11-08 10:04 ` Fiona Ebner
  0 siblings, 1 reply; 5+ messages in thread
From: Hannes Duerr @ 2023-11-08  8:51 UTC (permalink / raw)
  To: pve-devel

adds vendor and product information for SCSI devices to the json schema and
checks in the VM create/update API call if it is possible to add these to QEMU as a device option

Signed-off-by: Hannes Duerr <h.duerr@proxmox.com>
---

changes in v2:
- when calling the API to create/update a VM, check whether the devices
are "scsi-hd" or "scsi-cd" devices,where there is the option to add
vendor and product information, if not error out
- change the format in product_fmt and vendor_fmt to a pattern that only
allows 40 characters consisting of upper and lower case letters, numbers and '-' and '_'.

 PVE/API2/Qemu.pm        |  9 +++++
 PVE/QemuServer.pm       | 83 +++++++++++++++++++++++++++++------------
 PVE/QemuServer/Drive.pm | 24 ++++++++++++
 3 files changed, 92 insertions(+), 24 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 38bdaab..6898ec9 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1030,6 +1030,11 @@ __PACKAGE__->register_method({
 		    );
 		    $conf->{$_} = $created_opts->{$_} for keys $created_opts->%*;
 
+		    foreach my $opt (keys $created_opts->%*) {
+			if ($opt =~ m/scsi/) {
+			    PVE::QemuServer::check_scsi_feature_compatibility($opt, $created_opts, $conf, $storecfg, $param);
+			}
+		    }
 		    if (!$conf->{boot}) {
 			my $devs = PVE::QemuServer::get_default_bootdevices($conf);
 			$conf->{boot} = PVE::QemuServer::print_bootorder($devs);
@@ -1840,6 +1845,10 @@ my $update_vm_api  = sub {
 		    );
 		    $conf->{pending}->{$_} = $created_opts->{$_} for keys $created_opts->%*;
 
+		    if ($opt =~ m/scsi/) {
+			PVE::QemuServer::check_scsi_feature_compatibility($opt, $created_opts, $conf, $storecfg, $param);
+		    }
+
 		    # default legacy boot order implies all cdroms anyway
 		    if (@bootorder) {
 			# append new CD drives to bootorder to mark them bootable
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index dbcd568..919728b 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -26,6 +26,7 @@ use Storable qw(dclone);
 use Time::HiRes qw(gettimeofday usleep);
 use URI::Escape;
 use UUID;
+use Data::Dumper;
 
 use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_write_file);
 use PVE::CGroup;
@@ -1428,6 +1429,53 @@ my sub get_drive_id {
     return "$drive->{interface}$drive->{index}";
 }
 
+sub get_scsi_devicetype {
+    my ($drive, $storecfg, $machine_type) = @_;
+
+    my $devicetype = 'hd';
+    my $path = '';
+    if (drive_is_cdrom($drive)) {
+	$devicetype = 'cd';
+    } else {
+	if ($drive->{file} =~ m|^/|) {
+	    $path = $drive->{file};
+	    if (my $info = path_is_scsi($path)) {
+		if ($info->{type} == 0 && $drive->{scsiblock}) {
+		    $devicetype = 'block';
+		} elsif ($info->{type} == 1) { # tape
+		    $devicetype = 'generic';
+		}
+	    }
+	} else {
+	    $path = PVE::Storage::path($storecfg, $drive->{file});
+	}
+
+	# for compatibility only, we prefer scsi-hd (#2408, #2355, #2380)
+	my $version = kvm_user_version();
+	$version = extract_version($machine_type, $version);
+	if ($path =~ m/^iscsi\:\/\// &&
+	   !min_version($version, 4, 1)) {
+	    $devicetype = 'generic';
+	}
+    }
+
+    return $devicetype;
+}
+
+sub check_scsi_feature_compatibility {
+    my($opt, $created_opts, $conf, $storecfg, $param) = @_;
+
+    my $drive = parse_drive($opt, $created_opts->{$opt});
+    my $machine_type = get_vm_machine($conf, undef, $conf->{arch});
+    my $drivetype = get_scsi_devicetype($drive, $storecfg, $machine_type);
+
+    if ($drivetype ne 'hd' && $drivetype ne 'cd') {
+	if ($param->{$opt} =~ m/vendor/ || $param->{$opt} =~ m/product/) {
+	    die "only 'scsi-hd' and 'scsi-cd' devices support passing vendor and product information\n";
+	}
+    }
+}
+
 sub print_drivedevice_full {
     my ($storecfg, $conf, $vmid, $drive, $bridges, $arch, $machine_type) = @_;
 
@@ -1443,31 +1491,8 @@ sub print_drivedevice_full {
 
 	my ($maxdev, $controller, $controller_prefix) = scsihw_infos($conf, $drive);
 	my $unit = $drive->{index} % $maxdev;
-	my $devicetype = 'hd';
-	my $path = '';
-	if (drive_is_cdrom($drive)) {
-	    $devicetype = 'cd';
-	} else {
-	    if ($drive->{file} =~ m|^/|) {
-		$path = $drive->{file};
-		if (my $info = path_is_scsi($path)) {
-		    if ($info->{type} == 0 && $drive->{scsiblock}) {
-			$devicetype = 'block';
-		    } elsif ($info->{type} == 1) { # tape
-			$devicetype = 'generic';
-		    }
-		}
-	    } else {
-		 $path = PVE::Storage::path($storecfg, $drive->{file});
-	    }
 
-	    # for compatibility only, we prefer scsi-hd (#2408, #2355, #2380)
-	    my $version = extract_version($machine_type, kvm_user_version());
-	    if ($path =~ m/^iscsi\:\/\// &&
-	       !min_version($version, 4, 1)) {
-		$devicetype = 'generic';
-	    }
-	}
+	my $devicetype  = get_scsi_devicetype($drive, $storecfg, $machine_type);
 
 	if (!$conf->{scsihw} || $conf->{scsihw} =~ m/^lsi/ || $conf->{scsihw} eq 'pvscsi') {
 	    $device = "scsi-$devicetype,bus=$controller_prefix$controller.0,scsi-id=$unit";
@@ -1482,6 +1507,16 @@ sub print_drivedevice_full {
 	}
 	$device .= ",wwn=$drive->{wwn}" if $drive->{wwn};
 
+	# only scsi-hd and scsi-cd support passing vendor and product information
+	if ($devicetype eq 'hd' || $devicetype eq 'cd') {
+	    if (my $vendor = $drive->{vendor}) {
+		$device .= ",vendor=$vendor";
+	    }
+	    if (my $product = $drive->{product}) {
+		$device .= ",product=$product";
+	    }
+	}
+
     } elsif ($drive->{interface} eq 'ide' || $drive->{interface} eq 'sata') {
 	my $maxdev = ($drive->{interface} eq 'sata') ? $PVE::QemuServer::Drive::MAX_SATA_DISKS : 2;
 	my $controller = int($drive->{index} / $maxdev);
diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
index e24ba12..021f182 100644
--- a/PVE/QemuServer/Drive.pm
+++ b/PVE/QemuServer/Drive.pm
@@ -159,6 +159,26 @@ my %iothread_fmt = ( iothread => {
 	optional => 1,
 });
 
+my %product_fmt = (
+    product => {
+	type => 'string',
+	pattern => '[A-Za-z0-9\-_]{,40}',
+	format_description => 'product',
+	description => "The drive's product name, up to 40 bytes long.",
+	optional => 1,
+    },
+);
+
+my %vendor_fmt = (
+    vendor => {
+	type => 'string',
+	pattern => '[A-Za-z0-9\-_]{,40}',
+	format_description => 'vendor',
+	description => "The drive's vendor name, up to 40 bytes long.",
+	optional => 1,
+    },
+);
+
 my %model_fmt = (
     model => {
 	type => 'string',
@@ -281,6 +301,8 @@ my $scsi_fmt = {
     %scsiblock_fmt,
     %ssd_fmt,
     %wwn_fmt,
+    %vendor_fmt,
+    %product_fmt,
 };
 my $scsidesc = {
     optional => 1,
@@ -404,6 +426,8 @@ my $alldrive_fmt = {
     %readonly_fmt,
     %scsiblock_fmt,
     %ssd_fmt,
+    %vendor_fmt,
+    %product_fmt,
     %wwn_fmt,
     %tpmversion_fmt,
     %efitype_fmt,
-- 
2.39.2





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

end of thread, other threads:[~2023-11-09  8:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-08  8:51 [pve-devel] [PATCH v2 qemu-server] fix #4957: add vendor and product information passthrough for SCSI-Disks Hannes Duerr
2023-11-08 10:04 ` Fiona Ebner
2023-11-08 14:28   ` Hannes Dürr
2023-11-08 14:43     ` Thomas Lamprecht
2023-11-09  8:34     ` Fiona Ebner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal