From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 20E261FF183 for ; Wed, 18 Jun 2025 15:08:52 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id DCF6D13499; Wed, 18 Jun 2025 15:09:06 +0200 (CEST) From: Fiona Ebner To: pve-devel@lists.proxmox.com Date: Wed, 18 Jun 2025 15:01:59 +0200 Message-Id: <20250618130209.90649-23-f.ebner@proxmox.com> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20250618130209.90649-1-f.ebner@proxmox.com> References: <20250618130209.90649-1-f.ebner@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.032 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [pci.pm, rng.pm, qemuserver.pm, usb.pm] Subject: [pve-devel] [PATCH qemu-server v2 22/32] pci: code cleanup: remove superfluous machine type paramater from print_pci_addr X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" The only supported machine for aarch64 is 'virt', so there is no need to check if that is the machine. Also many (all?) other machines for aarch64 in QEMU also don't have a 'pci' bus by default. The parameter is also transitively removed from the functions: 1. print_hostpci_devices() 2. print_rng_device_commandline() 3. get_usb_controllers() 4. print_netdevice_full() 5. print_vga_device() Should this ever be required in the future again, or also for the $arch itself right now, it would be nicer to properly abstract this away instead of passing the parameters along everywhere, e.g. by using a class. Signed-off-by: Fiona Ebner --- New in v2. src/PVE/QemuServer.pm | 79 ++++++++++++--------------------------- src/PVE/QemuServer/PCI.pm | 10 ++--- src/PVE/QemuServer/RNG.pm | 4 +- src/PVE/QemuServer/USB.pm | 8 ++-- 4 files changed, 35 insertions(+), 66 deletions(-) diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm index 48ae41c0..bffce201 100644 --- a/src/PVE/QemuServer.pm +++ b/src/PVE/QemuServer.pm @@ -1405,7 +1405,7 @@ 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, $machine_type); + my $pciaddr = print_pci_addr("$drive_id", $bridges, $arch); $device = "virtio-blk-pci,drive=drive-$drive_id,id=${drive_id}${pciaddr}"; $device .= ",iothread=iothread-$drive_id" if $drive->{iothread}; } elsif ($drive->{interface} eq 'scsi') { @@ -1616,24 +1616,14 @@ sub print_pbs_blockdev { } sub print_netdevice_full { - my ( - $vmid, - $conf, - $net, - $netid, - $bridges, - $use_old_bios_files, - $arch, - $machine_type, - $machine_version, - ) = @_; + my ($vmid, $conf, $net, $netid, $bridges, $use_old_bios_files, $arch, $machine_version) = @_; my $device = $net->{model}; if ($net->{model} eq 'virtio') { $device = 'virtio-net-pci'; } - my $pciaddr = print_pci_addr("$netid", $bridges, $arch, $machine_type); + my $pciaddr = print_pci_addr("$netid", $bridges, $arch); my $tmpstr = "$device,mac=$net->{macaddr},netdev=$netid$pciaddr,id=$netid"; if ($net->{queues} && $net->{queues} > 1 && $net->{model} eq 'virtio') { # Consider we have N queues, the number of vectors needed is 2 * N + 2, i.e., one per in @@ -1736,7 +1726,7 @@ my $vga_map = { }; sub print_vga_device { - my ($conf, $vga, $arch, $machine_version, $machine, $id, $qxlnum, $bridges) = @_; + my ($conf, $vga, $arch, $machine_version, $id, $qxlnum, $bridges) = @_; my $type = $vga_map->{ $vga->{type} }; if ($arch eq 'aarch64' && defined($type) && $type eq 'virtio-vga') { @@ -1788,7 +1778,7 @@ sub print_vga_device { # the first display uses pcie.0 bus on q35 machines $pciaddr = print_pcie_addr($vgaid); } else { - $pciaddr = print_pci_addr($vgaid, $bridges, $arch, $machine); + $pciaddr = print_pci_addr($vgaid, $bridges, $arch); } if ($vga->{type} eq 'virtio-gl') { @@ -3720,9 +3710,8 @@ sub config_to_command { } # add usb controllers - my @usbcontrollers = PVE::QemuServer::USB::get_usb_controllers( - $conf, $bridges, $arch, $machine_type, $machine_version, - ); + my @usbcontrollers = + PVE::QemuServer::USB::get_usb_controllers($conf, $bridges, $arch, $machine_version); push @$devices, @usbcontrollers if @usbcontrollers; my ($vga, $qxlnum) = get_vga_properties($conf, $arch, $machine_version, $winversion); @@ -3746,15 +3735,7 @@ sub config_to_command { # host pci device passthrough my ($kvm_off, $gpu_passthrough, $legacy_igd, $pci_devices) = PVE::QemuServer::PCI::print_hostpci_devices( - $vmid, - $conf, - $devices, - $vga, - $winversion, - $bridges, - $arch, - $machine_type, - $bootorder, + $vmid, $conf, $devices, $vga, $winversion, $bridges, $arch, $bootorder, ); # usb devices @@ -3798,7 +3779,7 @@ sub config_to_command { } if (min_version($machine_version, 4, 0) && (my $audio = conf_has_audio($conf))) { - my $audiopciaddr = print_pci_addr("audio0", $bridges, $arch, $machine_type); + my $audiopciaddr = print_pci_addr("audio0", $bridges, $arch); my $audio_devs = audio_devs($audio, $audiopciaddr, $machine_version); push @$devices, @$audio_devs; } @@ -3843,9 +3824,7 @@ sub config_to_command { if ($vga->{type} && $vga->{type} !~ m/^serial\d+$/ && $vga->{type} ne 'none') { push @$devices, '-device', - print_vga_device( - $conf, $vga, $arch, $machine_version, $machine_type, undef, $qxlnum, $bridges, - ); + print_vga_device($conf, $vga, $arch, $machine_version, undef, $qxlnum, $bridges); push @$cmd, '-display', 'egl-headless,gl=core' if $vga->{type} eq 'virtio-gl'; # VIRGL @@ -3915,7 +3894,7 @@ sub config_to_command { push @$devices, '-chardev', "socket,path=$qgasocket,server=on,wait=off,id=qga0"; if (!$guest_agent->{type} || $guest_agent->{type} eq 'virtio') { - my $pciaddr = print_pci_addr("qga0", $bridges, $arch, $machine_type); + my $pciaddr = print_pci_addr("qga0", $bridges, $arch); push @$devices, '-device', "virtio-serial,id=qga0$pciaddr"; push @$devices, '-device', 'virtserialport,chardev=qga0,name=org.qemu.guest_agent.0'; } elsif ($guest_agent->{type} eq 'isa') { @@ -3926,7 +3905,7 @@ sub config_to_command { my $rng = $conf->{rng0} ? parse_rng($conf->{rng0}) : undef; if ($rng && $version_guard->(4, 1, 2)) { my $rng_object = print_rng_object_commandline('rng0', $rng); - my $rng_device = print_rng_device_commandline('rng0', $rng, $bridges, $arch, $machine_type); + my $rng_device = print_rng_device_commandline('rng0', $rng, $bridges, $arch); push @$devices, '-object', $rng_object; push @$devices, '-device', $rng_device; } @@ -3942,14 +3921,7 @@ sub config_to_command { for (my $i = 1; $i < $qxlnum; $i++) { push @$devices, '-device', print_vga_device( - $conf, - $vga, - $arch, - $machine_version, - $machine_type, - $i, - $qxlnum, - $bridges, + $conf, $vga, $arch, $machine_version, $i, $qxlnum, $bridges, ); } } else { @@ -3964,7 +3936,7 @@ sub config_to_command { } } - my $pciaddr = print_pci_addr("spice", $bridges, $arch, $machine_type); + my $pciaddr = print_pci_addr("spice", $bridges, $arch); push @$devices, '-device', "virtio-serial,id=spice$pciaddr"; if ($vga->{'clipboard'} && $vga->{'clipboard'} eq 'vnc') { @@ -4002,7 +3974,7 @@ sub config_to_command { # enable balloon by default, unless explicitly disabled if (!defined($conf->{balloon}) || $conf->{balloon}) { - my $pciaddr = print_pci_addr("balloon0", $bridges, $arch, $machine_type); + my $pciaddr = print_pci_addr("balloon0", $bridges, $arch); my $ballooncmd = "virtio-balloon-pci,id=balloon0$pciaddr"; $ballooncmd .= ",free-page-reporting=on" if min_version($machine_version, 6, 2); push @$devices, '-device', $ballooncmd; @@ -4010,7 +3982,7 @@ sub config_to_command { if ($conf->{watchdog}) { my $wdopts = parse_watchdog($conf->{watchdog}); - my $pciaddr = print_pci_addr("watchdog", $bridges, $arch, $machine_type); + my $pciaddr = print_pci_addr("watchdog", $bridges, $arch); my $watchdog = $wdopts->{model} || 'i6300esb'; push @$devices, '-device', "$watchdog$pciaddr"; push @$devices, '-watchdog-action', $wdopts->{action} if $wdopts->{action}; @@ -4051,8 +4023,7 @@ sub config_to_command { "scsi$drive->{index}: machine version 4.1~pve2 or higher is required to use more than 14 SCSI disks\n" if $drive->{index} > 13 && !&$version_guard(4, 1, 2); - my $pciaddr = - print_pci_addr("$controller_prefix$controller", $bridges, $arch, $machine_type); + my $pciaddr = print_pci_addr("$controller_prefix$controller", $bridges, $arch); my $scsihw_type = $scsihw =~ m/^virtio-scsi-single/ ? "virtio-scsi-pci" : $scsihw; @@ -4087,7 +4058,7 @@ sub config_to_command { if ($drive->{interface} eq 'sata') { my $controller = int($drive->{index} / $PVE::QemuServer::Drive::MAX_SATA_DISKS); - my $pciaddr = print_pci_addr("ahci$controller", $bridges, $arch, $machine_type); + my $pciaddr = print_pci_addr("ahci$controller", $bridges, $arch); push @$devices, '-device', "ahci,id=ahci$controller,multifunction=on$pciaddr" if !$ahcicontroller->{$controller}; $ahcicontroller->{$controller} = 1; @@ -4137,7 +4108,6 @@ sub config_to_command { $bridges, $use_old_bios_files, $arch, - $machine_type, $machine_version, ); @@ -4151,7 +4121,7 @@ sub config_to_command { if ($q35) { $bus = print_pcie_addr("ivshmem"); } else { - $bus = print_pci_addr("ivshmem", $bridges, $arch, $machine_type); + $bus = print_pci_addr("ivshmem", $bridges, $arch); } my $ivshmem_name = $ivshmem->{name} // $vmid; @@ -4180,7 +4150,7 @@ sub config_to_command { if ($k == 2 && $legacy_igd) { $k_name = "$k-igd"; } - my $pciaddr = print_pci_addr("pci.$k_name", undef, $arch, $machine_type); + my $pciaddr = print_pci_addr("pci.$k_name", undef, $arch); my $devstr = "pci-bridge,id=pci.$k,chassis_nr=$k$pciaddr"; if ($q35) { # add after -readconfig pve-q35.cfg @@ -4344,7 +4314,7 @@ sub vm_deviceplug { } } elsif ($deviceid =~ m/^(virtioscsi|scsihw)(\d+)$/) { my $scsihw = defined($conf->{scsihw}) ? $conf->{scsihw} : "lsi"; - my $pciaddr = print_pci_addr($deviceid, undef, $arch, $machine_type); + my $pciaddr = print_pci_addr($deviceid, undef, $arch); my $scsihw_type = $scsihw eq 'virtio-scsi-single' ? "virtio-scsi-pci" : $scsihw; my $devicefull = "$scsihw_type,id=$deviceid$pciaddr"; @@ -4388,7 +4358,6 @@ sub vm_deviceplug { undef, $use_old_bios_files, $arch, - $machine_type, $machine_version, ); qemu_deviceadd($vmid, $netdevicefull); @@ -4403,7 +4372,7 @@ sub vm_deviceplug { } } elsif (!$q35 && $deviceid =~ m/^(pci\.)(\d+)$/) { my $bridgeid = $2; - my $pciaddr = print_pci_addr($deviceid, undef, $arch, $machine_type); + my $pciaddr = print_pci_addr($deviceid, undef, $arch); my $devicefull = "pci-bridge,id=pci.$bridgeid,chassis_nr=$bridgeid$pciaddr"; qemu_deviceadd($vmid, $devicefull); @@ -4609,7 +4578,7 @@ sub qemu_add_pci_bridge { my $bridgeid; - print_pci_addr($device, $bridges, $arch, $machine_type); + print_pci_addr($device, $bridges, $arch); while (my ($k, $v) = each %$bridges) { $bridgeid = $k; @@ -4672,7 +4641,7 @@ sub qemu_usb_hotplug { my $devicelist = vm_devices_list($vmid); if (!$devicelist->{xhci}) { - my $pciaddr = print_pci_addr("xhci", undef, $arch, $machine_type); + my $pciaddr = print_pci_addr("xhci", undef, $arch); qemu_deviceadd($vmid, PVE::QemuServer::USB::print_qemu_xhci_controller($pciaddr)); } diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm index 9b343115..2dbc87a7 100644 --- a/src/PVE/QemuServer/PCI.pm +++ b/src/PVE/QemuServer/PCI.pm @@ -289,14 +289,14 @@ my $get_addr_mapping_from_id = sub { }; sub print_pci_addr { - my ($id, $bridges, $arch, $machine) = @_; + my ($id, $bridges, $arch) = @_; my $res = ''; # using same bus slots on all HW, so we need to check special cases here: my $busname = 'pci'; - if ($arch eq 'aarch64' && $machine =~ /^virt/) { - die "aarch64/virt cannot use IDE devices\n" if $id =~ /^ide/; + if ($arch eq 'aarch64') { + die "aarch64 cannot use IDE devices\n" if $id =~ /^ide/; $busname = 'pcie'; } @@ -645,7 +645,7 @@ sub choose_hostpci_devices { } sub print_hostpci_devices { - my ($vmid, $conf, $devices, $vga, $winversion, $bridges, $arch, $machine_type, $bootorder) = @_; + my ($vmid, $conf, $devices, $vga, $winversion, $bridges, $arch, $bootorder) = @_; my $kvm_off = 0; my $gpu_passthrough = 0; @@ -676,7 +676,7 @@ sub print_hostpci_devices { } } else { my $pci_name = $d->{'legacy-igd'} ? 'legacy-igd' : $id; - $pciaddr = print_pci_addr($pci_name, $bridges, $arch, $machine_type); + $pciaddr = print_pci_addr($pci_name, $bridges, $arch); } my $num_devices = scalar($d->{ids}->@*); diff --git a/src/PVE/QemuServer/RNG.pm b/src/PVE/QemuServer/RNG.pm index 6e0949be..eb477a5d 100644 --- a/src/PVE/QemuServer/RNG.pm +++ b/src/PVE/QemuServer/RNG.pm @@ -87,7 +87,7 @@ sub check_rng_source { } sub print_rng_device_commandline { - my ($id, $rng, $bridges, $arch, $machine) = @_; + my ($id, $rng, $bridges, $arch) = @_; die "no rng device specified\n" if !$rng; @@ -98,7 +98,7 @@ sub print_rng_device_commandline { $limiter_str = ",max-bytes=$max_bytes,period=$period"; } - my $rng_addr = print_pci_addr($id, $bridges, $arch, $machine); + my $rng_addr = print_pci_addr($id, $bridges, $arch); return "virtio-rng-pci,rng=$id$limiter_str$rng_addr"; } diff --git a/src/PVE/QemuServer/USB.pm b/src/PVE/QemuServer/USB.pm index 5b3d1c5d..c9408a42 100644 --- a/src/PVE/QemuServer/USB.pm +++ b/src/PVE/QemuServer/USB.pm @@ -120,7 +120,7 @@ my sub assert_usb_index_is_useable { } sub get_usb_controllers { - my ($conf, $bridges, $arch, $machine, $machine_version) = @_; + my ($conf, $bridges, $arch, $machine_version) = @_; my $devices = []; my $pciaddr = ""; @@ -134,10 +134,10 @@ sub get_usb_controllers { my $is_q35 = PVE::QemuServer::Machine::machine_type_is_q35($conf); if ($arch eq 'aarch64') { - $pciaddr = print_pci_addr('ehci', $bridges, $arch, $machine); + $pciaddr = print_pci_addr('ehci', $bridges, $arch); push @$devices, '-device', "usb-ehci,id=ehci$pciaddr"; } elsif (!$is_q35) { - $pciaddr = print_pci_addr("piix3", $bridges, $arch, $machine); + $pciaddr = print_pci_addr("piix3", $bridges, $arch); push @$devices, '-device', "piix3-usb-uhci,id=uhci$pciaddr.0x2"; } @@ -157,7 +157,7 @@ sub get_usb_controllers { push @$devices, '-readconfig', '/usr/share/qemu-server/pve-usb.cfg'; } - $pciaddr = print_pci_addr("xhci", $bridges, $arch, $machine); + $pciaddr = print_pci_addr("xhci", $bridges, $arch); if ($use_qemu_xhci && $any_usb) { push @$devices, '-device', print_qemu_xhci_controller($pciaddr); } elsif ($use_usb3) { -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel