public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH qemu-server/pve-manager] use qemu-xhci for new guests
@ 2022-11-10 14:35 Dominik Csapak
  2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 1/7] print_tabletdevice_full: make use of $q35 variable Dominik Csapak
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Dominik Csapak @ 2022-11-10 14:35 UTC (permalink / raw)
  To: pve-devel

qemu-xhci is to be preferred to nec-usb-xhci, but for compatibility
reasons, only use it for machine types >= 7.1 and ostypes l26 and
windows > 7 (older os' don't support it)
(depending on that machine version should be ok as long as we don't
roll out qemu 7.1 before applying this series)

With that controller we can specify how many ports we want (we chose 15,
the maximum) and for those guests we now set the usb devices on fixed
ports (usb0 gets port 1 and so on) so that we can enable hotplugging
too. Lastly, since we set the maximum of 15 ports, we can safely
use up to 14 of them (without qemu plugging a usb-hub in between)

For hotplugging we check the actual running qemu machine version, but
the gui only uses a heuristic based on the config (still good enough
since it allows more than the back end)

For guests with older machine types or other ostypes, the current
behaviour remains

Note: patch 1,2 of qemu-server are only cleanups/refactor that could be
applied independently (no behaviour change intended)
patch 1 of pve-manager could be squashed into 2, but if we decide
not to use that heuristic in the gui, can be applied separately

qemu-server:

Dominik Csapak (7):
  print_tabletdevice_full: make use of $q35 variable
  move 'windows_version' to Helpers
  USB: print_usbdevice_full: error out on invalid configuration
  USB: use machine_type_is_q35 instead of regex
  fix #4324: USB: use qemu-xhci for machine versions >= 7.1
  USB: increase max usb devices to 14 for newer machine version and
    ostype
  fix #3271: USB: allow usb hotplugging for modern guests

 PVE/API2/Qemu.pm                    |   2 +-
 PVE/QemuServer.pm                   | 134 ++++++++++++++++------------
 PVE/QemuServer/Cloudinit.pm         |   3 +-
 PVE/QemuServer/Helpers.pm           |  19 ++++
 PVE/QemuServer/USB.pm               |  94 ++++++++++++++-----
 test/cfg2cmd/qemu-xhci-7.1.conf     |  12 +++
 test/cfg2cmd/qemu-xhci-7.1.conf.cmd |  34 +++++++
 7 files changed, 220 insertions(+), 78 deletions(-)
 create mode 100644 test/cfg2cmd/qemu-xhci-7.1.conf
 create mode 100644 test/cfg2cmd/qemu-xhci-7.1.conf.cmd

pve-manager:

Dominik Csapak (2):
  ui: USBInputPanel: use correct maximum usb index
  ui: qemu: increase available usb ports depending on machine and ostype

 www/manager6/Utils.js             | 53 ++++++++++++++++++++++++++++++-
 www/manager6/qemu/HardwareView.js |  7 +++-
 www/manager6/qemu/USBEdit.js      |  7 +++-
 3 files changed, 64 insertions(+), 3 deletions(-)

-- 
2.30.2





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

* [pve-devel] [PATCH qemu-server 1/7] print_tabletdevice_full: make use of $q35 variable
  2022-11-10 14:35 [pve-devel] [PATCH qemu-server/pve-manager] use qemu-xhci for new guests Dominik Csapak
@ 2022-11-10 14:35 ` Dominik Csapak
  2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 2/7] move 'windows_version' to Helpers Dominik Csapak
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Dominik Csapak @ 2022-11-10 14:35 UTC (permalink / raw)
  To: pve-devel

just outside of context, we already save the result from
machine_type_is_q35 into the $q35 variable, but never use it.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/QemuServer.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 0a8442c..642c302 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -1457,7 +1457,7 @@ sub print_tabletdevice_full {
 
     # we use uhci for old VMs because tablet driver was buggy in older qemu
     my $usbbus;
-    if (PVE::QemuServer::Machine::machine_type_is_q35($conf) || $arch eq 'aarch64') {
+    if ($q35 || $arch eq 'aarch64') {
 	$usbbus = 'ehci';
     } else {
 	$usbbus = 'uhci';
-- 
2.30.2





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

* [pve-devel] [PATCH qemu-server 2/7] move 'windows_version' to Helpers
  2022-11-10 14:35 [pve-devel] [PATCH qemu-server/pve-manager] use qemu-xhci for new guests Dominik Csapak
  2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 1/7] print_tabletdevice_full: make use of $q35 variable Dominik Csapak
@ 2022-11-10 14:35 ` Dominik Csapak
  2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 3/7] USB: print_usbdevice_full: error out on invalid configuration Dominik Csapak
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Dominik Csapak @ 2022-11-10 14:35 UTC (permalink / raw)
  To: pve-devel

to avoid a cyclic dependency when we want to use that in PVE::QemuServer::USB

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Qemu.pm            |  2 +-
 PVE/QemuServer.pm           | 20 +-------------------
 PVE/QemuServer/Cloudinit.pm |  3 ++-
 PVE/QemuServer/Helpers.pm   | 19 +++++++++++++++++++
 4 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index a539b5c..30348e6 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -978,7 +978,7 @@ __PACKAGE__->register_method({
 		    my $machine = $conf->{machine};
 		    if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
 			# always pin Windows' machine version on create, they get to easily confused
-			if (PVE::QemuServer::windows_version($conf->{ostype})) {
+			if (PVE::QemuServer::Helpers::windows_version($conf->{ostype})) {
 			    $conf->{machine} = PVE::QemuServer::windows_get_pinned_machine_version($machine);
 			}
 		    }
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 642c302..06ffead 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -46,7 +46,7 @@ use PVE::Tools qw(run_command file_read_firstline file_get_contents dir_glob_for
 
 use PVE::QMPClient;
 use PVE::QemuConfig;
-use PVE::QemuServer::Helpers qw(min_version config_aware_timeout);
+use PVE::QemuServer::Helpers qw(min_version config_aware_timeout windows_version);
 use PVE::QemuServer::Cloudinit;
 use PVE::QemuServer::CGroup;
 use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options);
@@ -7955,24 +7955,6 @@ sub scsihw_infos {
     return ($maxdev, $controller, $controller_prefix);
 }
 
-sub windows_version {
-    my ($ostype) = @_;
-
-    return 0 if !$ostype;
-
-    my $winversion = 0;
-
-    if($ostype eq 'wxp' || $ostype eq 'w2k3' || $ostype eq 'w2k') {
-        $winversion = 5;
-    } elsif($ostype eq 'w2k8' || $ostype eq 'wvista') {
-        $winversion = 6;
-    } elsif ($ostype =~ m/^win(\d+)$/) {
-        $winversion = $1;
-    }
-
-    return $winversion;
-}
-
 sub resolve_dst_disk_format {
 	my ($storecfg, $storeid, $src_volname, $format) = @_;
 	my ($defFormat, $validFormats) = PVE::Storage::storage_default_format($storecfg, $storeid);
diff --git a/PVE/QemuServer/Cloudinit.pm b/PVE/QemuServer/Cloudinit.pm
index 3e93692..b616c7b 100644
--- a/PVE/QemuServer/Cloudinit.pm
+++ b/PVE/QemuServer/Cloudinit.pm
@@ -12,6 +12,7 @@ use Storable qw(dclone);
 use PVE::Tools qw(run_command file_set_contents);
 use PVE::Storage;
 use PVE::QemuServer;
+use PVE::QemuServer::Helpers;
 
 use constant CLOUDINIT_DISK_SIZE => 4 * 1024 * 1024; # 4MiB in bytes
 
@@ -71,7 +72,7 @@ sub get_cloudinit_format {
     # the new predicatble network device naming scheme.
     if (defined(my $ostype = $conf->{ostype})) {
 	return 'configdrive2'
-	    if PVE::QemuServer::windows_version($ostype);
+	    if PVE::QemuServer::Helpers::windows_version($ostype);
     }
 
     return 'nocloud';
diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
index a2ba20a..e91f906 100644
--- a/PVE/QemuServer/Helpers.pm
+++ b/PVE/QemuServer/Helpers.pm
@@ -13,6 +13,7 @@ use base 'Exporter';
 our @EXPORT_OK = qw(
 min_version
 config_aware_timeout
+windows_version
 );
 
 my $nodename = PVE::INotify::nodename();
@@ -185,4 +186,22 @@ sub pvecfg_min_version {
     die "internal error: cannot check version of invalid string '$verstr'";
 }
 
+sub windows_version {
+    my ($ostype) = @_;
+
+    return 0 if !$ostype;
+
+    my $winversion = 0;
+
+    if($ostype eq 'wxp' || $ostype eq 'w2k3' || $ostype eq 'w2k') {
+        $winversion = 5;
+    } elsif($ostype eq 'w2k8' || $ostype eq 'wvista') {
+        $winversion = 6;
+    } elsif ($ostype =~ m/^win(\d+)$/) {
+        $winversion = $1;
+    }
+
+    return $winversion;
+}
+
 1;
-- 
2.30.2





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

* [pve-devel] [PATCH qemu-server 3/7] USB: print_usbdevice_full: error out on invalid configuration
  2022-11-10 14:35 [pve-devel] [PATCH qemu-server/pve-manager] use qemu-xhci for new guests Dominik Csapak
  2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 1/7] print_tabletdevice_full: make use of $q35 variable Dominik Csapak
  2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 2/7] move 'windows_version' to Helpers Dominik Csapak
@ 2022-11-10 14:35 ` Dominik Csapak
  2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 4/7] USB: use machine_type_is_q35 instead of regex Dominik Csapak
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Dominik Csapak @ 2022-11-10 14:35 UTC (permalink / raw)
  To: pve-devel

should not happen normally, but an inattentive user of that function
may forget to check the validity of the parsed device, so err
on the safe side here

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/QemuServer/USB.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/PVE/QemuServer/USB.pm b/PVE/QemuServer/USB.pm
index 3c8da2c..b669c91 100644
--- a/PVE/QemuServer/USB.pm
+++ b/PVE/QemuServer/USB.pm
@@ -120,6 +120,8 @@ sub print_usbdevice_full {
 	$usbdevice .= ",vendorid=0x$device->{vendorid},productid=0x$device->{productid}";
     } elsif (defined($device->{hostbus}) && defined($device->{hostport})) {
 	$usbdevice .= ",hostbus=$device->{hostbus},hostport=$device->{hostport}";
+    } else {
+	die "no usb id or path given\n";
     }
 
     $usbdevice .= ",id=$deviceid";
-- 
2.30.2





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

* [pve-devel] [PATCH qemu-server 4/7] USB: use machine_type_is_q35 instead of regex
  2022-11-10 14:35 [pve-devel] [PATCH qemu-server/pve-manager] use qemu-xhci for new guests Dominik Csapak
                   ` (2 preceding siblings ...)
  2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 3/7] USB: print_usbdevice_full: error out on invalid configuration Dominik Csapak
@ 2022-11-10 14:35 ` Dominik Csapak
  2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 5/7] fix #4324: USB: use qemu-xhci for machine versions >= 7.1 Dominik Csapak
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Dominik Csapak @ 2022-11-10 14:35 UTC (permalink / raw)
  To: pve-devel

we refactored that into PVE::QemuServer::Machine a while ago, so we can
use it here

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/QemuServer/USB.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuServer/USB.pm b/PVE/QemuServer/USB.pm
index b669c91..10c9bca 100644
--- a/PVE/QemuServer/USB.pm
+++ b/PVE/QemuServer/USB.pm
@@ -3,6 +3,7 @@ package PVE::QemuServer::USB;
 use strict;
 use warnings;
 use PVE::QemuServer::PCI qw(print_pci_addr);
+use PVE::QemuServer::Machine;
 use PVE::JSONSchema;
 use base 'Exporter';
 
@@ -42,7 +43,7 @@ sub get_usb_controllers {
     if ($arch eq 'aarch64') {
         $pciaddr = print_pci_addr('ehci', $bridges, $arch, $machine);
         push @$devices, '-device', "usb-ehci,id=ehci$pciaddr";
-    } elsif ($machine !~ /q35/) { # FIXME: combine this and machine_type_is_q35
+    } elsif (!PVE::QemuServer::Machine::machine_type_is_q35($conf)) {
         $pciaddr = print_pci_addr("piix3", $bridges, $arch, $machine);
         push @$devices, '-device', "piix3-usb-uhci,id=uhci$pciaddr.0x2";
 
-- 
2.30.2





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

* [pve-devel] [PATCH qemu-server 5/7] fix #4324: USB: use qemu-xhci for machine versions >= 7.1
  2022-11-10 14:35 [pve-devel] [PATCH qemu-server/pve-manager] use qemu-xhci for new guests Dominik Csapak
                   ` (3 preceding siblings ...)
  2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 4/7] USB: use machine_type_is_q35 instead of regex Dominik Csapak
@ 2022-11-10 14:35 ` Dominik Csapak
  2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 6/7] USB: increase max usb devices to 14 for newer machine version and ostype Dominik Csapak
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Dominik Csapak @ 2022-11-10 14:35 UTC (permalink / raw)
  To: pve-devel

going by reports in the forum (e.g. [0]) and semi-official qemu
information[1], we should prefer qemu-xhci over nec-usb-xhci

for compatibility purposes, we guard that behind the machine version,
so that guests with a fixed version don't suddenly have a different usb
controller after a reboot (which could potentially break some hardcoded
guest configs)

0: https://forum.proxmox.com/threads/proxmox-usb-connect-disconnect-loop.117063/
1: https://www.kraxel.org/blog/2018/08/qemu-usb-tips/

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/QemuServer.pm                   |  8 +--
 PVE/QemuServer/USB.pm               | 76 +++++++++++++++++++++--------
 test/cfg2cmd/qemu-xhci-7.1.conf     | 12 +++++
 test/cfg2cmd/qemu-xhci-7.1.conf.cmd | 34 +++++++++++++
 4 files changed, 108 insertions(+), 22 deletions(-)
 create mode 100644 test/cfg2cmd/qemu-xhci-7.1.conf
 create mode 100644 test/cfg2cmd/qemu-xhci-7.1.conf.cmd

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 06ffead..452eb10 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -1092,7 +1092,9 @@ EODESCR
     usb3 => {
 	optional => 1,
 	type => 'boolean',
-	description => "Specifies whether if given host option is a USB3 device or port.",
+	description => "Specifies whether if given host option is a USB3 device or port."
+	    ." For modern guests (machine version >= 7.1 and ostype l26 and windows > 7), this flag"
+	    ." is irrelevant (all devices are plugged into a xhci controller).",
         default => 0,
     },
 };
@@ -3677,7 +3679,7 @@ sub config_to_command {
 
     # add usb controllers
     my @usbcontrollers = PVE::QemuServer::USB::get_usb_controllers(
-        $conf, $bridges, $arch, $machine_type, $usbdesc->{format}, $MAX_USB_DEVICES);
+	$conf, $bridges, $arch, $machine_type, $usbdesc->{format}, $MAX_USB_DEVICES, $machine_version);
     push @$devices, @usbcontrollers if @usbcontrollers;
     my $vga = parse_vga($conf->{vga});
 
@@ -3719,7 +3721,7 @@ sub config_to_command {
     $usb_dev_features->{spice_usb3} = 1 if min_version($machine_version, 4, 0);
 
     my @usbdevices = PVE::QemuServer::USB::get_usb_devices(
-        $conf, $usbdesc->{format}, $MAX_USB_DEVICES, $usb_dev_features, $bootorder);
+	$conf, $usbdesc->{format}, $MAX_USB_DEVICES, $usb_dev_features, $bootorder, $machine_version);
     push @$devices, @usbdevices if @usbdevices;
 
     # serial devices
diff --git a/PVE/QemuServer/USB.pm b/PVE/QemuServer/USB.pm
index 10c9bca..96931d4 100644
--- a/PVE/QemuServer/USB.pm
+++ b/PVE/QemuServer/USB.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 use PVE::QemuServer::PCI qw(print_pci_addr);
 use PVE::QemuServer::Machine;
+use PVE::QemuServer::Helpers qw(min_version windows_version);
 use PVE::JSONSchema;
 use base 'Exporter';
 
@@ -35,11 +36,16 @@ sub parse_usb_device {
 }
 
 sub get_usb_controllers {
-    my ($conf, $bridges, $arch, $machine, $format, $max_usb_devices) = @_;
+    my ($conf, $bridges, $arch, $machine, $format, $max_usb_devices, $machine_version) = @_;
 
     my $devices = [];
     my $pciaddr = "";
 
+    my $ostype = $conf->{ostype};
+
+    my $use_qemu_xhci = min_version($machine_version, 7, 1)
+	&& defined($ostype) && ($ostype eq 'l26' || windows_version($ostype) > 7);
+
     if ($arch eq 'aarch64') {
         $pciaddr = print_pci_addr('ehci', $bridges, $arch, $machine);
         push @$devices, '-device', "usb-ehci,id=ehci$pciaddr";
@@ -47,58 +53,75 @@ sub get_usb_controllers {
         $pciaddr = print_pci_addr("piix3", $bridges, $arch, $machine);
         push @$devices, '-device', "piix3-usb-uhci,id=uhci$pciaddr.0x2";
 
-        my $use_usb2 = 0;
-	for (my $i = 0; $i < $max_usb_devices; $i++)  {
-	    next if !$conf->{"usb$i"};
-	    my $d = eval { PVE::JSONSchema::parse_property_string($format,$conf->{"usb$i"}) };
-	    next if !$d || $d->{usb3}; # do not add usb2 controller if we have only usb3 devices
-	    $use_usb2 = 1;
+	if (!$use_qemu_xhci) {
+	    my $use_usb2 = 0;
+	    for (my $i = 0; $i < $max_usb_devices; $i++)  {
+		next if !$conf->{"usb$i"};
+		my $d = eval { PVE::JSONSchema::parse_property_string($format,$conf->{"usb$i"}) };
+		next if !$d || $d->{usb3}; # do not add usb2 controller if we have only usb3 devices
+		$use_usb2 = 1;
+	    }
+	    # include usb device config
+	    push @$devices, '-readconfig', '/usr/share/qemu-server/pve-usb.cfg' if $use_usb2;
 	}
-	# include usb device config
-	push @$devices, '-readconfig', '/usr/share/qemu-server/pve-usb.cfg' if $use_usb2;
     }
 
     # add usb3 controller if needed
 
     my $use_usb3 = 0;
+    my $use_usb = 0;
     for (my $i = 0; $i < $max_usb_devices; $i++)  {
 	next if !$conf->{"usb$i"};
 	my $d = eval { PVE::JSONSchema::parse_property_string($format,$conf->{"usb$i"}) };
-	next if !$d || !$d->{usb3};
-	$use_usb3 = 1;
+	next if !$d;
+	$use_usb = 1;
+	$use_usb3 = 1 if $d->{usb3};
     }
 
     $pciaddr = print_pci_addr("xhci", $bridges, $arch, $machine);
-    push @$devices, '-device', "nec-usb-xhci,id=xhci$pciaddr" if $use_usb3;
+    if ($use_qemu_xhci && $use_usb) {
+	push @$devices, '-device', print_qemu_xhci_controller($pciaddr);
+    } else {
+	push @$devices, '-device', "nec-usb-xhci,id=xhci$pciaddr" if $use_usb3;
+    }
 
     return @$devices;
 }
 
 sub get_usb_devices {
-    my ($conf, $format, $max_usb_devices, $features, $bootorder) = @_;
+    my ($conf, $format, $max_usb_devices, $features, $bootorder, $machine_version) = @_;
 
     my $devices = [];
 
+    my $ostype = $conf->{ostype};
+    my $use_qemu_xhci = min_version($machine_version, 7, 1)
+	&& defined($ostype) && ($ostype eq 'l26' || windows_version($ostype) > 7);
+
     for (my $i = 0; $i < $max_usb_devices; $i++)  {
 	my $devname = "usb$i";
 	next if !$conf->{$devname};
 	my $d = eval { PVE::JSONSchema::parse_property_string($format,$conf->{$devname}) };
 	next if !$d;
 
+	my $port;
+	if ($use_qemu_xhci) {
+	    $port = $i + 1;
+	}
+
 	if (defined($d->{host})) {
 	    my $hostdevice = parse_usb_device($d->{host});
 	    $hostdevice->{usb3} = $d->{usb3};
 	    if ($hostdevice->{spice}) {
 		# usb redir support for spice
 		my $bus = 'ehci';
-		$bus = 'xhci' if $hostdevice->{usb3} && $features->{spice_usb3};
+		$bus = 'xhci' if ($hostdevice->{usb3} && $features->{spice_usb3}) || $use_qemu_xhci;
 
 		push @$devices, '-chardev', "spicevmc,id=usbredirchardev$i,name=usbredir";
-		push @$devices, '-device', "usb-redir,chardev=usbredirchardev$i,id=usbredirdev$i,bus=$bus.0";
+		push @$devices, '-device', print_spice_usbdevice($i, $bus, $port);
 
 		warn "warning: spice usb port set as bootdevice, ignoring\n" if $bootorder->{$devname};
 	    } else {
-		push @$devices, '-device', print_usbdevice_full($conf, $devname, $hostdevice, $bootorder);
+		push @$devices, '-device', print_usbdevice_full($conf, $devname, $hostdevice, $bootorder, $port);
 	    }
 	}
     }
@@ -106,15 +129,30 @@ sub get_usb_devices {
     return @$devices;
 }
 
+sub print_qemu_xhci_controller {
+    my ($pciaddr) = @_;
+    return "qemu-xhci,p2=15,p3=15,id=xhci$pciaddr";
+}
+
+sub print_spice_usbdevice {
+    my ($index, $bus, $port) = @_;
+    my $device = "usb-redir,chardev=usbredirchardev$index,id=usbredirdev$index,bus=$bus.0";
+    if (defined($port)) {
+	$device .= ",port=$port";
+    }
+    return $device;
+}
+
 sub print_usbdevice_full {
-    my ($conf, $deviceid, $device, $bootorder) = @_;
+    my ($conf, $deviceid, $device, $bootorder, $port) = @_;
 
     return if !$device;
     my $usbdevice = "usb-host";
 
-    # if it is a usb3 device, attach it to the xhci controller, else omit the bus option
-    if($device->{usb3}) {
+    # if it is a usb3 device or with newer qemu, attach it to the xhci controller, else omit the bus option
+    if($device->{usb3} || defined($port)) {
 	$usbdevice .= ",bus=xhci.0";
+	$usbdevice .= ",port=$port" if defined($port);
     }
 
     if (defined($device->{vendorid}) && defined($device->{productid})) {
diff --git a/test/cfg2cmd/qemu-xhci-7.1.conf b/test/cfg2cmd/qemu-xhci-7.1.conf
new file mode 100644
index 0000000..3d50a1d
--- /dev/null
+++ b/test/cfg2cmd/qemu-xhci-7.1.conf
@@ -0,0 +1,12 @@
+# TEST: Test for new xhci controller with new machine version
+# QEMU_VERSION: 7.1.0
+cores: 2
+memory: 768
+name: spiceusb3
+net0: virtio=A2:C0:43:77:08:A1,bridge=vmbr0
+ostype: l26
+scsihw: virtio-scsi-pci
+smbios1: uuid=7b10d7af-b932-4c66-b2c3-3996152ec465
+vmgenid: c773c261-d800-4348-9f5d-167fadd53cf8
+vga: qxl
+usb1: spice
diff --git a/test/cfg2cmd/qemu-xhci-7.1.conf.cmd b/test/cfg2cmd/qemu-xhci-7.1.conf.cmd
new file mode 100644
index 0000000..af3a7e0
--- /dev/null
+++ b/test/cfg2cmd/qemu-xhci-7.1.conf.cmd
@@ -0,0 +1,34 @@
+/usr/bin/kvm \
+  -id 8006 \
+  -name 'spiceusb3,debug-threads=on' \
+  -no-shutdown \
+  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
+  -mon 'chardev=qmp,mode=control' \
+  -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
+  -mon 'chardev=qmp-event,mode=control' \
+  -pidfile /var/run/qemu-server/8006.pid \
+  -daemonize \
+  -smbios 'type=1,uuid=7b10d7af-b932-4c66-b2c3-3996152ec465' \
+  -smp '2,sockets=1,cores=2,maxcpus=2' \
+  -nodefaults \
+  -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
+  -vnc 'unix:/var/run/qemu-server/8006.vnc,password=on' \
+  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+  -m 768 \
+  -device 'pci-bridge,id=pci.1,chassis_nr=1,bus=pci.0,addr=0x1e' \
+  -device 'pci-bridge,id=pci.2,chassis_nr=2,bus=pci.0,addr=0x1f' \
+  -device 'vmgenid,guid=c773c261-d800-4348-9f5d-167fadd53cf8' \
+  -device 'piix3-usb-uhci,id=uhci,bus=pci.0,addr=0x1.0x2' \
+  -device 'qemu-xhci,p2=15,p3=15,id=xhci,bus=pci.1,addr=0x1b' \
+  -chardev 'spicevmc,id=usbredirchardev1,name=usbredir' \
+  -device 'usb-redir,chardev=usbredirchardev1,id=usbredirdev1,bus=xhci.0,port=2' \
+  -device 'qxl-vga,id=vga,max_outputs=4,bus=pci.0,addr=0x2' \
+  -device 'virtio-serial,id=spice,bus=pci.0,addr=0x9' \
+  -chardev 'spicevmc,id=vdagent,name=vdagent' \
+  -device 'virtserialport,chardev=vdagent,name=com.redhat.spice.0' \
+  -spice 'tls-port=61000,addr=127.0.0.1,tls-ciphers=HIGH,seamless-migration=on' \
+  -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3,free-page-reporting=on' \
+  -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
+  -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \
+  -device 'virtio-net-pci,mac=A2:C0:43:77:08:A1,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=300' \
+  -machine 'type=pc+pve0'
-- 
2.30.2





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

* [pve-devel] [PATCH qemu-server 6/7] USB: increase max usb devices to 14 for newer machine version and ostype
  2022-11-10 14:35 [pve-devel] [PATCH qemu-server/pve-manager] use qemu-xhci for new guests Dominik Csapak
                   ` (4 preceding siblings ...)
  2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 5/7] fix #4324: USB: use qemu-xhci for machine versions >= 7.1 Dominik Csapak
@ 2022-11-10 14:35 ` Dominik Csapak
  2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 7/7] fix #3271: USB: allow usb hotplugging for modern guests Dominik Csapak
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Dominik Csapak @ 2022-11-10 14:35 UTC (permalink / raw)
  To: pve-devel

for machine versions >= 7.1 and ostype linux or windows > 7, we use the
qemu-xhci controller where we have up to 14 usable ports, so make them
available to the user

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/QemuServer.pm     |  5 +++--
 PVE/QemuServer/USB.pm | 13 +++++++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 452eb10..3a0704a 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -823,7 +823,7 @@ while (my ($k, $v) = each %$confdesc) {
     PVE::JSONSchema::register_standard_option("pve-qm-$k", $v);
 }
 
-my $MAX_USB_DEVICES = 5;
+my $MAX_USB_DEVICES = 14;
 my $MAX_NETS = 32;
 my $MAX_SERIAL_PORTS = 4;
 my $MAX_PARALLEL_PORTS = 3;
@@ -1102,7 +1102,8 @@ EODESCR
 my $usbdesc = {
     optional => 1,
     type => 'string', format => $usb_fmt,
-    description => "Configure an USB device (n is 0 to 4).",
+    description => "Configure an USB device (n is 0 to 4, for machine version >= 7.1 and ostype"
+	." l26 or windows > 7, n can be up to 14).",
 };
 PVE::JSONSchema::register_standard_option("pve-qm-usb", $usbdesc);
 
diff --git a/PVE/QemuServer/USB.pm b/PVE/QemuServer/USB.pm
index 96931d4..7def04d 100644
--- a/PVE/QemuServer/USB.pm
+++ b/PVE/QemuServer/USB.pm
@@ -14,6 +14,8 @@ get_usb_controllers
 get_usb_devices
 );
 
+my $OLD_MAX_USB = 5;
+
 sub parse_usb_device {
     my ($value) = @_;
 
@@ -35,6 +37,15 @@ sub parse_usb_device {
     return $res;
 }
 
+my sub check_usb_index {
+    my ($index, $use_qemu_xhci) = @_;
+
+    die "using usb$index is only possible with machine type >= 7.1 and ostype l26 or windows > 7\n"
+	if $index >= $OLD_MAX_USB && !$use_qemu_xhci;
+
+    return undef;
+}
+
 sub get_usb_controllers {
     my ($conf, $bridges, $arch, $machine, $format, $max_usb_devices, $machine_version) = @_;
 
@@ -72,6 +83,7 @@ sub get_usb_controllers {
     my $use_usb = 0;
     for (my $i = 0; $i < $max_usb_devices; $i++)  {
 	next if !$conf->{"usb$i"};
+	check_usb_index($i, $use_qemu_xhci);
 	my $d = eval { PVE::JSONSchema::parse_property_string($format,$conf->{"usb$i"}) };
 	next if !$d;
 	$use_usb = 1;
@@ -100,6 +112,7 @@ sub get_usb_devices {
     for (my $i = 0; $i < $max_usb_devices; $i++)  {
 	my $devname = "usb$i";
 	next if !$conf->{$devname};
+	check_usb_index($i, $use_qemu_xhci);
 	my $d = eval { PVE::JSONSchema::parse_property_string($format,$conf->{$devname}) };
 	next if !$d;
 
-- 
2.30.2





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

* [pve-devel] [PATCH qemu-server 7/7] fix #3271: USB: allow usb hotplugging for modern guests
  2022-11-10 14:35 [pve-devel] [PATCH qemu-server/pve-manager] use qemu-xhci for new guests Dominik Csapak
                   ` (5 preceding siblings ...)
  2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 6/7] USB: increase max usb devices to 14 for newer machine version and ostype Dominik Csapak
@ 2022-11-10 14:35 ` Dominik Csapak
  2022-11-10 14:35 ` [pve-devel] [PATCH manager 1/2] ui: USBInputPanel: use correct maximum usb index Dominik Csapak
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Dominik Csapak @ 2022-11-10 14:35 UTC (permalink / raw)
  To: pve-devel

same as with the extended support for more usb devices, allow
hotplugging for guests that can use the qemu-xhci controller which
require a machine type >= 7.1 and a ostype l26 or windows > 7

if no usb device was passed through on startup, dynamically add
the xhci controller (and remove if the last usb device is unplugged)
so that live migration is still possible

much of the usb hotplug code was already there, but it still needed
a few adaptions, for example we have to add a chardev when adding
a spice redir port (that gets automatically removed when the
usb-redir device gets removed)

since the spice devices use the id 'usbredirdevX' instead of 'usbX', we
have to manually map that a bit around

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/QemuServer.pm | 99 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 68 insertions(+), 31 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 3a0704a..a107926 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -299,7 +299,9 @@ my $confdesc = {
 	type => 'string', format => 'pve-hotplug-features',
 	description => "Selectively enable hotplug features. This is a comma separated list of"
 	    ." hotplug features: 'network', 'disk', 'cpu', 'memory', 'usb' and 'cloudinit'. Use '0' to disable"
-	    ." hotplug completely. Using '1' as value is an alias for the default `network,disk,usb`.",
+	    ." hotplug completely. Using '1' as value is an alias for the default `network,disk,usb`."
+	    ." USB hotplugging is possible for guests with machine version >= 7.1 and ostype l26 or"
+	    ." windows > 7.",
         default => 'network,disk,usb',
     },
     reboot => {
@@ -4202,7 +4204,7 @@ sub vm_devices_list {
     # qom-list path=/machine/peripheral
     my $resperipheral = mon_cmd($vmid, 'qom-list', path => '/machine/peripheral');
     foreach my $per (@$resperipheral) {
-	if ($per->{name} =~ m/^usb\d+$/) {
+	if ($per->{name} =~ m/^usb(?:redirdev)?\d+$/) {
 	    $devices->{$per->{name}} = 1;
 	}
     }
@@ -4225,11 +4227,12 @@ sub vm_deviceplug {
 	qemu_deviceadd($vmid, print_tabletdevice_full($conf, $arch));
     } elsif ($deviceid eq 'keyboard') {
 	qemu_deviceadd($vmid, print_keyboarddevice_full($conf, $arch));
+    } elsif ($deviceid =~ m/^usbredirdev(\d+)$/) {
+	my $id = $1;
+	qemu_spice_usbredir_chardev_add($vmid, "usbredirchardev$id");
+	qemu_deviceadd($vmid, PVE::QemuServer::USB::print_spice_usbdevice($id, "xhci", $id + 1));
     } elsif ($deviceid =~ m/^usb(\d+)$/) {
-	die "usb hotplug currently not reliable\n";
-	# since we can't reliably hot unplug all added usb devices and usb
-	# passthrough breaks live migration we disable usb hotplugging for now
-	#qemu_deviceadd($vmid, PVE::QemuServer::USB::print_usbdevice_full($conf, $deviceid, $device));
+	qemu_deviceadd($vmid, PVE::QemuServer::USB::print_usbdevice_full($conf, $deviceid, $device, {}, $1 + 1));
     } elsif ($deviceid =~ m/^(virtio)(\d+)$/) {
 	qemu_iothread_add($vmid, $deviceid, $device);
 
@@ -4315,14 +4318,14 @@ sub vm_deviceunplug {
     my $bootdisks = PVE::QemuServer::Drive::get_bootdisks($conf);
     die "can't unplug bootdisk '$deviceid'\n" if grep {$_ eq $deviceid} @$bootdisks;
 
-    if ($deviceid eq 'tablet' || $deviceid eq 'keyboard') {
+    if ($deviceid eq 'tablet' || $deviceid eq 'keyboard' || $deviceid eq 'xhci') {
 	qemu_devicedel($vmid, $deviceid);
+    } elsif ($deviceid =~ m/^usbredirdev\d+$/) {
+	qemu_devicedel($vmid, $deviceid);
+	qemu_devicedelverify($vmid, $deviceid);
     } elsif ($deviceid =~ m/^usb\d+$/) {
-	die "usb hotplug currently not reliable\n";
-	# when unplugging usb devices this way, there may be remaining usb
-	# controllers/hubs so we disable it for now
-	#qemu_devicedel($vmid, $deviceid);
-	#qemu_devicedelverify($vmid, $deviceid);
+	qemu_devicedel($vmid, $deviceid);
+	qemu_devicedelverify($vmid, $deviceid);
     } elsif ($deviceid =~ m/^(virtio)(\d+)$/) {
 	my $device = parse_drive($deviceid, $conf->{$deviceid});
 
@@ -4354,6 +4357,20 @@ sub vm_deviceunplug {
     return 1;
 }
 
+sub qemu_spice_usbredir_chardev_add {
+    my ($vmid, $id) = @_;
+
+    mon_cmd($vmid, "chardev-add" , (
+	id => $id,
+	backend => {
+	    type => 'spicevmc',
+	    data => {
+		type => "usbredir",
+	    },
+	},
+    ));
+}
+
 sub qemu_deviceadd {
     my ($vmid, $devicefull) = @_;
 
@@ -4568,15 +4585,14 @@ sub qemu_usb_hotplug {
     vm_deviceunplug($vmid, $conf, $deviceid);
 
     # check if xhci controller is necessary and available
-    if ($device->{usb3}) {
-
-	my $devicelist = vm_devices_list($vmid);
+    my $devicelist = vm_devices_list($vmid);
 
-	if (!$devicelist->{xhci}) {
-	    my $pciaddr = print_pci_addr("xhci", undef, $arch, $machine_type);
-	    qemu_deviceadd($vmid, "nec-usb-xhci,id=xhci$pciaddr");
-	}
+    if (!$devicelist->{xhci}) {
+	my $pciaddr = print_pci_addr("xhci", undef, $arch, $machine_type);
+	qemu_deviceadd($vmid, PVE::QemuServer::USB::print_qemu_xhci_controller($pciaddr));
     }
+
+    # print_usbdevice_full expects the parsed device
     my $d = parse_usb_device($device->{host});
     $d->{usb3} = $device->{usb3};
 
@@ -4885,7 +4901,12 @@ sub vmconfig_hotplug_pending {
 	PVE::QemuConfig->write_config($vmid, $conf);
     }
 
+    my $ostype = $conf->{ostype};
+    my $version = extract_version($machine_type, get_running_qemu_version($vmid));
     my $hotplug_features = parse_hotplug_features(defined($conf->{hotplug}) ? $conf->{hotplug} : '1');
+    my $usb_hotplug = $hotplug_features->{usb}
+	&& min_version($version, 7, 1)
+	&& defined($ostype) && ($ostype eq 'l26' || windows_version($ostype) > 7);
 
     my $cgroup = PVE::QemuServer::CGroup->new($vmid);
     my $pending_delete_hash = PVE::QemuConfig->parse_pending_delete($conf->{pending}->{delete});
@@ -4905,11 +4926,11 @@ sub vmconfig_hotplug_pending {
 		    vm_deviceunplug($vmid, $conf, 'tablet');
 		    vm_deviceunplug($vmid, $conf, 'keyboard') if $arch eq 'aarch64';
 		}
-	    } elsif ($opt =~ m/^usb\d+/) {
-		die "skip\n";
-		# since we cannot reliably hot unplug usb devices we are disabling it
-		#die "skip\n" if !$hotplug_features->{usb} || $conf->{$opt} =~ m/spice/i;
-		#vm_deviceunplug($vmid, $conf, $opt);
+	    } elsif ($opt =~ m/^usb(\d+)$/) {
+		my $index = $1;
+		die "skip\n" if !$usb_hotplug;
+		vm_deviceunplug($vmid, $conf, "usbredirdev$index"); # if it's a spice port
+		vm_deviceunplug($vmid, $conf, $opt);
 	    } elsif ($opt eq 'vcpus') {
 		die "skip\n" if !$hotplug_features->{cpu};
 		qemu_cpu_hotplug($vmid, $conf, undef);
@@ -4963,13 +4984,15 @@ sub vmconfig_hotplug_pending {
 		    vm_deviceunplug($vmid, $conf, 'tablet');
 		    vm_deviceunplug($vmid, $conf, 'keyboard') if $arch eq 'aarch64';
 		}
-	    } elsif ($opt =~ m/^usb\d+$/) {
-		die "skip\n";
-		# since we cannot reliably hot unplug usb devices we disable it for now
-		#die "skip\n" if !$hotplug_features->{usb} || $value =~ m/spice/i;
-		#my $d = eval { parse_property_string($usbdesc->{format}, $value) };
-		#die "skip\n" if !$d;
-		#qemu_usb_hotplug($storecfg, $conf, $vmid, $opt, $d, $arch, $machine_type);
+	    } elsif ($opt =~ m/^usb(\d+)$/) {
+		my $index = $1;
+		die "skip\n" if !$usb_hotplug;
+		my $d = eval { parse_property_string($usbdesc->{format}, $value) };
+		my $id = $opt;
+		if ($d->{host} eq 'spice')  {
+		    $id = "usbredirdev$index";
+		}
+		qemu_usb_hotplug($storecfg, $conf, $vmid, $id, $d, $arch, $machine_type);
 	    } elsif ($opt eq 'vcpus') {
 		die "skip\n" if !$hotplug_features->{cpu};
 		qemu_cpu_hotplug($vmid, $conf, $value);
@@ -5019,6 +5042,20 @@ sub vmconfig_hotplug_pending {
 	    delete $conf->{pending}->{$opt};
 	}
     }
+
+    # unplug xhci controller if no usb device is left
+    if ($usb_hotplug) {
+	my $has_usb = 0;
+	for (my $i = 0; $i < $MAX_USB_DEVICES; $i++) {
+	    next if !defined($conf->{"usb$i"});
+	    $has_usb = 1;
+	    last;
+	}
+	if (!$has_usb) {
+	    vm_deviceunplug($vmid, $conf, 'xhci');
+	}
+    }
+
     PVE::QemuConfig->write_config($vmid, $conf);
 
     if($hotplug_features->{cloudinit}) {
-- 
2.30.2





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

* [pve-devel] [PATCH manager 1/2] ui: USBInputPanel: use correct maximum usb index
  2022-11-10 14:35 [pve-devel] [PATCH qemu-server/pve-manager] use qemu-xhci for new guests Dominik Csapak
                   ` (6 preceding siblings ...)
  2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 7/7] fix #3271: USB: allow usb hotplugging for modern guests Dominik Csapak
@ 2022-11-10 14:35 ` Dominik Csapak
  2022-11-10 14:36 ` [pve-devel] [PATCH manager 2/2] ui: qemu: increase available usb ports depending on machine and ostype Dominik Csapak
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Dominik Csapak @ 2022-11-10 14:35 UTC (permalink / raw)
  To: pve-devel

We already have that factored out in PVE.Utils, so use that.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/qemu/USBEdit.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/manager6/qemu/USBEdit.js b/www/manager6/qemu/USBEdit.js
index a2204584a..4373f82c3 100644
--- a/www/manager6/qemu/USBEdit.js
+++ b/www/manager6/qemu/USBEdit.js
@@ -17,7 +17,7 @@ Ext.define('PVE.qemu.USBInputPanel', {
     onGetValues: function(values) {
 	var me = this;
 	if (!me.confid) {
-	    for (let i = 0; i < 6; i++) {
+	    for (let i = 0; i < PVE.Utils.hardware_counts.usb; i++) {
 		let id = 'usb' + i.toString();
 		if (!me.vmconfig[id]) {
 		    me.confid = id;
-- 
2.30.2





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

* [pve-devel] [PATCH manager 2/2] ui: qemu: increase available usb ports depending on machine and ostype
  2022-11-10 14:35 [pve-devel] [PATCH qemu-server/pve-manager] use qemu-xhci for new guests Dominik Csapak
                   ` (7 preceding siblings ...)
  2022-11-10 14:35 ` [pve-devel] [PATCH manager 1/2] ui: USBInputPanel: use correct maximum usb index Dominik Csapak
@ 2022-11-10 14:36 ` Dominik Csapak
  2022-11-10 14:39 ` [pve-devel] [PATCH qemu-server/pve-manager] use qemu-xhci for new guests Dominik Csapak
  2022-11-10 17:02 ` [pve-devel] applied-series: " Thomas Lamprecht
  10 siblings, 0 replies; 12+ messages in thread
From: Dominik Csapak @ 2022-11-10 14:36 UTC (permalink / raw)
  To: pve-devel

in the backend, we allow up to 14 usb ports, but only if the vm can
use the qemu-xhci controller which is only possible since machine
version 7.1 and if the ostype is l26 or windows > 7

for this we introduce two helpers:
* qemu_min_version: modeled after the signature of 'min_version' from
  qemu-server, expects two arrays of versions and returns true if
  the first parameter is equal or greater than the second version
* get_max_usb_count looks at the given ostype and machine string
  and returns the proper maximum number

since we don't currently have the actual running version of the vm in
the gui, this is only a heuristic for running vms. but since the actual
running version could only be lower if none is set (e.g. for
migrated/long-running vms) we allow more in the gui and the backend will
do the proper thing (either hotplug it, or make it a pending change)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/Utils.js             | 53 ++++++++++++++++++++++++++++++-
 www/manager6/qemu/HardwareView.js |  7 +++-
 www/manager6/qemu/USBEdit.js      |  7 +++-
 3 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 7ca6a271b..adcf082ff 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1569,7 +1569,58 @@ Ext.define('PVE.Utils', {
 	}
     },
 
-    hardware_counts: { net: 32, usb: 5, hostpci: 16, audio: 1, efidisk: 1, serial: 4, rng: 1, tpmstate: 1 },
+    hardware_counts: {
+	net: 32,
+	usb: 14,
+	usb_old: 5,
+	hostpci: 16,
+	audio: 1,
+	efidisk: 1,
+	serial: 4,
+	rng: 1,
+	tpmstate: 1,
+    },
+
+    // we can have usb6 and up only for specific machine/ostypes
+    get_max_usb_count: function(ostype, machine) {
+	if (!ostype) {
+	    return PVE.Utils.hardware_counts.usb_old;
+	}
+
+	let match = /-(\d+).(\d+)/.exec(machine ?? '');
+	if (!match || PVE.Utils.qemu_min_version([match[1], match[2]], [7, 1])) {
+	    if (ostype === 'l26') {
+		return PVE.Utils.hardware_counts.usb;
+	    }
+	    let os_match = /^win(\d+)$/.exec(ostype);
+	    if (os_match && os_match[1] > 7) {
+		return PVE.Utils.hardware_counts.usb;
+	    }
+	}
+
+	return PVE.Utils.hardware_counts.usb_old;
+    },
+
+    // parameters are expected to be arrays, e.g. [7,1], [4,0,1]
+    // returns true if toCheck is equal or greater than minVersion
+    qemu_min_version: function(toCheck, minVersion) {
+	let i;
+	for (i = 0; i < toCheck.length && i < minVersion.length; i++) {
+	    if (toCheck[i] < minVersion[i]) {
+		return false;
+	    }
+	}
+
+	if (minVersion.length > toCheck.length) {
+	    for (; i < minVersion.length; i++) {
+		if (minVersion[i] !== 0) {
+		    return false;
+		}
+	    }
+	}
+
+	return true;
+    },
 
     cleanEmptyObjectKeys: function(obj) {
 	for (const propName of Object.keys(obj)) {
diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
index 6e9d03b4a..96fd37e98 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -544,6 +544,11 @@ Ext.define('PVE.qemu.HardwareView', {
 
 	let counts = {};
 	let isAtLimit = (type) => counts[type] >= PVE.Utils.hardware_counts[type];
+	let isAtUsbLimit = () => {
+	    let ostype = me.getObjectValue('ostype');
+	    let machine = me.getObjectValue('machine');
+	    return counts.usb >= PVE.Utils.get_max_usb_count(ostype, machine);
+	};
 
 	let set_button_status = function() {
 	    let selection_model = me.getSelectionModel();
@@ -570,7 +575,7 @@ Ext.define('PVE.qemu.HardwareView', {
 	    const noVMConfigNetPerm = !caps.vms['VM.Config.Network'];
 	    const noVMConfigDiskPerm = !caps.vms['VM.Config.Disk'];
 
-	    me.down('#addUsb').setDisabled(noSysConsolePerm || isAtLimit('usb'));
+	    me.down('#addUsb').setDisabled(noSysConsolePerm || isAtUsbLimit());
 	    me.down('#addPci').setDisabled(noSysConsolePerm || isAtLimit('hostpci'));
 	    me.down('#addAudio').setDisabled(noVMConfigHWTypePerm || isAtLimit('audio'));
 	    me.down('#addSerial').setDisabled(noVMConfigHWTypePerm || isAtLimit('serial'));
diff --git a/www/manager6/qemu/USBEdit.js b/www/manager6/qemu/USBEdit.js
index 4373f82c3..fe51d186f 100644
--- a/www/manager6/qemu/USBEdit.js
+++ b/www/manager6/qemu/USBEdit.js
@@ -12,12 +12,17 @@ Ext.define('PVE.qemu.USBInputPanel', {
     setVMConfig: function(vmconfig) {
 	var me = this;
 	me.vmconfig = vmconfig;
+	let max_usb = PVE.Utils.get_max_usb_count(me.vmconfig.ostype, me.vmconfig.machine);
+	if (max_usb > PVE.Utils.hardware_counts.usb_old) {
+	    me.down('field[name=usb3]').setDisabled(true);
+	}
     },
 
     onGetValues: function(values) {
 	var me = this;
 	if (!me.confid) {
-	    for (let i = 0; i < PVE.Utils.hardware_counts.usb; i++) {
+	    let max_usb = PVE.Utils.get_max_usb_count(me.vmconfig.ostype, me.vmconfig.machine);
+	    for (let i = 0; i < max_usb; i++) {
 		let id = 'usb' + i.toString();
 		if (!me.vmconfig[id]) {
 		    me.confid = id;
-- 
2.30.2





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

* Re: [pve-devel] [PATCH qemu-server/pve-manager] use qemu-xhci for new guests
  2022-11-10 14:35 [pve-devel] [PATCH qemu-server/pve-manager] use qemu-xhci for new guests Dominik Csapak
                   ` (8 preceding siblings ...)
  2022-11-10 14:36 ` [pve-devel] [PATCH manager 2/2] ui: qemu: increase available usb ports depending on machine and ostype Dominik Csapak
@ 2022-11-10 14:39 ` Dominik Csapak
  2022-11-10 17:02 ` [pve-devel] applied-series: " Thomas Lamprecht
  10 siblings, 0 replies; 12+ messages in thread
From: Dominik Csapak @ 2022-11-10 14:39 UTC (permalink / raw)
  To: pve-devel

forgot to mention: i tested the passthrough with various devices (mouse, usb key, onlykey..) and
the hotplugging with live migration with hot(un)plugging multiple times in various slots
without any problems




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

* [pve-devel] applied-series: [PATCH qemu-server/pve-manager] use qemu-xhci for new guests
  2022-11-10 14:35 [pve-devel] [PATCH qemu-server/pve-manager] use qemu-xhci for new guests Dominik Csapak
                   ` (9 preceding siblings ...)
  2022-11-10 14:39 ` [pve-devel] [PATCH qemu-server/pve-manager] use qemu-xhci for new guests Dominik Csapak
@ 2022-11-10 17:02 ` Thomas Lamprecht
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Lamprecht @ 2022-11-10 17:02 UTC (permalink / raw)
  To: Proxmox VE development discussion, Dominik Csapak

Am 10/11/2022 um 15:35 schrieb Dominik Csapak:
> qemu-xhci is to be preferred to nec-usb-xhci, but for compatibility
> reasons, only use it for machine types >= 7.1 and ostypes l26 and
> windows > 7 (older os' don't support it)
> (depending on that machine version should be ok as long as we don't
> roll out qemu 7.1 before applying this series)
> 
> With that controller we can specify how many ports we want (we chose 15,
> the maximum) and for those guests we now set the usb devices on fixed
> ports (usb0 gets port 1 and so on) so that we can enable hotplugging
> too. Lastly, since we set the maximum of 15 ports, we can safely
> use up to 14 of them (without qemu plugging a usb-hub in between)
> 
> For hotplugging we check the actual running qemu machine version, but
> the gui only uses a heuristic based on the config (still good enough
> since it allows more than the back end)
> 
> For guests with older machine types or other ostypes, the current
> behaviour remains
> 
> Note: patch 1,2 of qemu-server are only cleanups/refactor that could be
> applied independently (no behaviour change intended)
> patch 1 of pve-manager could be squashed into 2, but if we decide
> not to use that heuristic in the gui, can be applied separately
> 
> qemu-server:
> 
> Dominik Csapak (7):
>   print_tabletdevice_full: make use of $q35 variable
>   move 'windows_version' to Helpers
>   USB: print_usbdevice_full: error out on invalid configuration
>   USB: use machine_type_is_q35 instead of regex
>   fix #4324: USB: use qemu-xhci for machine versions >= 7.1
>   USB: increase max usb devices to 14 for newer machine version and
>     ostype
>   fix #3271: USB: allow usb hotplugging for modern guests
> 
>  PVE/API2/Qemu.pm                    |   2 +-
>  PVE/QemuServer.pm                   | 134 ++++++++++++++++------------
>  PVE/QemuServer/Cloudinit.pm         |   3 +-
>  PVE/QemuServer/Helpers.pm           |  19 ++++
>  PVE/QemuServer/USB.pm               |  94 ++++++++++++++-----
>  test/cfg2cmd/qemu-xhci-7.1.conf     |  12 +++
>  test/cfg2cmd/qemu-xhci-7.1.conf.cmd |  34 +++++++
>  7 files changed, 220 insertions(+), 78 deletions(-)
>  create mode 100644 test/cfg2cmd/qemu-xhci-7.1.conf
>  create mode 100644 test/cfg2cmd/qemu-xhci-7.1.conf.cmd
> 
> pve-manager:
> 
> Dominik Csapak (2):
>   ui: USBInputPanel: use correct maximum usb index
>   ui: qemu: increase available usb ports depending on machine and ostype
> 
>  www/manager6/Utils.js             | 53 ++++++++++++++++++++++++++++++-
>  www/manager6/qemu/HardwareView.js |  7 +++-
>  www/manager6/qemu/USBEdit.js      |  7 +++-
>  3 files changed, 64 insertions(+), 3 deletions(-)
> 


applied, with a bit of code cleanup as talked off-list, thanks!




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

end of thread, other threads:[~2022-11-10 17:03 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-10 14:35 [pve-devel] [PATCH qemu-server/pve-manager] use qemu-xhci for new guests Dominik Csapak
2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 1/7] print_tabletdevice_full: make use of $q35 variable Dominik Csapak
2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 2/7] move 'windows_version' to Helpers Dominik Csapak
2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 3/7] USB: print_usbdevice_full: error out on invalid configuration Dominik Csapak
2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 4/7] USB: use machine_type_is_q35 instead of regex Dominik Csapak
2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 5/7] fix #4324: USB: use qemu-xhci for machine versions >= 7.1 Dominik Csapak
2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 6/7] USB: increase max usb devices to 14 for newer machine version and ostype Dominik Csapak
2022-11-10 14:35 ` [pve-devel] [PATCH qemu-server 7/7] fix #3271: USB: allow usb hotplugging for modern guests Dominik Csapak
2022-11-10 14:35 ` [pve-devel] [PATCH manager 1/2] ui: USBInputPanel: use correct maximum usb index Dominik Csapak
2022-11-10 14:36 ` [pve-devel] [PATCH manager 2/2] ui: qemu: increase available usb ports depending on machine and ostype Dominik Csapak
2022-11-10 14:39 ` [pve-devel] [PATCH qemu-server/pve-manager] use qemu-xhci for new guests Dominik Csapak
2022-11-10 17:02 ` [pve-devel] applied-series: " 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