From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
by lore.proxmox.com (Postfix) with ESMTPS id 28BD71FF183
for <inbox@lore.proxmox.com>; Wed, 18 Jun 2025 17:20:19 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 9FF6D169BC;
Wed, 18 Jun 2025 17:20:46 +0200 (CEST)
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Wed, 18 Jun 2025 17:20:08 +0200
Message-Id: <20250618152009.120524-3-f.ebner@proxmox.com>
X-Mailer: git-send-email 2.39.5
In-Reply-To: <20250618152009.120524-1-f.ebner@proxmox.com>
References: <20250618152009.120524-1-f.ebner@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.030 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
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: [pve-devel] [PATCH qemu-server 2/3] pci: code cleanup: remove
superfluous machine type parameter 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 <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
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 <f.ebner@proxmox.com>
---
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 23fde3f8..22b6a11e 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1396,7 +1396,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') {
@@ -1657,24 +1657,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
@@ -1777,7 +1767,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') {
@@ -1829,7 +1819,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') {
@@ -3761,9 +3751,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);
@@ -3787,15 +3776,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
@@ -3839,7 +3820,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;
}
@@ -3884,9 +3865,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
@@ -3956,7 +3935,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') {
@@ -3967,7 +3946,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;
}
@@ -3983,14 +3962,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 {
@@ -4005,7 +3977,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') {
@@ -4043,7 +4015,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;
@@ -4051,7 +4023,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};
@@ -4098,8 +4070,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;
@@ -4134,7 +4105,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;
@@ -4189,7 +4160,6 @@ sub config_to_command {
$bridges,
$use_old_bios_files,
$arch,
- $machine_type,
$machine_version,
);
@@ -4203,7 +4173,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;
@@ -4232,7 +4202,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
@@ -4397,7 +4367,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";
@@ -4441,7 +4411,6 @@ sub vm_deviceplug {
undef,
$use_old_bios_files,
$arch,
- $machine_type,
$machine_version,
);
qemu_deviceadd($vmid, $netdevicefull);
@@ -4456,7 +4425,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);
@@ -4664,7 +4633,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;
@@ -4727,7 +4696,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 66bd1534..89931465 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -284,14 +284,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';
}
@@ -640,7 +640,7 @@ my 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;
@@ -671,7 +671,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