* [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev
@ 2025-06-18 13:01 Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH common v2 01/32] schema: parse property string: support skipping keys Fiona Ebner
` (32 more replies)
0 siblings, 33 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
Changes in v2 (I hope I remembered everything, because of the
indentation changes, I couldn't use my usual git range-diff O:)):
* Rebase + tidy all patches (this is not noted on individual patches).
* Indicate that version guards are for the switch to -blockdev via
comments.
* Fix deleting throttle group, ID string was wrongly quoted.
* For backup restore, only skip dropped keys when parsing drive
property string, rather than allowing all additional properties.
* Order Perl file names in Makefile.
* Add patch to drop superfluous fallack for migration type.
* For templates, keep using minimized config for timeout calculation.
* Also support unix socket NBD paths.
* Coerce some more strings by quoting for JSON, just to be sure.
* Introduce json_bool() helper.
The following (i.e. patches 19/32 to 24/32) are orthogonal:
* Add patches for PCI module improvements.
* Never reserve PCI usage for 'qm showcmd', add dedicated test for
checking that reservation is respected.
This is a preparatory series for the switch to -blockdev with Proxmox
VE 9, QEMU machine version 10.0, based in part on Alexandre's series
here [0].
While the last patch actually does the switch, many operations are not
yet supported. It is included to show what changes I made there. It
should not yet be applied and supporting everything is the goal for a
following series based on the rest of Alexandre's patches. It was time
to send this however, as it's already gotten large enough (but most
patches are quite small, so don't worry reviewers ;)).
There are mostly cleanups, moving and defining helpers, as well as
doing non-intrusive preparation for the switch with machine version
10.0. Quite a large chunk of the series is for having
activate_volumes() be called before config_to_command() with proper
cleanup handling.
A breaking change is the removal of the gone-since-QEMU-3.1 drive
geometry options. It was not possible to even start or backup a VM
with those options set since then. Still, restore of old backups with
such an option is made explicitly supported.
NOTE: Dependency bump qemu-server -> libpve-common-perl needed.
[0]: https://lore.proxmox.com/pve-devel/mailman.347.1749728041.395.pve-devel@lists.proxmox.com/T/
common:
Fiona Ebner (2):
schema: parse property string: support skipping keys
json schema: add helper to convert to JSON boolean
src/PVE/JSONSchema.pm | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
qemu-server:
Alexandre Derumier via pve-devel (1):
vm devices list: prepare querying block device names for -blockdev
Fiona Ebner (29):
buildsys: order Perl source files in QemuServer/Makefile
drive: code cleanup: drop unused $vmid parameter from
get_path_and_format()
cfg2cmd: require at least QEMU binary version 6.0
drive: parse: use hash argument for optional parameters
drive: parse: properly handle dropped properties for restore
drive: remove geometry options gone since QEMU 3.1
clone disk: io uring check: fix call to determine cache direct
drive: move storage_allows_io_uring_default() and
drive_uses_cache_direct() helpers to drive module
drive: introduce aio_cmdline_option() helper
drive: introduce detect_zeroes_cmdline_option() helper
vm start: assert that migration type is set for 'tcp' migration
introduce StateFile module for state file related helpers
vm start: move state file handling to dedicated module
vm start: move config_to_command() call further down
vm start/commandline: also clean up pci reservation when
config_to_command() fails
vm start/commandline: activate volumes before config_to_command()
pci: add missing includes
test: add tests for PCI reservations
cfg2cmd: print vga: fix call to print_pcie_addr()
pci: code cleanup: remove superfluous machine type paramater from
print_pci_addr
cfg2cmd: collect optional parameters as a hash
qm: showcmd: never reserve PCI devices
print drive device: explicitly set write-cache starting with machine
version 10.0
print drive device: set {r,w}error front-end properties starting with
machine version 10.0
print drive device: don't reference any drive for 'none' starting with
machine version 10.0
drive: create a throttle group for each drive starting with machine
version 10.0
blockdev: add helpers to generate blockdev commandline
blockdev: add support for NBD paths
command line: switch to blockdev starting with machine version 10.0
src/PVE/API2/Qemu.pm | 15 +-
src/PVE/QemuServer.pm | 605 +++++++++---------
src/PVE/QemuServer/Blockdev.pm | 234 +++++++
src/PVE/QemuServer/Drive.pm | 108 +++-
src/PVE/QemuServer/Makefile | 26 +-
src/PVE/QemuServer/PCI.pm | 30 +-
src/PVE/QemuServer/RNG.pm | 4 +-
src/PVE/QemuServer/StateFile.pm | 79 +++
src/PVE/QemuServer/USB.pm | 8 +-
src/test/Makefile | 5 +-
src/test/cfg2cmd/aio.conf.cmd | 16 +-
src/test/cfg2cmd/old-qemu.conf | 4 +-
.../q35-linux-hostpci-mapping.conf.cmd | 2 +-
src/test/run_config2command_tests.pl | 24 +-
src/test/run_pci_reservation_tests.pl | 175 +++++
15 files changed, 950 insertions(+), 385 deletions(-)
create mode 100644 src/PVE/QemuServer/Blockdev.pm
create mode 100644 src/PVE/QemuServer/StateFile.pm
create mode 100755 src/test/run_pci_reservation_tests.pl
Summary over all repositories:
16 files changed, 960 insertions(+), 386 deletions(-)
--
Generated by git-murpp 0.5.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH common v2 01/32] schema: parse property string: support skipping keys
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-20 11:00 ` Fabian Grünbichler
2025-06-18 13:01 ` [pve-devel] [PATCH common v2 02/32] json schema: add helper to convert to JSON boolean Fiona Ebner
` (31 subsequent siblings)
32 siblings, 1 reply; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
In certain situations like restoring a backup, it can be useful to
skip certain properties. This allows to drop outdated properties from
the schema while still being able to parse property strings that
contain them, but without allowing all additional properties.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
New in v2.
src/PVE/JSONSchema.pm | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index 8c90986..e617fc1 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -1033,7 +1033,7 @@ sub parse_boolean {
}
sub parse_property_string {
- my ($format, $data, $path, $additional_properties) = @_;
+ my ($format, $data, $path, $additional_properties, $options) = @_;
# In property strings we default to not allowing additional properties
$additional_properties = 0 if !defined($additional_properties);
@@ -1057,6 +1057,7 @@ sub parse_property_string {
}
my $default_key;
+ my $skip = $options->{skip} ? { map { $_ => 1 } $options->{skip}->@* } : {};
my $res = {};
foreach my $part (split(/,/, $data)) {
@@ -1064,6 +1065,7 @@ sub parse_property_string {
if ($part =~ /^([^=]+)=(.+)$/) {
my ($k, $v) = ($1, $2);
+ next if $skip->{$k};
die "duplicate key in comma-separated list property: $k\n" if defined($res->{$k});
my $schema = $format->{$k};
if (my $alias = $schema->{alias}) {
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH common v2 02/32] json schema: add helper to convert to JSON boolean
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH common v2 01/32] schema: parse property string: support skipping keys Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 03/32] buildsys: order Perl source files in QemuServer/Makefile Fiona Ebner
` (30 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
New in v2.
src/PVE/JSONSchema.pm | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index e617fc1..43e0ca0 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -21,6 +21,7 @@ our @EXPORT_OK = qw(
get_standard_option
parse_property_string
print_property_string
+ json_bool
);
our $CONFIGID_RE = qr/[a-z][a-z0-9_-]+/i;
@@ -2513,4 +2514,10 @@ sub schema_get_type_text {
return "<$type>";
}
+sub json_bool {
+ my ($value) = @_;
+
+ return $value ? JSON::true : JSON::false;
+}
+
1;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 03/32] buildsys: order Perl source files in QemuServer/Makefile
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH common v2 01/32] schema: parse property string: support skipping keys Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH common v2 02/32] json schema: add helper to convert to JSON boolean Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 04/32] drive: code cleanup: drop unused $vmid parameter from get_path_and_format() Fiona Ebner
` (29 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
New in v2.
src/PVE/QemuServer/Makefile | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/PVE/QemuServer/Makefile b/src/PVE/QemuServer/Makefile
index 57ebfae5..9812c404 100644
--- a/src/PVE/QemuServer/Makefile
+++ b/src/PVE/QemuServer/Makefile
@@ -2,21 +2,21 @@ DESTDIR=
PREFIX=/usr
PERLDIR=$(PREFIX)/share/perl5
-SOURCES=PCI.pm \
+SOURCES=Agent.pm \
+ CGroup.pm \
+ Cloudinit.pm \
+ CPUConfig.pm \
+ Drive.pm \
+ Helpers.pm \
+ ImportDisk.pm \
+ Machine.pm \
+ Memory.pm \
+ MetaInfo.pm \
+ Monitor.pm \
+ PCI.pm \
+ QMPHelpers.pm \
RNG.pm \
USB.pm \
- Memory.pm \
- ImportDisk.pm \
- Cloudinit.pm \
- Agent.pm \
- Helpers.pm \
- Monitor.pm \
- Machine.pm \
- MetaInfo.pm \
- CPUConfig.pm \
- CGroup.pm \
- Drive.pm \
- QMPHelpers.pm \
Virtiofs.pm
.PHONY: install
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 04/32] drive: code cleanup: drop unused $vmid parameter from get_path_and_format()
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (2 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 03/32] buildsys: order Perl source files in QemuServer/Makefile Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 05/32] cfg2cmd: require at least QEMU binary version 6.0 Fiona Ebner
` (28 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
The parameter is unused since commit f97b3da1 ("drive: code cleanup:
remove unused $vmid argument from get_iso_path() helper").
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
No changes in v2.
src/PVE/QemuServer.pm | 6 +++---
src/PVE/QemuServer/Drive.pm | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 8eecbd53..89dd7065 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1538,7 +1538,7 @@ sub print_drive_commandline_full {
my $vtype = $storeid ? (PVE::Storage::parse_volname($storecfg, $drive->{file}))[0] : undef;
my ($path, $format) =
- PVE::QemuServer::Drive::get_path_and_format($storecfg, $vmid, $drive, $live_restore_name);
+ PVE::QemuServer::Drive::get_path_and_format($storecfg, $drive, $live_restore_name);
my $is_rbd = $path =~ m/^rbd:/;
@@ -5775,7 +5775,7 @@ sub vmconfig_update_disk {
}
} else {
my ($path, $format) =
- PVE::QemuServer::Drive::get_path_and_format($storecfg, $vmid, $drive);
+ PVE::QemuServer::Drive::get_path_and_format($storecfg, $drive);
# force eject if locked
mon_cmd($vmid, "eject", force => JSON::true, id => "$opt");
@@ -5828,7 +5828,7 @@ sub vmconfig_update_cloudinit_drive {
if ($running) {
my ($path, $format) =
- PVE::QemuServer::Drive::get_path_and_format($storecfg, $vmid, $cloudinit_drive);
+ PVE::QemuServer::Drive::get_path_and_format($storecfg, $cloudinit_drive);
if ($path) {
mon_cmd($vmid, "eject", force => JSON::true, id => "$cloudinit_ds");
mon_cmd(
diff --git a/src/PVE/QemuServer/Drive.pm b/src/PVE/QemuServer/Drive.pm
index f6351a98..0947b9ed 100644
--- a/src/PVE/QemuServer/Drive.pm
+++ b/src/PVE/QemuServer/Drive.pm
@@ -107,7 +107,7 @@ sub get_iso_path {
# Returns the path that can be used on the QEMU commandline and in QMP commands as well as the
# checked format of the drive.
sub get_path_and_format {
- my ($storecfg, $vmid, $drive, $live_restore_name) = @_;
+ my ($storecfg, $drive, $live_restore_name) = @_;
my $path;
my $volid = $drive->{file};
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 05/32] cfg2cmd: require at least QEMU binary version 6.0
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (3 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 04/32] drive: code cleanup: drop unused $vmid parameter from get_path_and_format() Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 06/32] drive: parse: use hash argument for optional parameters Fiona Ebner
` (27 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
The minimum supported binary version for Proxmox VE 9 is QEMU 10.0, so
there still is more than enough room for eventual regression testing
with older binaries.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
No changes in v2.
src/PVE/QemuServer.pm | 21 +++++++--------------
src/test/cfg2cmd/old-qemu.conf | 4 ++--
2 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 89dd7065..3960203b 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1529,7 +1529,7 @@ my sub drive_uses_cache_direct {
}
sub print_drive_commandline_full {
- my ($storecfg, $vmid, $drive, $live_restore_name, $io_uring) = @_;
+ my ($storecfg, $vmid, $drive, $live_restore_name) = @_;
my $drive_id = PVE::QemuServer::Drive::get_drive_id($drive);
@@ -1594,7 +1594,7 @@ sub print_drive_commandline_full {
$opts .= ",cache=none" if !$drive->{cache} && $cache_direct;
if (!$drive->{aio}) {
- if ($io_uring && storage_allows_io_uring_default($scfg, $cache_direct)) {
+ if (storage_allows_io_uring_default($scfg, $cache_direct)) {
# io_uring supports all cache modes
$opts .= ",aio=io_uring";
} else {
@@ -3618,9 +3618,9 @@ sub config_to_command {
my $kvm_binary = PVE::QemuServer::Helpers::get_command_for_arch($arch);
my $kvmver = kvm_user_version($kvm_binary);
- if (!$kvmver || $kvmver !~ m/^(\d+)\.(\d+)/ || $1 < 5) {
+ if (!$kvmver || $kvmver !~ m/^(\d+)\.(\d+)/ || $1 < 6) {
$kvmver //= "undefined";
- die "Detected old QEMU binary ('$kvmver', at least 5.0 is required)\n";
+ die "Detected old QEMU binary ('$kvmver', at least 6.0 is required)\n";
}
my $machine_type = PVE::QemuServer::Machine::get_vm_machine($conf, $forcemachine, $arch);
@@ -4147,13 +4147,8 @@ sub config_to_command {
push @$devices, '-blockdev', $live_restore->{blockdev};
}
- my $drive_cmd = print_drive_commandline_full(
- $storecfg,
- $vmid,
- $drive,
- $live_blockdev_name,
- min_version($kvmver, 6, 0),
- );
+ my $drive_cmd =
+ print_drive_commandline_full($storecfg, $vmid, $drive, $live_blockdev_name);
# extra protection for templates, but SATA and IDE don't support it..
$drive_cmd .= ',readonly=on' if drive_is_read_only($conf, $drive);
@@ -4557,9 +4552,7 @@ sub qemu_iothread_del {
sub qemu_driveadd {
my ($storecfg, $vmid, $device) = @_;
- my $kvmver = get_running_qemu_version($vmid);
- my $io_uring = min_version($kvmver, 6, 0);
- my $drive = print_drive_commandline_full($storecfg, $vmid, $device, undef, $io_uring);
+ my $drive = print_drive_commandline_full($storecfg, $vmid, $device, undef);
$drive =~ s/\\/\\\\/g;
my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_add auto \"$drive\"", 60);
diff --git a/src/test/cfg2cmd/old-qemu.conf b/src/test/cfg2cmd/old-qemu.conf
index e8129cf6..e560cac7 100644
--- a/src/test/cfg2cmd/old-qemu.conf
+++ b/src/test/cfg2cmd/old-qemu.conf
@@ -1,4 +1,4 @@
# TEST: Test QEMU version detection and expect fail on old version
-# QEMU_VERSION: 4.0.1
-# EXPECT_ERROR: Detected old QEMU binary ('4.0.1', at least 5.0 is required)
+# QEMU_VERSION: 5.0.1
+# EXPECT_ERROR: Detected old QEMU binary ('5.0.1', at least 6.0 is required)
smbios1: uuid=7b10d7af-b932-4c66-b2c3-3996152ec465
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 06/32] drive: parse: use hash argument for optional parameters
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (4 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 05/32] cfg2cmd: require at least QEMU binary version 6.0 Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 07/32] drive: parse: properly handle dropped properties for restore Fiona Ebner
` (26 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
In preparation to add a new one.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
No changes in v2.
src/PVE/API2/Qemu.pm | 15 ++++++++++-----
src/PVE/QemuServer/Drive.pm | 6 ++++--
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index ce6f362d..9b07db25 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -89,7 +89,8 @@ my $foreach_volume_with_alloc = sub {
for my $opt (sort keys $param->%*) {
next if !PVE::QemuServer::is_valid_drivename($opt);
- my $drive = PVE::QemuServer::Drive::parse_drive($opt, $param->{$opt}, 1);
+ my $drive =
+ PVE::QemuServer::Drive::parse_drive($opt, $param->{$opt}, { 'with-alloc' => 1 });
next if !$drive;
$func->($opt, $drive);
@@ -102,7 +103,7 @@ my $check_drive_param = sub {
for my $opt (sort keys $param->%*) {
next if !PVE::QemuServer::is_valid_drivename($opt);
- my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt}, 1);
+ my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt}, { 'with-alloc' => 1 });
raise_param_exc({ $opt => "unable to parse drive options" }) if !$drive;
if ($drive->{'import-from'}) {
@@ -969,7 +970,7 @@ my $check_vm_modify_config_perm = sub {
sub assert_scsi_feature_compatibility {
my ($opt, $conf, $storecfg, $drive_attributes) = @_;
- my $drive = PVE::QemuServer::Drive::parse_drive($opt, $drive_attributes, 1);
+ my $drive = PVE::QemuServer::Drive::parse_drive($opt, $drive_attributes, { 'with-alloc' => 1 });
my $machine_type = PVE::QemuServer::Machine::get_vm_machine($conf, undef, $conf->{arch});
my $machine_version = PVE::QemuServer::Machine::extract_version(
@@ -2144,7 +2145,7 @@ my $update_vm_api = sub {
my $check_drive_perms = sub {
my ($opt, $val) = @_;
- my $drive = PVE::QemuServer::parse_drive($opt, $val, 1);
+ my $drive = PVE::QemuServer::parse_drive($opt, $val, { 'with-alloc' => 1 });
if (PVE::QemuServer::drive_is_cloudinit($drive)) {
$rpcenv->check_vm_perm(
$authuser,
@@ -2311,7 +2312,11 @@ my $update_vm_api = sub {
# default legacy boot order implies all cdroms anyway
if (@bootorder) {
# append new CD drives to bootorder to mark them bootable
- my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt}, 1);
+ my $drive = PVE::QemuServer::parse_drive(
+ $opt,
+ $param->{$opt},
+ { 'with-alloc' => 1 },
+ );
if (
PVE::QemuServer::drive_is_cdrom($drive, 1)
&& !grep(/^$opt$/, @bootorder)
diff --git a/src/PVE/QemuServer/Drive.pm b/src/PVE/QemuServer/Drive.pm
index 0947b9ed..b9ceb130 100644
--- a/src/PVE/QemuServer/Drive.pm
+++ b/src/PVE/QemuServer/Drive.pm
@@ -770,7 +770,9 @@ sub drive_is_read_only {
# [,iothread=on][,serial=serial][,model=model]
sub parse_drive {
- my ($key, $data, $with_alloc) = @_;
+ my ($key, $data, $parse_opts) = @_;
+
+ $parse_opts //= {};
my ($interface, $index);
@@ -781,7 +783,7 @@ sub parse_drive {
return;
}
- my $desc_hash = $with_alloc ? $drivedesc_hash_with_alloc : $drivedesc_hash;
+ my $desc_hash = $parse_opts->{'with-alloc'} ? $drivedesc_hash_with_alloc : $drivedesc_hash;
if (!defined($desc_hash->{$key})) {
warn "invalid drive key: $key\n";
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 07/32] drive: parse: properly handle dropped properties for restore
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (5 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 06/32] drive: parse: use hash argument for optional parameters Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 08/32] drive: remove geometry options gone since QEMU 3.1 Fiona Ebner
` (25 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
Restoring old backups should still work when properties are dropped
from the schema. In particular, it is necessary to skip such
properties when parsing the configuration during restore.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Dependency bump for libpve-common-perl needed.
Changes in v2:
* Different approach, skip dropped keys, rather than allowing all
additional properties.
src/PVE/QemuServer.pm | 4 ++--
src/PVE/QemuServer/Drive.pm | 7 ++++++-
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 3960203b..cfeb8949 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -7151,7 +7151,7 @@ my $parse_backup_hints = sub {
$virtdev_hash->{$virtdev} = $devinfo->{$devname};
} elsif ($line =~ m/^((?:ide|sata|scsi)\d+):\s*(.*)\s*$/) {
my $virtdev = $1;
- my $drive = parse_drive($virtdev, $2);
+ my $drive = parse_drive($virtdev, $2, { 'for-restore' => 1 });
if (drive_is_cloudinit($drive)) {
my ($storeid, $volname) = PVE::Storage::parse_volume_id($drive->{file});
@@ -7257,7 +7257,7 @@ sub restore_update_config_line {
} elsif ($line =~ m/^((ide|scsi|virtio|sata|efidisk|tpmstate)\d+):\s*(\S+)\s*$/) {
my $virtdev = $1;
my $value = $3;
- my $di = parse_drive($virtdev, $value);
+ my $di = parse_drive($virtdev, $value, { 'for-restore' => 1 });
if (defined($di->{backup}) && !$di->{backup}) {
$res .= "#$line";
} elsif ($map->{$virtdev}) {
diff --git a/src/PVE/QemuServer/Drive.pm b/src/PVE/QemuServer/Drive.pm
index b9ceb130..0e99257b 100644
--- a/src/PVE/QemuServer/Drive.pm
+++ b/src/PVE/QemuServer/Drive.pm
@@ -26,6 +26,8 @@ our @EXPORT_OK = qw(
print_drive
);
+my $DROPPED_PROPERTIES = [];
+
our $QEMU_FORMAT_RE = qr/raw|qcow|qcow2|qed|vmdk|cloop/;
PVE::JSONSchema::register_standard_option(
@@ -791,7 +793,10 @@ sub parse_drive {
}
my $desc = $desc_hash->{$key}->{format};
- my $res = eval { PVE::JSONSchema::parse_property_string($desc, $data) };
+ my $res = eval {
+ my $pps_opts = $parse_opts->{'for-restore'} ? { skip => $DROPPED_PROPERTIES } : {};
+ PVE::JSONSchema::parse_property_string($desc, $data, undef, undef, $pps_opts);
+ };
return if !$res;
$res->{interface} = $interface;
$res->{index} = $index;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 08/32] drive: remove geometry options gone since QEMU 3.1
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (6 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 07/32] drive: parse: properly handle dropped properties for restore Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-20 11:03 ` Fabian Grünbichler
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 09/32] clone disk: io uring check: fix call to determine cache direct Fiona Ebner
` (24 subsequent siblings)
32 siblings, 1 reply; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
It was not possible to start a QEMU instance with these options set
since QEMU version 3.1, QEMU commit b24ec3c462 ("block: Remove
deprecated -drive geometry options") and thus also not to take a
backup. It is still possible to restore an old backup with these
options set.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Changes in v2:
* Different approach, skip dropped keys, rather than allowing all
additional properties.
src/PVE/QemuServer.pm | 2 +-
src/PVE/QemuServer/Drive.pm | 28 +++-------------------------
2 files changed, 4 insertions(+), 26 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index cfeb8949..95a84c56 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1543,7 +1543,7 @@ sub print_drive_commandline_full {
my $is_rbd = $path =~ m/^rbd:/;
my $opts = '';
- my @qemu_drive_options = qw(heads secs cyls trans media cache rerror werror aio discard);
+ my @qemu_drive_options = qw(media cache rerror werror aio discard);
foreach my $o (@qemu_drive_options) {
$opts .= ",$o=$drive->{$o}" if defined($drive->{$o});
}
diff --git a/src/PVE/QemuServer/Drive.pm b/src/PVE/QemuServer/Drive.pm
index 0e99257b..96bb12c4 100644
--- a/src/PVE/QemuServer/Drive.pm
+++ b/src/PVE/QemuServer/Drive.pm
@@ -26,7 +26,7 @@ our @EXPORT_OK = qw(
print_drive
);
-my $DROPPED_PROPERTIES = [];
+my $DROPPED_PROPERTIES = ['cyls', 'heads', 'secs', 'trans'];
our $QEMU_FORMAT_RE = qr/raw|qcow|qcow2|qed|vmdk|cloop/;
@@ -176,27 +176,6 @@ my %drivedesc_base = (
default => 'disk',
optional => 1,
},
- cyls => {
- type => 'integer',
- description => "Force the drive's physical geometry to have a specific cylinder count.",
- optional => 1,
- },
- heads => {
- type => 'integer',
- description => "Force the drive's physical geometry to have a specific head count.",
- optional => 1,
- },
- secs => {
- type => 'integer',
- description => "Force the drive's physical geometry to have a specific sector count.",
- optional => 1,
- },
- trans => {
- type => 'string',
- enum => [qw(none lba auto)],
- description => "Force disk geometry bios translation mode.",
- optional => 1,
- },
snapshot => {
type => 'boolean',
description => "Controls qemu's snapshot mode feature."
@@ -765,7 +744,7 @@ sub drive_is_read_only {
return $drive->{interface} ne 'sata' && $drive->{interface} ne 'ide';
}
-# ideX = [volume=]volume-id[,media=d][,cyls=c,heads=h,secs=s[,trans=t]]
+# ideX = [volume=]volume-id[,media=d]
# [,snapshot=on|off][,cache=on|off][,format=f][,backup=yes|no]
# [,rerror=ignore|report|stop][,werror=enospc|ignore|report|stop]
# [,aio=native|threads][,discard=ignore|on][,detect_zeroes=on|off]
@@ -843,8 +822,7 @@ sub parse_drive {
return if $res->{iops_wr} && $res->{iops};
if ($res->{media} && ($res->{media} eq 'cdrom')) {
- return if $res->{snapshot} || $res->{trans} || $res->{format};
- return if $res->{heads} || $res->{secs} || $res->{cyls};
+ return if $res->{snapshot} || $res->{format};
return if $res->{interface} eq 'virtio';
}
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 09/32] clone disk: io uring check: fix call to determine cache direct
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (7 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 08/32] drive: remove geometry options gone since QEMU 3.1 Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 10/32] drive: move storage_allows_io_uring_default() and drive_uses_cache_direct() helpers to drive module Fiona Ebner
` (23 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
The $scfg parameter for drive_uses_cache_direct() is only relevant for
BTRFS and the io_uring default does not depend on the cache setting
for BTRFS, so it currently doesn't matter if the parameter is passed
along or not here. Still, fix it for the future.
Fixes: 8fbae1dc ("fix #4525: clone disk: disallow mirror if it might cause problems with io_uring")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
No changes in v2.
src/PVE/QemuServer.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 95a84c56..76fc121c 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -8864,7 +8864,7 @@ my sub clone_disk_check_io_uring {
my $src_scfg = PVE::Storage::storage_config($storecfg, $src_storeid);
my $dst_scfg = PVE::Storage::storage_config($storecfg, $dst_storeid);
- my $cache_direct = drive_uses_cache_direct($src_drive);
+ my $cache_direct = drive_uses_cache_direct($src_drive, $src_scfg);
my $src_uses_io_uring;
if ($src_drive->{aio}) {
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 10/32] drive: move storage_allows_io_uring_default() and drive_uses_cache_direct() helpers to drive module
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (8 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 09/32] clone disk: io uring check: fix call to determine cache direct Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 11/32] drive: introduce aio_cmdline_option() helper Fiona Ebner
` (22 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
Suggested-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
No changes in v2.
src/PVE/QemuServer.pm | 47 ++++++++++---------------------------
src/PVE/QemuServer/Drive.pm | 33 ++++++++++++++++++++++++++
2 files changed, 45 insertions(+), 35 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 76fc121c..a3c37707 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -56,8 +56,16 @@ use PVE::QemuServer::Cloudinit;
use PVE::QemuServer::CGroup;
use PVE::QemuServer::CPUConfig
qw(print_cpu_device get_cpu_options get_cpu_bitness is_native_arch get_amd_sev_object get_amd_sev_type);
-use PVE::QemuServer::Drive
- qw(is_valid_drivename checked_volume_format drive_is_cloudinit drive_is_cdrom drive_is_read_only parse_drive print_drive);
+use PVE::QemuServer::Drive qw(
+ is_valid_drivename
+ checked_volume_format
+ drive_is_cloudinit
+ drive_is_cdrom
+ drive_is_read_only
+ parse_drive
+ print_drive
+ storage_allows_io_uring_default
+);
use PVE::QemuServer::Machine;
use PVE::QemuServer::Memory qw(get_current_memory);
use PVE::QemuServer::MetaInfo;
@@ -1497,37 +1505,6 @@ sub get_initiator_name {
return $initiator;
}
-my sub storage_allows_io_uring_default {
- my ($scfg, $cache_direct) = @_;
-
- # io_uring with cache mode writeback or writethrough on krbd will hang...
- return if $scfg && $scfg->{type} eq 'rbd' && $scfg->{krbd} && !$cache_direct;
-
- # io_uring with cache mode writeback or writethrough on LVM will hang, without cache only
- # sometimes, just plain disable...
- return if $scfg && $scfg->{type} eq 'lvm';
-
- # io_uring causes problems when used with CIFS since kernel 5.15
- # Some discussion: https://www.spinics.net/lists/linux-cifs/msg26734.html
- return if $scfg && $scfg->{type} eq 'cifs';
-
- return 1;
-}
-
-my sub drive_uses_cache_direct {
- my ($drive, $scfg) = @_;
-
- my $cache_direct = 0;
-
- if (my $cache = $drive->{cache}) {
- $cache_direct = $cache =~ /^(?:off|none|directsync)$/;
- } elsif (!drive_is_cdrom($drive) && !($scfg && $scfg->{type} eq 'btrfs' && !$scfg->{nocow})) {
- $cache_direct = 1;
- }
-
- return $cache_direct;
-}
-
sub print_drive_commandline_full {
my ($storecfg, $vmid, $drive, $live_restore_name) = @_;
@@ -1589,7 +1566,7 @@ sub print_drive_commandline_full {
$opts .= ",format=$format";
}
- my $cache_direct = drive_uses_cache_direct($drive, $scfg);
+ my $cache_direct = PVE::QemuServer::Drive::drive_uses_cache_direct($drive, $scfg);
$opts .= ",cache=none" if !$drive->{cache} && $cache_direct;
@@ -8864,7 +8841,7 @@ my sub clone_disk_check_io_uring {
my $src_scfg = PVE::Storage::storage_config($storecfg, $src_storeid);
my $dst_scfg = PVE::Storage::storage_config($storecfg, $dst_storeid);
- my $cache_direct = drive_uses_cache_direct($src_drive, $src_scfg);
+ my $cache_direct = PVE::QemuServer::Drive::drive_uses_cache_direct($src_drive, $src_scfg);
my $src_uses_io_uring;
if ($src_drive->{aio}) {
diff --git a/src/PVE/QemuServer/Drive.pm b/src/PVE/QemuServer/Drive.pm
index 96bb12c4..377cf54e 100644
--- a/src/PVE/QemuServer/Drive.pm
+++ b/src/PVE/QemuServer/Drive.pm
@@ -24,6 +24,7 @@ our @EXPORT_OK = qw(
get_scsi_devicetype
parse_drive
print_drive
+ storage_allows_io_uring_default
);
my $DROPPED_PROPERTIES = ['cyls', 'heads', 'secs', 'trans'];
@@ -1060,4 +1061,36 @@ sub get_scsi_device_type {
return $devicetype;
}
+
+sub storage_allows_io_uring_default {
+ my ($scfg, $cache_direct) = @_;
+
+ # io_uring with cache mode writeback or writethrough on krbd will hang...
+ return if $scfg && $scfg->{type} eq 'rbd' && $scfg->{krbd} && !$cache_direct;
+
+ # io_uring with cache mode writeback or writethrough on LVM will hang, without cache only
+ # sometimes, just plain disable...
+ return if $scfg && $scfg->{type} eq 'lvm';
+
+ # io_uring causes problems when used with CIFS since kernel 5.15
+ # Some discussion: https://www.spinics.net/lists/linux-cifs/msg26734.html
+ return if $scfg && $scfg->{type} eq 'cifs';
+
+ return 1;
+}
+
+sub drive_uses_cache_direct {
+ my ($drive, $scfg) = @_;
+
+ my $cache_direct = 0;
+
+ if (my $cache = $drive->{cache}) {
+ $cache_direct = $cache =~ /^(?:off|none|directsync)$/;
+ } elsif (!drive_is_cdrom($drive) && !($scfg && $scfg->{type} eq 'btrfs' && !$scfg->{nocow})) {
+ $cache_direct = 1;
+ }
+
+ return $cache_direct;
+}
+
1;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 11/32] drive: introduce aio_cmdline_option() helper
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (9 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 10/32] drive: move storage_allows_io_uring_default() and drive_uses_cache_direct() helpers to drive module Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 12/32] drive: introduce detect_zeroes_cmdline_option() helper Fiona Ebner
` (21 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
No changes in v2.
src/PVE/QemuServer.pm | 17 +++--------------
src/PVE/QemuServer/Drive.pm | 18 ++++++++++++++++++
src/test/cfg2cmd/aio.conf.cmd | 16 ++++++++--------
3 files changed, 29 insertions(+), 22 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index a3c37707..7bcc36db 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1520,7 +1520,7 @@ sub print_drive_commandline_full {
my $is_rbd = $path =~ m/^rbd:/;
my $opts = '';
- my @qemu_drive_options = qw(media cache rerror werror aio discard);
+ my @qemu_drive_options = qw(media cache rerror werror discard);
foreach my $o (@qemu_drive_options) {
$opts .= ",$o=$drive->{$o}" if defined($drive->{$o});
}
@@ -1570,19 +1570,8 @@ sub print_drive_commandline_full {
$opts .= ",cache=none" if !$drive->{cache} && $cache_direct;
- if (!$drive->{aio}) {
- if (storage_allows_io_uring_default($scfg, $cache_direct)) {
- # io_uring supports all cache modes
- $opts .= ",aio=io_uring";
- } else {
- # aio native works only with O_DIRECT
- if ($cache_direct) {
- $opts .= ",aio=native";
- } else {
- $opts .= ",aio=threads";
- }
- }
- }
+ my $aio = PVE::QemuServer::Drive::aio_cmdline_option($scfg, $drive, $cache_direct);
+ $opts .= ",aio=$aio";
die "$drive_id: explicit media parameter is required for iso images\n"
if !defined($drive->{media}) && defined($vtype) && $vtype eq 'iso';
diff --git a/src/PVE/QemuServer/Drive.pm b/src/PVE/QemuServer/Drive.pm
index 377cf54e..73678ae2 100644
--- a/src/PVE/QemuServer/Drive.pm
+++ b/src/PVE/QemuServer/Drive.pm
@@ -1093,4 +1093,22 @@ sub drive_uses_cache_direct {
return $cache_direct;
}
+sub aio_cmdline_option {
+ my ($scfg, $drive, $cache_direct) = @_;
+
+ return $drive->{aio} if $drive->{aio};
+
+ if (storage_allows_io_uring_default($scfg, $cache_direct)) {
+ # io_uring supports all cache modes
+ return 'io_uring';
+ } else {
+ # aio native works only with O_DIRECT
+ if ($cache_direct) {
+ return 'native';
+ } else {
+ return 'threads';
+ }
+ }
+}
+
1;
diff --git a/src/test/cfg2cmd/aio.conf.cmd b/src/test/cfg2cmd/aio.conf.cmd
index 851cb90b..9d29a34f 100644
--- a/src/test/cfg2cmd/aio.conf.cmd
+++ b/src/test/cfg2cmd/aio.conf.cmd
@@ -24,33 +24,33 @@
-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' \
-device 'lsi,id=scsihw0,bus=pci.0,addr=0x5' \
- -drive 'file=/var/lib/vz/images/8006/vm-8006-disk-0.raw,if=none,id=drive-scsi0,aio=threads,discard=on,format=raw,cache=none,detect-zeroes=unmap' \
+ -drive 'file=/var/lib/vz/images/8006/vm-8006-disk-0.raw,if=none,id=drive-scsi0,discard=on,format=raw,cache=none,aio=threads,detect-zeroes=unmap' \
-device 'scsi-hd,bus=scsihw0.0,scsi-id=0,drive=drive-scsi0,id=scsi0' \
- -drive 'file=/var/lib/vz/images/8006/vm-8006-disk-1.raw,if=none,id=drive-scsi1,aio=native,discard=on,format=raw,cache=none,detect-zeroes=unmap' \
+ -drive 'file=/var/lib/vz/images/8006/vm-8006-disk-1.raw,if=none,id=drive-scsi1,discard=on,format=raw,cache=none,aio=native,detect-zeroes=unmap' \
-device 'scsi-hd,bus=scsihw0.0,scsi-id=1,drive=drive-scsi1,id=scsi1' \
- -drive 'file=/var/lib/vz/images/8006/vm-8006-disk-2.raw,if=none,id=drive-scsi2,aio=io_uring,discard=on,format=raw,cache=none,detect-zeroes=unmap' \
+ -drive 'file=/var/lib/vz/images/8006/vm-8006-disk-2.raw,if=none,id=drive-scsi2,discard=on,format=raw,cache=none,aio=io_uring,detect-zeroes=unmap' \
-device 'scsi-hd,bus=scsihw0.0,scsi-id=2,drive=drive-scsi2,id=scsi2' \
-drive 'file=/var/lib/vz/images/8006/vm-8006-disk-3.raw,if=none,id=drive-scsi3,discard=on,format=raw,cache=none,aio=io_uring,detect-zeroes=unmap' \
-device 'scsi-hd,bus=scsihw0.0,scsi-id=3,drive=drive-scsi3,id=scsi3' \
-drive 'file=/mnt/pve/cifs-store/images/8006/vm-8006-disk-4.raw,if=none,id=drive-scsi4,discard=on,format=raw,cache=none,aio=native,detect-zeroes=unmap' \
-device 'scsi-hd,bus=scsihw0.0,scsi-id=4,drive=drive-scsi4,id=scsi4' \
- -drive 'file=/mnt/pve/cifs-store/images/8006/vm-8006-disk-5.raw,if=none,id=drive-scsi5,aio=io_uring,discard=on,format=raw,cache=none,detect-zeroes=unmap' \
+ -drive 'file=/mnt/pve/cifs-store/images/8006/vm-8006-disk-5.raw,if=none,id=drive-scsi5,discard=on,format=raw,cache=none,aio=io_uring,detect-zeroes=unmap' \
-device 'scsi-hd,bus=scsihw0.0,scsi-id=5,drive=drive-scsi5,id=scsi5' \
-drive 'file=/dev/rbd-pve/fc4181a6-56eb-4f68-b452-8ba1f381ca2a/cpool/vm-8006-disk-6,if=none,id=drive-scsi6,discard=on,format=raw,cache=none,aio=io_uring,detect-zeroes=unmap' \
-device 'scsi-hd,bus=scsihw0.0,scsi-id=6,drive=drive-scsi6,id=scsi6' \
-device 'lsi,id=scsihw1,bus=pci.0,addr=0x6' \
- -drive 'file=/dev/rbd-pve/fc4181a6-56eb-4f68-b452-8ba1f381ca2a/cpool/vm-8006-disk-7,if=none,id=drive-scsi7,aio=io_uring,discard=on,format=raw,cache=none,detect-zeroes=unmap' \
+ -drive 'file=/dev/rbd-pve/fc4181a6-56eb-4f68-b452-8ba1f381ca2a/cpool/vm-8006-disk-7,if=none,id=drive-scsi7,discard=on,format=raw,cache=none,aio=io_uring,detect-zeroes=unmap' \
-device 'scsi-hd,bus=scsihw1.0,scsi-id=0,drive=drive-scsi7,id=scsi7' \
-drive 'file=/dev/rbd-pve/fc4181a6-56eb-4f68-b452-8ba1f381ca2a/cpool/vm-8006-disk-8,if=none,id=drive-scsi8,cache=writeback,discard=on,format=raw,aio=threads,detect-zeroes=unmap' \
-device 'scsi-hd,bus=scsihw1.0,scsi-id=1,drive=drive-scsi8,id=scsi8' \
- -drive 'file=/dev/rbd-pve/fc4181a6-56eb-4f68-b452-8ba1f381ca2a/cpool/vm-8006-disk-9,if=none,id=drive-scsi9,cache=writeback,aio=io_uring,discard=on,format=raw,detect-zeroes=unmap' \
+ -drive 'file=/dev/rbd-pve/fc4181a6-56eb-4f68-b452-8ba1f381ca2a/cpool/vm-8006-disk-9,if=none,id=drive-scsi9,cache=writeback,discard=on,format=raw,aio=io_uring,detect-zeroes=unmap' \
-device 'scsi-hd,bus=scsihw1.0,scsi-id=2,drive=drive-scsi9,id=scsi9' \
-drive 'file=rbd:cpool/vm-8006-disk-8:mon_host=127.0.0.42;127.0.0.21;[\:\:1]:auth_supported=none,if=none,id=drive-scsi10,discard=on,format=raw,cache=none,aio=io_uring,detect-zeroes=unmap' \
-device 'scsi-hd,bus=scsihw1.0,scsi-id=3,drive=drive-scsi10,id=scsi10' \
- -drive 'file=rbd:cpool/vm-8006-disk-8:mon_host=127.0.0.42;127.0.0.21;[\:\:1]:auth_supported=none,if=none,id=drive-scsi11,aio=io_uring,discard=on,format=raw,cache=none,detect-zeroes=unmap' \
+ -drive 'file=rbd:cpool/vm-8006-disk-8:mon_host=127.0.0.42;127.0.0.21;[\:\:1]:auth_supported=none,if=none,id=drive-scsi11,discard=on,format=raw,cache=none,aio=io_uring,detect-zeroes=unmap' \
-device 'scsi-hd,bus=scsihw1.0,scsi-id=4,drive=drive-scsi11,id=scsi11' \
-drive 'file=/dev/veegee/vm-8006-disk-9,if=none,id=drive-scsi12,discard=on,format=raw,cache=none,aio=native,detect-zeroes=unmap' \
-device 'scsi-hd,bus=scsihw1.0,scsi-id=5,drive=drive-scsi12,id=scsi12' \
- -drive 'file=/dev/veegee/vm-8006-disk-9,if=none,id=drive-scsi13,aio=io_uring,discard=on,format=raw,cache=none,detect-zeroes=unmap' \
+ -drive 'file=/dev/veegee/vm-8006-disk-9,if=none,id=drive-scsi13,discard=on,format=raw,cache=none,aio=io_uring,detect-zeroes=unmap' \
-device 'scsi-hd,bus=scsihw1.0,scsi-id=6,drive=drive-scsi13,id=scsi13' \
-machine 'type=pc+pve1'
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 12/32] drive: introduce detect_zeroes_cmdline_option() helper
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (10 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 11/32] drive: introduce aio_cmdline_option() helper Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 13/32] vm start: assert that migration type is set for 'tcp' migration Fiona Ebner
` (20 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
No changes in v2.
src/PVE/QemuServer.pm | 10 +---------
src/PVE/QemuServer/Drive.pm | 16 ++++++++++++++++
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 7bcc36db..5ab3a8c3 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1577,15 +1577,7 @@ sub print_drive_commandline_full {
if !defined($drive->{media}) && defined($vtype) && $vtype eq 'iso';
if (!drive_is_cdrom($drive)) {
- my $detectzeroes;
- if (defined($drive->{detect_zeroes}) && !$drive->{detect_zeroes}) {
- $detectzeroes = 'off';
- } elsif ($drive->{discard}) {
- $detectzeroes = $drive->{discard} eq 'on' ? 'unmap' : 'on';
- } else {
- # This used to be our default with discard not being specified:
- $detectzeroes = 'on';
- }
+ my $detectzeroes = PVE::QemuServer::Drive::detect_zeroes_cmdline_option($drive);
# note: 'detect-zeroes' works per blockdev and we want it to persist
# after the alloc-track is removed, so put it on 'file' directly
diff --git a/src/PVE/QemuServer/Drive.pm b/src/PVE/QemuServer/Drive.pm
index 73678ae2..c436d1d5 100644
--- a/src/PVE/QemuServer/Drive.pm
+++ b/src/PVE/QemuServer/Drive.pm
@@ -1111,4 +1111,20 @@ sub aio_cmdline_option {
}
}
+# must not be called for CD-ROMs
+sub detect_zeroes_cmdline_option {
+ my ($drive) = @_;
+
+ die "cannot use detect-zeroes for CD-ROM\n" if drive_is_cdrom($drive);
+
+ if (defined($drive->{detect_zeroes}) && !$drive->{detect_zeroes}) {
+ return 'off';
+ } elsif ($drive->{discard}) {
+ return $drive->{discard} eq 'on' ? 'unmap' : 'on';
+ }
+
+ # This used to be our default with discard not being specified:
+ return 'on';
+}
+
1;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 13/32] vm start: assert that migration type is set for 'tcp' migration
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (11 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 12/32] drive: introduce detect_zeroes_cmdline_option() helper Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 14/32] introduce StateFile module for state file related helpers Fiona Ebner
` (19 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
The parameter is always set since commit 2de2d6f7 ("allow dedicated
migration network, bug #1177"), qemu-server 4.0-93.
Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
New in v2.
src/PVE/QemuServer.pm | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 5ab3a8c3..ea990751 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -6043,16 +6043,9 @@ sub vm_start_nolock {
if ($statefile eq 'tcp') {
my $migrate = $res->{migrate} = { proto => 'tcp' };
$migrate->{addr} = "localhost";
- my $datacenterconf = PVE::Cluster::cfs_read_file('datacenter.cfg');
my $nodename = nodename();
- if (!defined($migration_type)) {
- if (defined($datacenterconf->{migration}->{type})) {
- $migration_type = $datacenterconf->{migration}->{type};
- } else {
- $migration_type = 'secure';
- }
- }
+ die "no migration type set\n" if !defined($migration_type);
if ($migration_type eq 'insecure') {
$migrate->{addr} = $get_migration_ip->($nodename);
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 14/32] introduce StateFile module for state file related helpers
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (12 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 13/32] vm start: assert that migration type is set for 'tcp' migration Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 15/32] vm start: move state file handling to dedicated module Fiona Ebner
` (18 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
The following commit will also move the state file handling which uses
the migration IP to the module.
Since QemuServer.pm does not use QemuMigrate.pm and since state file
handling can be both network migration or regular state file, it seems
best to add a new module. The goal is to make vm_start_nolock() more
readable.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Changes in v2:
* Fix style for error strings/ensure single-line post-if.
src/PVE/QemuServer.pm | 48 +++++++++------------------------
src/PVE/QemuServer/Makefile | 1 +
src/PVE/QemuServer/StateFile.pm | 32 ++++++++++++++++++++++
3 files changed, 46 insertions(+), 35 deletions(-)
create mode 100644 src/PVE/QemuServer/StateFile.pm
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index ea990751..c1133eb8 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -73,6 +73,7 @@ use PVE::QemuServer::Monitor qw(mon_cmd);
use PVE::QemuServer::PCI qw(print_pci_addr print_pcie_addr print_pcie_root_port parse_hostpci);
use PVE::QemuServer::QMPHelpers qw(qemu_deviceadd qemu_devicedel qemu_objectadd qemu_objectdel);
use PVE::QemuServer::RNG qw(parse_rng print_rng_device_commandline print_rng_object_commandline);
+use PVE::QemuServer::StateFile;
use PVE::QemuServer::USB;
use PVE::QemuServer::Virtiofs qw(max_virtiofs start_all_virtiofsd);
@@ -5943,6 +5944,7 @@ sub vm_start_nolock {
my $migratedfrom = $migrate_opts->{migratedfrom};
my $migration_type = $migrate_opts->{type};
+ my $nbd_protocol_version = $migrate_opts->{nbd_proto_version} // 0;
my $res = {};
@@ -6007,48 +6009,25 @@ sub vm_start_nolock {
);
my $migration_ip;
- my $get_migration_ip = sub {
- my ($nodename) = @_;
-
- return $migration_ip if defined($migration_ip);
-
- my $cidr = $migrate_opts->{network};
-
- if (!defined($cidr)) {
- my $dc_conf = PVE::Cluster::cfs_read_file('datacenter.cfg');
- $cidr = $dc_conf->{migration}->{network};
- }
-
- if (defined($cidr)) {
- my $ips = PVE::Network::get_local_ip_from_cidr($cidr);
-
- die "could not get IP: no address configured on local "
- . "node for network '$cidr'\n"
- if scalar(@$ips) == 0;
-
- die "could not get IP: multiple addresses configured on local "
- . "node for network '$cidr'\n"
- if scalar(@$ips) > 1;
-
- $migration_ip = @$ips[0];
- }
-
- $migration_ip = PVE::Cluster::remote_node_ip($nodename, 1)
- if !defined($migration_ip);
-
- return $migration_ip;
- };
+ if (
+ ($statefile && $statefile eq 'tcp' && $migration_type eq 'insecure')
+ || ($migrate_opts->{nbd}
+ && ($nbd_protocol_version == 0 || $migration_type eq 'insecure'))
+ ) {
+ my $nodename = nodename();
+ $migration_ip =
+ PVE::QemuServer::StateFile::get_migration_ip($nodename, $migrate_opts->{network});
+ }
if ($statefile) {
if ($statefile eq 'tcp') {
my $migrate = $res->{migrate} = { proto => 'tcp' };
$migrate->{addr} = "localhost";
- my $nodename = nodename();
die "no migration type set\n" if !defined($migration_type);
if ($migration_type eq 'insecure') {
- $migrate->{addr} = $get_migration_ip->($nodename);
+ $migrate->{addr} = $migration_ip // die "internal error - no migration IP";
$migrate->{addr} = "[$migrate->{addr}]" if Net::IP::ip_is_ipv6($migrate->{addr});
}
@@ -6264,7 +6243,6 @@ sub vm_start_nolock {
#start nbd server for storage migration
if (my $nbd = $migrate_opts->{nbd}) {
- my $nbd_protocol_version = $migrate_opts->{nbd_proto_version} // 0;
my $migrate_storage_uri;
# nbd_protocol_version > 0 for unix socket support
@@ -6282,7 +6260,7 @@ sub vm_start_nolock {
$res->{migrate}->{unix_sockets} = [$socket_path];
} else {
my $nodename = nodename();
- my $localip = $get_migration_ip->($nodename);
+ my $localip = $migration_ip // die "internal error - no migration IP";
my $pfamily = PVE::Tools::get_host_address_family($nodename);
my $storage_migrate_port = PVE::Tools::next_migrate_port($pfamily);
diff --git a/src/PVE/QemuServer/Makefile b/src/PVE/QemuServer/Makefile
index 9812c404..0d09b763 100644
--- a/src/PVE/QemuServer/Makefile
+++ b/src/PVE/QemuServer/Makefile
@@ -16,6 +16,7 @@ SOURCES=Agent.pm \
PCI.pm \
QMPHelpers.pm \
RNG.pm \
+ StateFile.pm \
USB.pm \
Virtiofs.pm
diff --git a/src/PVE/QemuServer/StateFile.pm b/src/PVE/QemuServer/StateFile.pm
new file mode 100644
index 00000000..d2cdde46
--- /dev/null
+++ b/src/PVE/QemuServer/StateFile.pm
@@ -0,0 +1,32 @@
+package PVE::QemuServer::StateFile;
+
+use strict;
+use warnings;
+
+use PVE::Cluster;
+use PVE::Network;
+
+sub get_migration_ip {
+ my ($nodename, $cidr) = @_;
+
+ if (!defined($cidr)) {
+ my $dc_conf = PVE::Cluster::cfs_read_file('datacenter.cfg');
+ $cidr = $dc_conf->{migration}->{network};
+ }
+
+ if (defined($cidr)) {
+ my $ips = PVE::Network::get_local_ip_from_cidr($cidr);
+
+ die "could not get IP: no address configured on local node for network '$cidr'\n"
+ if scalar(@$ips) == 0;
+
+ die "could not get IP: multiple addresses configured on local node for network '$cidr'\n"
+ if scalar(@$ips) > 1;
+
+ return $ips->[0];
+ }
+
+ return PVE::Cluster::remote_node_ip($nodename, 1);
+}
+
+1;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 15/32] vm start: move state file handling to dedicated module
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (13 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 14/32] introduce StateFile module for state file related helpers Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 16/32] vm start: move config_to_command() call further down Fiona Ebner
` (17 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Changes in v2:
* Re-do on top of change from patch 13/32.
src/PVE/QemuServer.pm | 49 ++++++++-------------------------
src/PVE/QemuServer/StateFile.pm | 47 +++++++++++++++++++++++++++++++
2 files changed, 58 insertions(+), 38 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index c1133eb8..442adde6 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5946,8 +5946,6 @@ sub vm_start_nolock {
my $migration_type = $migrate_opts->{type};
my $nbd_protocol_version = $migrate_opts->{nbd_proto_version} // 0;
- my $res = {};
-
# clean up leftover reboot request files
eval { clear_reboot_request($vmid); };
warn $@ if $@;
@@ -6019,46 +6017,21 @@ sub vm_start_nolock {
PVE::QemuServer::StateFile::get_migration_ip($nodename, $migrate_opts->{network});
}
+ my $res = {};
+ my $statefile_is_a_volume;
+ my $state_cmdline = [];
if ($statefile) {
- if ($statefile eq 'tcp') {
- my $migrate = $res->{migrate} = { proto => 'tcp' };
- $migrate->{addr} = "localhost";
-
- die "no migration type set\n" if !defined($migration_type);
-
- if ($migration_type eq 'insecure') {
- $migrate->{addr} = $migration_ip // die "internal error - no migration IP";
- $migrate->{addr} = "[$migrate->{addr}]" if Net::IP::ip_is_ipv6($migrate->{addr});
- }
-
- # see #4501: port reservation should be done close to usage - tell QEMU where to listen
- # via QMP later
- push @$cmd, '-incoming', 'defer';
- push @$cmd, '-S';
-
- } elsif ($statefile eq 'unix') {
- # should be default for secure migrations as a ssh TCP forward
- # tunnel is not deterministic reliable ready and fails regurarly
- # to set up in time, so use UNIX socket forwards
- my $migrate = $res->{migrate} = { proto => 'unix' };
- $migrate->{addr} = "/run/qemu-server/$vmid.migrate";
- unlink $migrate->{addr};
-
- $migrate->{uri} = "unix:$migrate->{addr}";
- push @$cmd, '-incoming', $migrate->{uri};
- push @$cmd, '-S';
-
- } elsif (-e $statefile) {
- push @$cmd, '-loadstate', $statefile;
- } else {
- my $statepath = PVE::Storage::path($storecfg, $statefile);
- push @$vollist, $statefile;
- push @$cmd, '-loadstate', $statepath;
- }
+ ($state_cmdline, $res->{migrate}, $statefile_is_a_volume) =
+ PVE::QemuServer::StateFile::statefile_cmdline_option(
+ $storecfg, $vmid, $statefile, $migration_type, $migration_ip,
+ );
+ push @$vollist, $statefile if $statefile_is_a_volume;
} elsif ($params->{paused}) {
- push @$cmd, '-S';
+ $state_cmdline = ['-S'];
}
+ push $cmd->@*, $state_cmdline->@*;
+
my $memory = get_current_memory($conf->{memory});
my $start_timeout = $params->{timeout} // config_aware_timeout($conf, $memory, $resume);
diff --git a/src/PVE/QemuServer/StateFile.pm b/src/PVE/QemuServer/StateFile.pm
index d2cdde46..3c26a9c6 100644
--- a/src/PVE/QemuServer/StateFile.pm
+++ b/src/PVE/QemuServer/StateFile.pm
@@ -29,4 +29,51 @@ sub get_migration_ip {
return PVE::Cluster::remote_node_ip($nodename, 1);
}
+# $migration_ip must be defined if using insecure TCP migration
+sub statefile_cmdline_option {
+ my ($storecfg, $vmid, $statefile, $migration_type, $migration_ip) = @_;
+
+ my $statefile_is_a_volume = 0;
+ my $res = {};
+ my $cmd = [];
+
+ if ($statefile eq 'tcp') {
+ my $migrate = $res->{migrate} = { proto => 'tcp' };
+ $migrate->{addr} = "localhost";
+
+ die "no migration type set\n" if !defined($migration_type);
+
+ if ($migration_type eq 'insecure') {
+ $migrate->{addr} = $migration_ip // die "internal error - no migration IP";
+ $migrate->{addr} = "[$migrate->{addr}]" if Net::IP::ip_is_ipv6($migrate->{addr});
+ }
+
+ # see #4501: port reservation should be done close to usage - tell QEMU where to listen
+ # via QMP later
+ push @$cmd, '-incoming', 'defer';
+ push @$cmd, '-S';
+
+ } elsif ($statefile eq 'unix') {
+ # should be default for secure migrations as a ssh TCP forward
+ # tunnel is not deterministic reliable ready and fails regurarly
+ # to set up in time, so use UNIX socket forwards
+ my $migrate = $res->{migrate} = { proto => 'unix' };
+ $migrate->{addr} = "/run/qemu-server/$vmid.migrate";
+ unlink $migrate->{addr};
+
+ $migrate->{uri} = "unix:$migrate->{addr}";
+ push @$cmd, '-incoming', $migrate->{uri};
+ push @$cmd, '-S';
+
+ } elsif (-e $statefile) {
+ push @$cmd, '-loadstate', $statefile;
+ } else {
+ my $statepath = PVE::Storage::path($storecfg, $statefile);
+ $statefile_is_a_volume = 1;
+ push @$cmd, '-loadstate', $statepath;
+ }
+
+ return ($cmd, $res->{migrate}, $statefile_is_a_volume);
+}
+
1;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 16/32] vm start: move config_to_command() call further down
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (14 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 15/32] vm start: move state file handling to dedicated module Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 17/32] vm start/commandline: also clean up pci reservation when config_to_command() fails Fiona Ebner
` (16 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
For command line generation of '-blockdev', it will be necessary to
activate the VM's volumes before calling config_to_command() and thus
also deactivate in an error scenario. Avoid the need to put more code
than necessary into the resulting eval.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Changes in v2:
* For templates, keep using minimized config for timeout calculation.
src/PVE/QemuServer.pm | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 442adde6..4ddab4c1 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5994,18 +5994,6 @@ sub vm_start_nolock {
print "Resuming suspended VM\n";
}
- # Note that for certain cases like templates, the configuration is minimized, so need to ensure
- # the rest of the function here uses the same configuration that was used to build the command
- (my $cmd, my $vollist, my $spice_port, my $pci_devices, $conf) = config_to_command(
- $storecfg,
- $vmid,
- $conf,
- $defaults,
- $forcemachine,
- $forcecpu,
- $params->{'live-restore-backing'},
- );
-
my $migration_ip;
if (
($statefile && $statefile eq 'tcp' && $migration_type eq 'insecure')
@@ -6025,12 +6013,24 @@ sub vm_start_nolock {
PVE::QemuServer::StateFile::statefile_cmdline_option(
$storecfg, $vmid, $statefile, $migration_type, $migration_ip,
);
- push @$vollist, $statefile if $statefile_is_a_volume;
} elsif ($params->{paused}) {
$state_cmdline = ['-S'];
}
+ # Note that for certain cases like templates, the configuration is minimized, so need to ensure
+ # the rest of the function here uses the same configuration that was used to build the command
+ (my $cmd, my $vollist, my $spice_port, my $pci_devices, $conf) = config_to_command(
+ $storecfg,
+ $vmid,
+ $conf,
+ $defaults,
+ $forcemachine,
+ $forcecpu,
+ $params->{'live-restore-backing'},
+ );
+
push $cmd->@*, $state_cmdline->@*;
+ push @$vollist, $statefile if $statefile_is_a_volume;
my $memory = get_current_memory($conf->{memory});
my $start_timeout = $params->{timeout} // config_aware_timeout($conf, $memory, $resume);
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 17/32] vm start/commandline: also clean up pci reservation when config_to_command() fails
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (15 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 16/32] vm start: move config_to_command() call further down Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 18/32] vm start/commandline: activate volumes before config_to_command() Fiona Ebner
` (15 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
The config_to_command() function already calls into
print_hostpci_devices() which makes reservations.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Changes in v2:
* Adapt to changes in previous patch.
src/PVE/QemuServer.pm | 60 ++++++++++++++++++++++++-------------------
1 file changed, 33 insertions(+), 27 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 4ddab4c1..68d5c1b5 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -6017,34 +6017,35 @@ sub vm_start_nolock {
$state_cmdline = ['-S'];
}
- # Note that for certain cases like templates, the configuration is minimized, so need to ensure
- # the rest of the function here uses the same configuration that was used to build the command
- (my $cmd, my $vollist, my $spice_port, my $pci_devices, $conf) = config_to_command(
- $storecfg,
- $vmid,
- $conf,
- $defaults,
- $forcemachine,
- $forcecpu,
- $params->{'live-restore-backing'},
- );
-
- push $cmd->@*, $state_cmdline->@*;
- push @$vollist, $statefile if $statefile_is_a_volume;
-
- my $memory = get_current_memory($conf->{memory});
- my $start_timeout = $params->{timeout} // config_aware_timeout($conf, $memory, $resume);
-
+ my ($cmd, $vollist, $spice_port, $start_timeout);
my $pci_reserve_list = [];
- for my $device (values $pci_devices->%*) {
- next if $device->{mdev}; # we don't reserve for mdev devices
- push $pci_reserve_list->@*, map { $_->{id} } $device->{ids}->@*;
- }
-
- # reserve all PCI IDs before actually doing anything with them
- PVE::QemuServer::PCI::reserve_pci_usage($pci_reserve_list, $vmid, $start_timeout);
-
eval {
+ # Note that for certain cases like templates, the configuration is minimized, so need to ensure
+ # the rest of the function here uses the same configuration that was used to build the command
+ ($cmd, $vollist, $spice_port, my $pci_devices, $conf) = config_to_command(
+ $storecfg,
+ $vmid,
+ $conf,
+ $defaults,
+ $forcemachine,
+ $forcecpu,
+ $params->{'live-restore-backing'},
+ );
+
+ my $memory = get_current_memory($conf->{memory});
+ $start_timeout = $params->{timeout} // config_aware_timeout($conf, $memory, $resume);
+
+ push $cmd->@*, $state_cmdline->@*;
+ push @$vollist, $statefile if $statefile_is_a_volume;
+
+ for my $device (values $pci_devices->%*) {
+ next if $device->{mdev}; # we don't reserve for mdev devices
+ push $pci_reserve_list->@*, map { $_->{id} } $device->{ids}->@*;
+ }
+
+ # reserve all PCI IDs before actually doing anything with them
+ PVE::QemuServer::PCI::reserve_pci_usage($pci_reserve_list, $vmid, $start_timeout);
+
my $uuid;
for my $id (sort keys %$pci_devices) {
my $d = $pci_devices->{$id};
@@ -6368,13 +6369,18 @@ sub vm_commandline {
my $defaults = load_defaults();
- my $cmd = config_to_command($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu);
+ my $cmd;
+ eval { $cmd = config_to_command($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu); };
+ my $err = $@;
+
# if the vm is not running, we need to clean up the reserved/created devices
if (!PVE::QemuServer::Helpers::vm_running_locally($vmid)) {
eval { cleanup_pci_devices($vmid, $conf) };
warn $@ if $@;
}
+ die $err if $err;
+
return PVE::Tools::cmd2string($cmd);
}
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 18/32] vm start/commandline: activate volumes before config_to_command()
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (16 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 17/32] vm start/commandline: also clean up pci reservation when config_to_command() fails Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-20 11:33 ` Fabian Grünbichler
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 19/32] pci: add missing includes Fiona Ebner
` (14 subsequent siblings)
32 siblings, 1 reply; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
With '-blockdev', it is necessary to activate the volumes to generate
the command line, because it can be necessary to check whether the
volume is a block device or a regular file.
Do not deactivate after commandline generation for 'qm showcmd',
because there can be concurrent operations acting on the volumes.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Changes in v2:
* Do not deactivate after showcmd, because it might not be safe.
src/PVE/QemuServer.pm | 60 +++++++++++++++++++++-------
src/test/run_config2command_tests.pl | 10 +++++
2 files changed, 55 insertions(+), 15 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 68d5c1b5..38b3a569 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -4016,7 +4016,6 @@ sub config_to_command {
push @$devices, '-watchdog-action', $wdopts->{action} if $wdopts->{action};
}
- my $vollist = [];
my $scsicontroller = {};
my $ahcicontroller = {};
my $scsihw = defined($conf->{scsihw}) ? $conf->{scsihw} : $defaults->{scsihw};
@@ -4031,11 +4030,6 @@ sub config_to_command {
sub {
my ($ds, $drive) = @_;
- if (PVE::Storage::parse_volume_id($drive->{file}, 1)) {
- check_volume_storage_type($storecfg, $drive->{file});
- push @$vollist, $drive->{file};
- }
-
# ignore efidisk here, already added in bios/fw handling code above
return if $drive->{interface} eq 'efidisk';
# similar for TPM
@@ -4234,7 +4228,6 @@ sub config_to_command {
if (my $vmstate = $conf->{vmstate}) {
my $statepath = PVE::Storage::path($storecfg, $vmstate);
- push @$vollist, $vmstate;
push @$cmd, '-loadstate', $statepath;
print "activating and using '$vmstate' as vmstate\n";
}
@@ -4250,7 +4243,7 @@ sub config_to_command {
push @$cmd, @$aa;
}
- return wantarray ? ($cmd, $vollist, $spice_port, $pci_devices, $conf) : $cmd;
+ return wantarray ? ($cmd, $spice_port, $pci_devices, $conf) : $cmd;
}
sub spice_port {
@@ -6017,12 +6010,18 @@ sub vm_start_nolock {
$state_cmdline = ['-S'];
}
- my ($cmd, $vollist, $spice_port, $start_timeout);
+ my $vollist = get_current_vm_volumes($storecfg, $conf);
+ push $vollist->@*, $statefile if $statefile_is_a_volume;
+
+ my ($cmd, $spice_port, $start_timeout);
my $pci_reserve_list = [];
eval {
+ # With -blockdev, it is necessary to activate the volumes before generating the command line
+ PVE::Storage::activate_volumes($storecfg, $vollist);
+
# Note that for certain cases like templates, the configuration is minimized, so need to ensure
# the rest of the function here uses the same configuration that was used to build the command
- ($cmd, $vollist, $spice_port, my $pci_devices, $conf) = config_to_command(
+ ($cmd, $spice_port, my $pci_devices, $conf) = config_to_command(
$storecfg,
$vmid,
$conf,
@@ -6036,7 +6035,6 @@ sub vm_start_nolock {
$start_timeout = $params->{timeout} // config_aware_timeout($conf, $memory, $resume);
push $cmd->@*, $state_cmdline->@*;
- push @$vollist, $statefile if $statefile_is_a_volume;
for my $device (values $pci_devices->%*) {
next if $device->{mdev}; # we don't reserve for mdev devices
@@ -6080,13 +6078,13 @@ sub vm_start_nolock {
push @$cmd, '-uuid', $uuid if defined($uuid);
};
if (my $err = $@) {
+ eval { PVE::Storage::deactivate_volumes($storecfg, $vollist); };
+ warn $@ if $@;
eval { cleanup_pci_devices($vmid, $conf) };
warn $@ if $@;
die $err;
}
- PVE::Storage::activate_volumes($storecfg, $vollist);
-
my %silence_std_outs = (outfunc => sub { }, errfunc => sub { });
eval { run_command(['/bin/systemctl', 'reset-failed', "$vmid.scope"], %silence_std_outs) };
eval { run_command(['/bin/systemctl', 'stop', "$vmid.scope"], %silence_std_outs) };
@@ -6369,12 +6367,22 @@ sub vm_commandline {
my $defaults = load_defaults();
+ my $running = PVE::QemuServer::Helpers::vm_running_locally($vmid);
+ my $volumes = [];
+
+ # With -blockdev, it is necessary to activate the volumes before generating the command line
+ if (!$running) {
+ $volumes = get_current_vm_volumes($storecfg, $conf);
+ PVE::Storage::activate_volumes($storecfg, $volumes);
+ }
+
my $cmd;
eval { $cmd = config_to_command($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu); };
my $err = $@;
- # if the vm is not running, we need to clean up the reserved/created devices
- if (!PVE::QemuServer::Helpers::vm_running_locally($vmid)) {
+ # If the vm is not running, need to clean up the reserved/created devices.
+ # There might be concurrent operations on the volumes, so do not deactivate.
+ if (!$running) {
eval { cleanup_pci_devices($vmid, $conf) };
warn $@ if $@;
}
@@ -6421,6 +6429,28 @@ sub get_vm_volumes {
return $vollist;
}
+# Get volumes defined in the current VM configuration, including the VM state file.
+sub get_current_vm_volumes {
+ my ($storecfg, $conf) = @_;
+
+ my $volumes = [];
+
+ PVE::QemuConfig->foreach_volume_full(
+ $conf,
+ { extra_keys => ['vmstate'] },
+ sub {
+ my ($ds, $drive) = @_;
+
+ if (PVE::Storage::parse_volume_id($drive->{file}, 1)) {
+ check_volume_storage_type($storecfg, $drive->{file});
+ push $volumes->@*, $drive->{file};
+ }
+ },
+ );
+
+ return $volumes;
+}
+
sub cleanup_pci_devices {
my ($vmid, $conf) = @_;
diff --git a/src/test/run_config2command_tests.pl b/src/test/run_config2command_tests.pl
index 590f80f5..cbd8cd8b 100755
--- a/src/test/run_config2command_tests.pl
+++ b/src/test/run_config2command_tests.pl
@@ -256,6 +256,16 @@ $qemu_server_module->mock(
},
);
+my $storage_module = Test::MockModule->new("PVE::Storage");
+$storage_module->mock(
+ activate_volumes => sub {
+ return;
+ },
+ deactivate_volumes => sub {
+ return;
+ },
+);
+
my $zfsplugin_module = Test::MockModule->new("PVE::Storage::ZFSPlugin");
$zfsplugin_module->mock(
zfs_get_lu_name => sub {
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 19/32] pci: add missing includes
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (17 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 18/32] vm start/commandline: activate volumes before config_to_command() Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 20/32] test: add tests for PCI reservations Fiona Ebner
` (13 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
New in v2.
src/PVE/QemuServer/PCI.pm | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
index 66bd1534..751f8a84 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -3,11 +3,16 @@ package PVE::QemuServer::PCI;
use warnings;
use strict;
+use IO::File;
+
use PVE::JSONSchema;
use PVE::Mapping::PCI;
use PVE::SysFSTools;
use PVE::Tools;
+use PVE::QemuServer::Helpers;
+use PVE::QemuServer::Machine;
+
use base 'Exporter';
our @EXPORT_OK = qw(
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 20/32] test: add tests for PCI reservations
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (18 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 19/32] pci: add missing includes Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 21/32] cfg2cmd: print vga: fix call to print_pcie_addr() Fiona Ebner
` (12 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
This is currently tested as part of the config-to-command tests, but
the plan is to make command generation for 'qm showcmd' not actually
reserve anything. The reserve_pci_usage() function also serves as
checking what already is reserved by other VMs at the same time.
Thus, the config-to-command test will not be able to test whether
reservations are actually respected after the above-mentioned change.
Add a dedicated test for this.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
New in v2.
src/PVE/QemuServer/PCI.pm | 2 +-
src/test/Makefile | 5 +-
src/test/run_pci_reservation_tests.pl | 175 ++++++++++++++++++++++++++
3 files changed, 180 insertions(+), 2 deletions(-)
create mode 100755 src/test/run_pci_reservation_tests.pl
diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
index 751f8a84..9b343115 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -576,7 +576,7 @@ my sub create_nvidia_device {
#
# mdev devices must be chosen later when we actually allocate it, but we
# flatten the inner list since there can only be one device per alternative anyway
-my sub choose_hostpci_devices {
+sub choose_hostpci_devices {
my ($devices, $vmid) = @_;
# if the vm is running, we must be in 'showcmd', so don't actually reserve or create anything
diff --git a/src/test/Makefile b/src/test/Makefile
index f7372e2e..2ef9073a 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -1,6 +1,6 @@
all: test
-test: test_snapshot test_cfg_to_cmd test_pci_addr_conflicts test_qemu_img_convert test_migration test_restore_config test_parse_config
+test: test_snapshot test_cfg_to_cmd test_pci_addr_conflicts test_pci_reservation test_qemu_img_convert test_migration test_restore_config test_parse_config
test_snapshot: run_snapshot_tests.pl
./run_snapshot_tests.pl
@@ -15,6 +15,9 @@ test_qemu_img_convert: run_qemu_img_convert_tests.pl
test_pci_addr_conflicts: run_pci_addr_checks.pl
./run_pci_addr_checks.pl
+test_pci_reservation: run_pci_reservation_tests.pl
+ ./run_pci_reservation_tests.pl
+
MIGRATION_TEST_TARGETS := $(addprefix test_migration_,$(shell perl -ne 'print "$$1 " if /^\s*name\s*=>\s*["'\'']([^\s"'\'']+)["'\'']\s*,\s*$$/; END { print "\n" }' run_qemu_migrate_tests.pl))
test_migration: run_qemu_migrate_tests.pl MigrationTest/*.pm $(MIGRATION_TEST_TARGETS)
diff --git a/src/test/run_pci_reservation_tests.pl b/src/test/run_pci_reservation_tests.pl
new file mode 100755
index 00000000..bd428a80
--- /dev/null
+++ b/src/test/run_pci_reservation_tests.pl
@@ -0,0 +1,175 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use lib qw(..);
+
+my $vmid = 8006;
+
+use Test::MockModule;
+use Test::More;
+
+use PVE::Mapping::PCI;
+
+use PVE::QemuServer::PCI;
+
+my $pci_devs = [
+ "0000:00:43.1",
+ "0000:00:f4.0",
+ "0000:00:ff.1",
+ "0000:0f:f2.0",
+ "0000:d0:13.0",
+ "0000:d0:15.1",
+ "0000:d0:15.2",
+ "0000:d0:17.0",
+ "0000:f0:42.0",
+ "0000:f0:43.0",
+ "0000:f0:43.1",
+ "1234:f0:43.1",
+ "0000:01:00.4",
+ "0000:01:00.5",
+ "0000:01:00.6",
+ "0000:07:10.0",
+ "0000:07:10.1",
+ "0000:07:10.4",
+];
+
+my $pci_map_config = {
+ ids => {
+ someGpu => {
+ type => 'pci',
+ mdev => 1,
+ map => [
+ 'node=localhost,path=0000:01:00.4,id=10de:2231,iommugroup=1',
+ 'node=localhost,path=0000:01:00.5,id=10de:2231,iommugroup=1',
+ 'node=localhost,path=0000:01:00.6,id=10de:2231,iommugroup=1',
+ ],
+ },
+ someNic => {
+ type => 'pci',
+ map => [
+ 'node=localhost,path=0000:07:10.0,id=8086:1520,iommugroup=2',
+ 'node=localhost,path=0000:07:10.1,id=8086:1520,iommugroup=2',
+ 'node=localhost,path=0000:07:10.4,id=8086:1520,iommugroup=2',
+ ],
+ },
+ },
+};
+
+my $tests = [
+ {
+ name => 'reservation-is-respected',
+ conf => {
+ hostpci0 => 'mapping=someNic',
+ hostpci1 => 'mapping=someGpu,mdev=some-model',
+ hostpci2 => 'mapping=someNic',
+ },
+ expected => {
+ hostpci0 => { ids => [{ id => '0000:07:10.0' }] },
+ hostpci1 => {
+ ids => [
+ { id => '0000:01:00.4' }, { id => '0000:01:00.5' },
+ { id => '0000:01:00.6' },
+ ],
+ mdev => 'some-model',
+ },
+ hostpci2 => { ids => [{ id => '0000:07:10.4' }] },
+ },
+ },
+];
+
+plan tests => scalar($tests->@*);
+
+my $pve_common_inotify;
+$pve_common_inotify = Test::MockModule->new('PVE::INotify');
+$pve_common_inotify->mock(
+ nodename => sub {
+ return 'localhost';
+ },
+);
+
+my $pve_common_sysfstools;
+$pve_common_sysfstools = Test::MockModule->new('PVE::SysFSTools');
+$pve_common_sysfstools->mock(
+ lspci => sub {
+ my ($filter, $verbose) = @_;
+
+ return [
+ map { { id => $_ } }
+ grep {
+ !defined($filter)
+ || (!ref($filter) && $_ =~ m/^(0000:)?\Q$filter\E/)
+ || (ref($filter) eq 'CODE' && $filter->({ id => $_ }))
+ } sort @$pci_devs
+ ];
+ },
+ pci_device_info => sub {
+ my ($path, $noerr) = @_;
+
+ if ($path =~ m/^0000:01:00/) {
+ return {
+ mdev => 1,
+ iommugroup => 1,
+ mdev => 1,
+ vendor => "0x10de",
+ device => "0x2231",
+ };
+ } elsif ($path =~ m/^0000:07:10/) {
+ return {
+ iommugroup => 2,
+ vendor => "0x8086",
+ device => "0x1520",
+ };
+ } else {
+ return {};
+ }
+ },
+);
+
+my $mapping_pci_module = Test::MockModule->new("PVE::Mapping::PCI");
+$mapping_pci_module->mock(
+ config => sub {
+ return $pci_map_config;
+ },
+);
+
+my $pci_module = Test::MockModule->new("PVE::QemuServer::PCI");
+$pci_module->mock(
+ reserve_pci_usage => sub {
+ my ($ids, $vmid, $timeout, $pid, $dryrun) = @_;
+
+ $ids = [$ids] if !ref($ids);
+
+ for my $id (@$ids) {
+ if ($id eq "0000:07:10.1") {
+ die "reserved";
+ }
+ }
+
+ return undef;
+ },
+ create_nvidia_device => sub {
+ return 1;
+ },
+);
+
+for my $test ($tests->@*) {
+ my ($name, $conf, $expected) = $test->@{qw(name conf expected)};
+ my $pci_devices;
+ eval {
+ my $devices = PVE::QemuServer::PCI::parse_hostpci_devices($conf);
+ use JSON;
+ $pci_devices = PVE::QemuServer::PCI::choose_hostpci_devices($devices, $vmid);
+ };
+ if (my $err = $@) {
+ is($err, $expected, $name);
+ } elsif ($pci_devices) {
+ is_deeply($pci_devices, $expected, $name);
+ } else {
+ fail($name);
+ note("no result");
+ }
+}
+
+done_testing();
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 21/32] cfg2cmd: print vga: fix call to print_pcie_addr()
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (19 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 20/32] test: add tests for PCI reservations Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 22/32] pci: code cleanup: remove superfluous machine type paramater from print_pci_addr Fiona Ebner
` (11 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: pve-devel
The function print_pcie_addr() only takes a single parameter.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
New in v2.
src/PVE/QemuServer.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 38b3a569..48ae41c0 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1786,7 +1786,7 @@ sub print_vga_device {
my $pciaddr;
if ($q35 && $vgaid eq 'vga') {
# the first display uses pcie.0 bus on q35 machines
- $pciaddr = print_pcie_addr($vgaid, $bridges, $arch, $machine);
+ $pciaddr = print_pcie_addr($vgaid);
} else {
$pciaddr = print_pci_addr($vgaid, $bridges, $arch, $machine);
}
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 22/32] pci: code cleanup: remove superfluous machine type paramater from print_pci_addr
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (20 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 21/32] cfg2cmd: print vga: fix call to print_pcie_addr() Fiona Ebner
@ 2025-06-18 13:01 ` Fiona Ebner
2025-06-18 15:19 ` Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 23/32] cfg2cmd: collect optional parameters as a hash Fiona Ebner
` (10 subsequent siblings)
32 siblings, 1 reply; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:01 UTC (permalink / raw)
To: 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 <f.ebner@proxmox.com>
---
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 23/32] cfg2cmd: collect optional parameters as a hash
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (21 preceding siblings ...)
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 22/32] pci: code cleanup: remove superfluous machine type paramater from print_pci_addr Fiona Ebner
@ 2025-06-18 13:02 ` Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 24/32] qm: showcmd: never reserve PCI devices Fiona Ebner
` (9 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:02 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
New in v2.
src/PVE/QemuServer.pm | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index bffce201..245e1a26 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -3526,7 +3526,10 @@ my sub get_vga_properties {
}
sub config_to_command {
- my ($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu, $live_restore_backing) = @_;
+ my ($storecfg, $vmid, $conf, $defaults, $options) = @_;
+
+ my ($forcemachine, $forcecpu, $live_restore_backing) =
+ $options->@{qw(force-machine force-cpu live-restore-backing)};
# minimize config for templates, they can only start for backup,
# so most options besides the disks are irrelevant
@@ -5995,9 +5998,11 @@ sub vm_start_nolock {
$vmid,
$conf,
$defaults,
- $forcemachine,
- $forcecpu,
- $params->{'live-restore-backing'},
+ {
+ 'force-machine' => $forcemachine,
+ 'force-cpu' => $forcecpu,
+ 'live-restore-backing' => $params->{'live-restore-backing'},
+ },
);
my $memory = get_current_memory($conf->{memory});
@@ -6320,14 +6325,14 @@ sub vm_commandline {
my $conf = PVE::QemuConfig->load_config($vmid);
- my ($forcemachine, $forcecpu);
+ my $options = {};
if ($snapname) {
my $snapshot = $conf->{snapshots}->{$snapname};
die "snapshot '$snapname' does not exist\n" if !defined($snapshot);
# check for machine or CPU overrides in snapshot
- $forcemachine = $snapshot->{runningmachine};
- $forcecpu = $snapshot->{runningcpu};
+ $options->{'force-machine'} = $snapshot->{runningmachine};
+ $options->{'force-cpu'} = $snapshot->{runningcpu};
$snapshot->{digest} = $conf->{digest}; # keep file digest for API
@@ -6346,7 +6351,7 @@ sub vm_commandline {
}
my $cmd;
- eval { $cmd = config_to_command($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu); };
+ eval { $cmd = config_to_command($storecfg, $vmid, $conf, $defaults, $options); };
my $err = $@;
# If the vm is not running, need to clean up the reserved/created devices.
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 24/32] qm: showcmd: never reserve PCI devices
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (22 preceding siblings ...)
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 23/32] cfg2cmd: collect optional parameters as a hash Fiona Ebner
@ 2025-06-18 13:02 ` Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 25/32] vm devices list: prepare querying block device names for -blockdev Fiona Ebner
` (8 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:02 UTC (permalink / raw)
To: pve-devel
Never reserve PCI devices when config_to_command() is called for
'showcmd'. Previously, choose_hostpci_devices() only skipped reserving
PCI devices when the VM was already running as a heuristic. Make it
explicit via a new parameter. Adapt the config-to-command test to
never expect actual reservation. There is a dedicated test for this
since commit "test: add tests for PCI reservations".
Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
New in v2.
src/PVE/QemuServer.pm | 19 +++++--------------
src/PVE/QemuServer/PCI.pm | 15 ++++++---------
.../q35-linux-hostpci-mapping.conf.cmd | 2 +-
src/test/run_config2command_tests.pl | 14 ++------------
4 files changed, 14 insertions(+), 36 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 245e1a26..843db76a 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -3528,8 +3528,8 @@ my sub get_vga_properties {
sub config_to_command {
my ($storecfg, $vmid, $conf, $defaults, $options) = @_;
- my ($forcemachine, $forcecpu, $live_restore_backing) =
- $options->@{qw(force-machine force-cpu live-restore-backing)};
+ my ($forcemachine, $forcecpu, $live_restore_backing, $dry_run) =
+ $options->@{qw(force-machine force-cpu live-restore-backing dry-run)};
# minimize config for templates, they can only start for backup,
# so most options besides the disks are irrelevant
@@ -3738,7 +3738,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, $bootorder,
+ $vmid, $conf, $devices, $vga, $winversion, $bridges, $arch, $bootorder, $dry_run,
);
# usb devices
@@ -6325,7 +6325,7 @@ sub vm_commandline {
my $conf = PVE::QemuConfig->load_config($vmid);
- my $options = {};
+ my $options = { 'dry-run' => 1 };
if ($snapname) {
my $snapshot = $conf->{snapshots}->{$snapname};
die "snapshot '$snapname' does not exist\n" if !defined($snapshot);
@@ -6350,18 +6350,9 @@ sub vm_commandline {
PVE::Storage::activate_volumes($storecfg, $volumes);
}
- my $cmd;
- eval { $cmd = config_to_command($storecfg, $vmid, $conf, $defaults, $options); };
- my $err = $@;
-
- # If the vm is not running, need to clean up the reserved/created devices.
# There might be concurrent operations on the volumes, so do not deactivate.
- if (!$running) {
- eval { cleanup_pci_devices($vmid, $conf) };
- warn $@ if $@;
- }
- die $err if $err;
+ my $cmd = config_to_command($storecfg, $vmid, $conf, $defaults, $options);
return PVE::Tools::cmd2string($cmd);
}
diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
index 2dbc87a7..ddf2f028 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -577,10 +577,7 @@ my sub create_nvidia_device {
# mdev devices must be chosen later when we actually allocate it, but we
# flatten the inner list since there can only be one device per alternative anyway
sub choose_hostpci_devices {
- my ($devices, $vmid) = @_;
-
- # if the vm is running, we must be in 'showcmd', so don't actually reserve or create anything
- my $is_running = PVE::QemuServer::Helpers::vm_running_locally($vmid) ? 1 : 0;
+ my ($devices, $vmid, $dry_run) = @_;
my $used = {};
@@ -606,7 +603,7 @@ sub choose_hostpci_devices {
# we only have one alternative, use that
$device->{ids} = $device->{ids}->[0];
$add_used_device->($device->{ids});
- if ($device->{nvidia} && !$is_running) {
+ if ($device->{nvidia} && !$dry_run) {
reserve_pci_usage($device->{ids}->[0]->{id}, $vmid, 10, undef);
create_nvidia_device($device->{ids}->[0]->{id}, $device->{nvidia});
}
@@ -618,12 +615,12 @@ sub choose_hostpci_devices {
my $ids = [map { $_->{id} } @$alternative];
next if grep { defined($used->{$_}) } @$ids; # already used
- if (!$is_running) {
+ if (!$dry_run) {
eval { reserve_pci_usage($ids, $vmid, 10, undef) };
next if $@;
}
- if ($device->{nvidia} && !$is_running) {
+ if ($device->{nvidia} && !$dry_run) {
eval { create_nvidia_device($ids->[0], $device->{nvidia}) };
if (my $err = $@) {
warn $err;
@@ -645,14 +642,14 @@ sub choose_hostpci_devices {
}
sub print_hostpci_devices {
- my ($vmid, $conf, $devices, $vga, $winversion, $bridges, $arch, $bootorder) = @_;
+ my ($vmid, $conf, $devices, $vga, $winversion, $bridges, $arch, $bootorder, $dry_run) = @_;
my $kvm_off = 0;
my $gpu_passthrough = 0;
my $legacy_igd = 0;
my $pciaddr;
- my $pci_devices = choose_hostpci_devices(parse_hostpci_devices($conf), $vmid);
+ my $pci_devices = choose_hostpci_devices(parse_hostpci_devices($conf), $vmid, $dry_run);
for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++) {
my $id = "hostpci$i";
diff --git a/src/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd b/src/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd
index 533e1983..6df8b5f2 100644
--- a/src/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd
+++ b/src/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd
@@ -29,7 +29,7 @@
-device 'usb-tablet,id=tablet,bus=ehci.0,port=1' \
-device 'vfio-pci,host=0000:07:10.0,id=hostpci0,bus=pci.0,addr=0x10' \
-device 'vfio-pci,sysfsdev=/sys/bus/mdev/devices/00000001-0000-0000-0000-000000008006,id=hostpci1,bus=pci.0,addr=0x11' \
- -device 'vfio-pci,host=0000:07:10.4,id=hostpci2,bus=pci.0,addr=0x1b' \
+ -device 'vfio-pci,host=0000:07:10.1,id=hostpci2,bus=pci.0,addr=0x1b' \
-device 'VGA,id=vga,bus=pcie.0,addr=0x1' \
-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' \
diff --git a/src/test/run_config2command_tests.pl b/src/test/run_config2command_tests.pl
index cbd8cd8b..9f4ecabf 100755
--- a/src/test/run_config2command_tests.pl
+++ b/src/test/run_config2command_tests.pl
@@ -474,20 +474,10 @@ $mapping_pci_module->mock(
my $pci_module = Test::MockModule->new("PVE::QemuServer::PCI");
$pci_module->mock(
reserve_pci_usage => sub {
- my ($ids, $vmid, $timeout, $pid, $dryrun) = @_;
-
- $ids = [$ids] if !ref($ids);
-
- for my $id (@$ids) {
- if ($id eq "0000:07:10.1") {
- die "reserved";
- }
- }
-
- return undef;
+ die "reserve_pci_usage should not be called for 'qm showcmd'\n";
},
create_nvidia_device => sub {
- return 1;
+ die "create_nvidia_device should not be called for 'qm showcmd'\n";
},
);
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 25/32] vm devices list: prepare querying block device names for -blockdev
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (23 preceding siblings ...)
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 24/32] qm: showcmd: never reserve PCI devices Fiona Ebner
@ 2025-06-18 13:02 ` Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 26/32] print drive device: explicitly set write-cache starting with machine version 10.0 Fiona Ebner
` (7 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:02 UTC (permalink / raw)
To: pve-devel
From: Alexandre Derumier via pve-devel <pve-devel@lists.proxmox.com>
Look at the 'qdev' value, because the 'device' property is not
initialized with '-blockdev'. This can be seen in the QEMU source code
(the device property is the name of the block backend and blk->name is
assigned a value only in a code path reached via drive_new()). This
most likely was done to avoid confusion/clashes, since with
'-blockdev', the node that's inserted for a front-end device can
change and then both the block backend and the node would be named
the same, but not connected.
Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
[FE: fix commit message and comment - it does not depend on the presence of media
escape dot in regex
skip right away if qdev is undef to avoid warning when regex matching]
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Newly included in v2, makes hot{,un}plug after switch to -blockdev
work.
src/PVE/QemuServer.pm | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 843db76a..f8adb890 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -4249,11 +4249,19 @@ sub vm_devices_list {
$devices_to_check = $to_check;
}
+ # Block device IDs need to be checked at the qdev level, since with '-blockdev', the 'device'
+ # property will not be set.
my $resblock = mon_cmd($vmid, 'query-block');
- foreach my $block (@$resblock) {
- if ($block->{device} =~ m/^drive-(\S+)/) {
- $devices->{$1} = 1;
+ for my $block ($resblock->@*) {
+ my $qdev_id = $block->{qdev} or next;
+ if ($qdev_id =~ m|^/machine/peripheral/(virtio(\d+))/virtio-backend$|) {
+ $qdev_id = $1;
+ } elsif ($qdev_id =~ m|^/machine/system\.flash0$|) {
+ $qdev_id = 'pflash0';
+ } elsif ($qdev_id =~ m|^/machine/system\.flash1$|) {
+ $qdev_id = 'efidisk0';
}
+ $devices->{$qdev_id} = 1;
}
my $resmice = mon_cmd($vmid, 'query-mice');
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 26/32] print drive device: explicitly set write-cache starting with machine version 10.0
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (24 preceding siblings ...)
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 25/32] vm devices list: prepare querying block device names for -blockdev Fiona Ebner
@ 2025-06-18 13:02 ` Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 27/32] print drive device: set {r, w}error front-end properties " Fiona Ebner
` (6 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:02 UTC (permalink / raw)
To: pve-devel
With the 'blockdev' command line option, the cache options are split
up. While cache.direct and cache.no-flush can be set in the -blockdev
options, cache.writeback is a front-end property and was intentionally
removed from the 'blockdev' options by QEMU commit aaa436f998 ("block:
Remove cache.writeback from blockdev-add"). It needs to be configured
as the 'write-cache' property for the ide-hd/scsi-hd/virtio-blk
device.
Table from 'man kvm':
┌─────────────┬─────────────────┬──────────────┬────────────────┐
│ │ cache.writeback │ cache.direct │ cache.no-flush │
├─────────────┼─────────────────┼──────────────┼────────────────┤
│writeback │ on │ off │ off │
├─────────────┼─────────────────┼──────────────┼────────────────┤
│none │ on │ on │ off │
├─────────────┼─────────────────┼──────────────┼────────────────┤
│writethrough │ off │ off │ off │
├─────────────┼─────────────────┼──────────────┼────────────────┤
│directsync │ off │ on │ off │
├─────────────┼─────────────────┼──────────────┼────────────────┤
│unsafe │ on │ off │ on │
└─────────────┴─────────────────┴──────────────┴────────────────┘
Suggested-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Changes in v2:
* Comment that the version guard is for the switch to -blockdev.
src/PVE/QemuServer.pm | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index f8adb890..0b983ab9 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1403,6 +1403,8 @@ sub print_drivedevice_full {
my $device = '';
my $maxdev = 0;
+ my $machine_version = extract_version($machine_type, kvm_user_version());
+
my $drive_id = PVE::QemuServer::Drive::get_drive_id($drive);
if ($drive->{interface} eq 'virtio') {
my $pciaddr = print_pci_addr("$drive_id", $bridges, $arch);
@@ -1413,7 +1415,6 @@ sub print_drivedevice_full {
my ($maxdev, $controller, $controller_prefix) = scsihw_infos($conf, $drive);
my $unit = $drive->{index} % $maxdev;
- my $machine_version = extract_version($machine_type, kvm_user_version());
my $device_type =
PVE::QemuServer::Drive::get_scsi_device_type($drive, $storecfg, $machine_version);
@@ -1489,6 +1490,16 @@ sub print_drivedevice_full {
$device .= ",serial=$serial";
}
+ if (min_version($machine_version, 10, 0)) { # for the switch to -blockdev
+ if (!drive_is_cdrom($drive)) {
+ my $write_cache = 'on';
+ if (my $cache = $drive->{cache}) {
+ $write_cache = 'off' if $cache eq 'writethrough' || $cache eq 'directsync';
+ }
+ $device .= ",write-cache=$write_cache";
+ }
+ }
+
return $device;
}
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 27/32] print drive device: set {r, w}error front-end properties starting with machine version 10.0
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (25 preceding siblings ...)
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 26/32] print drive device: explicitly set write-cache starting with machine version 10.0 Fiona Ebner
@ 2025-06-18 13:02 ` Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 28/32] print drive device: don't reference any drive for 'none' " Fiona Ebner
` (5 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:02 UTC (permalink / raw)
To: pve-devel
These properties cannot be specified via '-blockdev' like they could
with '-drive', because they are properties of the front-end drive
device.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
No changes in v2.
src/PVE/QemuServer.pm | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 0b983ab9..b9bc6b53 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1498,6 +1498,9 @@ sub print_drivedevice_full {
}
$device .= ",write-cache=$write_cache";
}
+ for my $o (qw(rerror werror)) {
+ $device .= ",$o=$drive->{$o}" if defined($drive->{$o});
+ }
}
return $device;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 28/32] print drive device: don't reference any drive for 'none' starting with machine version 10.0
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (26 preceding siblings ...)
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 27/32] print drive device: set {r, w}error front-end properties " Fiona Ebner
@ 2025-06-18 13:02 ` Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 29/32] drive: create a throttle group for each drive " Fiona Ebner
` (4 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:02 UTC (permalink / raw)
To: pve-devel
There will be no block node for 'none' after switching to '-blockdev'.
Co-developed-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
[FE: split out from larger patch
do it also for non-SCSI cases]
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Changes in v2:
* Shorten code using better conditional.
* Comment that the version guard is for the switch to -blockdev.
src/PVE/QemuServer.pm | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index b9bc6b53..4eccfb62 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1408,7 +1408,12 @@ 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,drive=drive-$drive_id,id=${drive_id}${pciaddr}";
+ $device = 'virtio-blk-pci';
+ # 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 .= ",id=${drive_id}${pciaddr}";
$device .= ",iothread=iothread-$drive_id" if $drive->{iothread};
} elsif ($drive->{interface} eq 'scsi') {
@@ -1424,7 +1429,11 @@ sub print_drivedevice_full {
$device = "scsi-$device_type,bus=$controller_prefix$controller.0,channel=0,scsi-id=0"
. ",lun=$drive->{index}";
}
- $device .= ",drive=drive-$drive_id,id=$drive_id";
+ # 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 .= ",id=$drive_id";
if ($drive->{ssd} && ($device_type eq 'block' || $device_type eq 'hd')) {
$device .= ",rotation_rate=1";
@@ -1464,7 +1473,10 @@ sub print_drivedevice_full {
} else {
$device .= ",bus=ahci$controller.$unit";
}
- $device .= ",drive=drive-$drive_id,id=$drive_id";
+ if (!min_version($machine_version, 10, 0) || $drive->{file} ne 'none') {
+ $device .= ",drive=drive-$drive_id";
+ }
+ $device .= ",id=$drive_id";
if ($device_type eq 'hd') {
if (my $model = $drive->{model}) {
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 29/32] drive: create a throttle group for each drive starting with machine version 10.0
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (27 preceding siblings ...)
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 28/32] print drive device: don't reference any drive for 'none' " Fiona Ebner
@ 2025-06-18 13:02 ` Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 30/32] blockdev: add helpers to generate blockdev commandline Fiona Ebner
` (3 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:02 UTC (permalink / raw)
To: pve-devel
The throttle group will be referenced via the 'blockdev' schema.
Co-developed-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Changes in v2:
* Drop unused get_drive_id() call.
* Fix quoting ID string for drivedel, I had accidentally used single
quotes there leading to variable not getting expanded.
* Comment that the version guard is for the switch to -blockdev.
src/PVE/QemuServer.pm | 52 ++++++++++++++++++++++++++++++++++
src/PVE/QemuServer/Blockdev.pm | 44 ++++++++++++++++++++++++++++
src/PVE/QemuServer/Makefile | 1 +
3 files changed, 97 insertions(+)
create mode 100644 src/PVE/QemuServer/Blockdev.pm
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 4eccfb62..1b8d42a3 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -51,6 +51,7 @@ use PVE::Tools
use PVE::QMPClient;
use PVE::QemuConfig;
use PVE::QemuConfig::NoWrite;
+use PVE::QemuServer::Blockdev;
use PVE::QemuServer::Helpers qw(config_aware_timeout min_version kvm_user_version windows_version);
use PVE::QemuServer::Cloudinit;
use PVE::QemuServer::CGroup;
@@ -4100,6 +4101,11 @@ sub config_to_command {
push @$devices, '-blockdev', $live_restore->{blockdev};
}
+ if (min_version($machine_version, 10, 0)) { # for the switch to -blockdev
+ my $throttle_group = PVE::QemuServer::Blockdev::generate_throttle_group($drive);
+ push @$cmd, '-object', to_json($throttle_group, { canonical => 1 });
+ }
+
my $drive_cmd =
print_drive_commandline_full($storecfg, $vmid, $drive, $live_blockdev_name);
@@ -4510,6 +4516,14 @@ sub qemu_iothread_del {
sub qemu_driveadd {
my ($storecfg, $vmid, $device) = @_;
+ my $machine_type = PVE::QemuServer::Machine::get_current_qemu_machine($vmid);
+
+ # for the switch to -blockdev
+ if (PVE::QemuServer::Machine::is_machine_version_at_least($machine_type, 10, 0)) {
+ my $throttle_group = PVE::QemuServer::Blockdev::generate_throttle_group($device);
+ mon_cmd($vmid, 'object-add', %$throttle_group);
+ }
+
my $drive = print_drive_commandline_full($storecfg, $vmid, $device, undef);
$drive =~ s/\\/\\\\/g;
my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_add auto \"$drive\"", 60);
@@ -4523,6 +4537,13 @@ sub qemu_driveadd {
sub qemu_drivedel {
my ($vmid, $deviceid) = @_;
+ my $machine_type = PVE::QemuServer::Machine::get_current_qemu_machine($vmid);
+
+ # for the switch to -blockdev
+ if (PVE::QemuServer::Machine::is_machine_version_at_least($machine_type, 10, 0)) {
+ mon_cmd($vmid, 'object-del', id => "throttle-drive-$deviceid");
+ }
+
my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_del drive-$deviceid", 10 * 60);
$ret =~ s/^\s+//;
@@ -4788,6 +4809,37 @@ sub qemu_block_set_io_throttle {
return if !check_running($vmid);
+ my $machine_type = PVE::QemuServer::Machine::get_current_qemu_machine($vmid);
+ # for the switch to -blockdev
+ if (PVE::QemuServer::Machine::is_machine_version_at_least($machine_type, 10, 0)) {
+ mon_cmd(
+ $vmid,
+ 'qom-set',
+ path => "throttle-$deviceid",
+ property => "limits",
+ value => {
+ 'bps-total' => int($bps),
+ 'bps-read' => int($bps_rd),
+ 'bps-write' => int($bps_wr),
+ 'iops-total' => int($iops),
+ 'iops-read' => int($iops_rd),
+ 'iops-write' => int($iops_wr),
+ 'bps-total-max' => int($bps_max),
+ 'bps-read-max' => int($bps_rd_max),
+ 'bps-write-max' => int($bps_wr_max),
+ 'iops-total-max' => int($iops_max),
+ 'iops-read-max' => int($iops_rd_max),
+ 'iops-write-max' => int($iops_wr_max),
+ 'bps-total-max-length' => int($bps_max_length),
+ 'bps-read-max-length' => int($bps_rd_max_length),
+ 'bps-write-max-length' => int($bps_wr_max_length),
+ 'iops-total-max-length' => int($iops_max_length),
+ 'iops-read-max-length' => int($iops_rd_max_length),
+ 'iops-write-max-length' => int($iops_wr_max_length),
+ },
+ );
+ }
+
mon_cmd(
$vmid, "block_set_io_throttle",
device => $deviceid,
diff --git a/src/PVE/QemuServer/Blockdev.pm b/src/PVE/QemuServer/Blockdev.pm
new file mode 100644
index 00000000..4d1266b8
--- /dev/null
+++ b/src/PVE/QemuServer/Blockdev.pm
@@ -0,0 +1,44 @@
+package PVE::QemuServer::Blockdev;
+
+use strict;
+use warnings;
+
+use PVE::QemuServer::Drive;
+
+sub generate_throttle_group {
+ my ($drive) = @_;
+
+ my $drive_id = PVE::QemuServer::Drive::get_drive_id($drive);
+
+ my $limits = {};
+
+ for my $type (['', '-total'], [_rd => '-read'], [_wr => '-write']) {
+ my ($dir, $qmpname) = @$type;
+ if (my $v = $drive->{"mbps$dir"}) {
+ $limits->{"bps$qmpname"} = int($v * 1024 * 1024);
+ }
+ if (my $v = $drive->{"mbps${dir}_max"}) {
+ $limits->{"bps$qmpname-max"} = int($v * 1024 * 1024);
+ }
+ if (my $v = $drive->{"bps${dir}_max_length"}) {
+ $limits->{"bps$qmpname-max-length"} = int($v);
+ }
+ if (my $v = $drive->{"iops${dir}"}) {
+ $limits->{"iops$qmpname"} = int($v);
+ }
+ if (my $v = $drive->{"iops${dir}_max"}) {
+ $limits->{"iops$qmpname-max"} = int($v);
+ }
+ if (my $v = $drive->{"iops${dir}_max_length"}) {
+ $limits->{"iops$qmpname-max-length"} = int($v);
+ }
+ }
+
+ return {
+ id => "throttle-drive-$drive_id",
+ limits => $limits,
+ 'qom-type' => 'throttle-group',
+ };
+}
+
+1;
diff --git a/src/PVE/QemuServer/Makefile b/src/PVE/QemuServer/Makefile
index 0d09b763..7d3830de 100644
--- a/src/PVE/QemuServer/Makefile
+++ b/src/PVE/QemuServer/Makefile
@@ -3,6 +3,7 @@ PREFIX=/usr
PERLDIR=$(PREFIX)/share/perl5
SOURCES=Agent.pm \
+ Blockdev.pm \
CGroup.pm \
Cloudinit.pm \
CPUConfig.pm \
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [PATCH qemu-server v2 30/32] blockdev: add helpers to generate blockdev commandline
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (28 preceding siblings ...)
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 29/32] drive: create a throttle group for each drive " Fiona Ebner
@ 2025-06-18 13:02 ` Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [RFC qemu-server v2 31/32] blockdev: add support for NBD paths Fiona Ebner
` (2 subsequent siblings)
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:02 UTC (permalink / raw)
To: pve-devel
The drive device and node structure is:
front-end device {ide-hd,scsi-hd,virtio-blk-pci} (id=$drive_id)
- throttle node (node-name=$drive_id)
- format node (node-name=f$encoded-info)
- file node (node-name=e$encoded-info)
The node-name can only be 31 characters long and needs to start with a
letter. The throttle node will stay inserted below the front-end
device. The other nodes might change however, because of drive
mirroring and similar. There currently are no good helpers to
query/walk the block graph via QMP, x-debug-query-block-graph is
experimental and for debugging only. Therefore, necessary information
is encoded in the node name to be able to find it again. In
particular, this is the volume ID, the drive ID and optionally a
snapshot name. As long as the configuration file matches with the
running instance, this is enough to find the correct node for
block operations like mirror and resize.
The 'snapshot' option, for QEMU's snapshot mode, i.e. writes are only
temporary, is not yet supported.
Originally-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
[FE: split up patch
expand commit message
explicitly test for drivers with aio setting
improve readonly handling
improve CD-ROM handling
fix failure for storage named 'nbd' by always using full regex
improve node name generation
fail when drive->{snapshot} is set]
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Changes in v2:
* Use json_bool helper.
* Coerce some additional strings by quoting for JSON, just to be sure.
src/PVE/QemuServer/Blockdev.pm | 171 ++++++++++++++++++++++++++++++++-
1 file changed, 170 insertions(+), 1 deletion(-)
diff --git a/src/PVE/QemuServer/Blockdev.pm b/src/PVE/QemuServer/Blockdev.pm
index 4d1266b8..76a00383 100644
--- a/src/PVE/QemuServer/Blockdev.pm
+++ b/src/PVE/QemuServer/Blockdev.pm
@@ -3,7 +3,41 @@ package PVE::QemuServer::Blockdev;
use strict;
use warnings;
-use PVE::QemuServer::Drive;
+use Digest::SHA;
+use Fcntl qw(S_ISBLK S_ISCHR);
+use File::stat;
+
+use PVE::JSONSchema qw(json_bool);
+use PVE::Storage;
+
+use PVE::QemuServer::Drive qw(drive_is_cdrom);
+
+my sub get_node_name {
+ my ($type, $drive_id, $volid, $snap) = @_;
+
+ my $info = "drive=$drive_id,";
+ $info .= "snap=$snap," if defined($snap);
+ $info .= "volid=$volid";
+
+ my $hash = substr(Digest::SHA::sha256_hex($info), 0, 30);
+
+ my $prefix = "";
+ if ($type eq 'fmt') {
+ $prefix = 'f';
+ } elsif ($type eq 'file') {
+ $prefix = 'e';
+ } else {
+ die "unknown node type '$type'";
+ }
+ # node-name must start with an alphabetical character
+ return "${prefix}${hash}";
+}
+
+my sub read_only_json_option {
+ my ($drive, $options) = @_;
+
+ return json_bool($drive->{ro} || drive_is_cdrom($drive) || $options->{'read-only'});
+}
sub generate_throttle_group {
my ($drive) = @_;
@@ -41,4 +75,139 @@ sub generate_throttle_group {
};
}
+sub generate_blockdev_drive_cache {
+ my ($drive, $scfg) = @_;
+
+ my $cache_direct = PVE::QemuServer::Drive::drive_uses_cache_direct($drive, $scfg);
+ return {
+ direct => json_bool($cache_direct),
+ 'no-flush' => json_bool($drive->{cache} && $drive->{cache} eq 'unsafe'),
+ };
+}
+
+sub generate_file_blockdev {
+ my ($storecfg, $drive, $options) = @_;
+
+ my $blockdev = {};
+ my $scfg = undef;
+
+ die "generate_file_blockdev called without volid/path\n" if !$drive->{file};
+ die "generate_file_blockdev called with 'none'\n" if $drive->{file} eq 'none';
+ # FIXME use overlay and new config option to define storage for temp write device
+ die "'snapshot' option is not yet supported for '-blockdev'\n" if $drive->{snapshot};
+
+ my $drive_id = PVE::QemuServer::Drive::get_drive_id($drive);
+
+ if ($drive->{file} eq 'cdrom') {
+ my $path = PVE::QemuServer::Drive::get_iso_path($storecfg, $drive->{file});
+ $blockdev = { driver => 'host_cdrom', filename => "$path" };
+ } elsif ($drive->{file} =~ m|^/|) {
+ my $path = $drive->{file};
+ # The 'file' driver only works for regular files. The check below is taken from
+ # block/file-posix.c:hdev_probe_device() in QEMU. To detect CD-ROM host devices, QEMU issues
+ # an ioctl, while the code here relies on the media=cdrom flag instead.
+ my $st = File::stat::stat($path) or die "stat for '$path' failed - $!\n";
+ my $driver = 'file';
+ if (S_ISCHR($st->mode) || S_ISBLK($st->mode)) {
+ $driver = drive_is_cdrom($drive) ? 'host_cdrom' : 'host_device';
+ }
+ $blockdev = { driver => "$driver", filename => "$path" };
+ } else {
+ my $volid = $drive->{file};
+ my ($storeid) = PVE::Storage::parse_volume_id($volid);
+
+ my $vtype = (PVE::Storage::parse_volname($storecfg, $drive->{file}))[0];
+ die "$drive_id: explicit media parameter is required for iso images\n"
+ if !defined($drive->{media}) && defined($vtype) && $vtype eq 'iso';
+
+ my $storage_opts = { hints => {} };
+ $storage_opts->{hints}->{'efi-disk'} = 1 if $drive->{interface} eq 'efidisk';
+ $storage_opts->{'snapshot-name'} = $options->{'snapshot-name'}
+ if defined($options->{'snapshot-name'});
+ $blockdev = PVE::Storage::qemu_blockdev_options($storecfg, $volid, $storage_opts);
+ $scfg = PVE::Storage::storage_config($storecfg, $storeid);
+ }
+
+ $blockdev->{cache} = generate_blockdev_drive_cache($drive, $scfg);
+
+ my $driver = $blockdev->{driver};
+ # only certain drivers have the aio setting
+ if ($driver eq 'file' || $driver eq 'host_cdrom' || $driver eq 'host_device') {
+ $blockdev->{aio} =
+ PVE::QemuServer::Drive::aio_cmdline_option($scfg, $drive, $blockdev->{cache}->{direct});
+ }
+
+ if (!drive_is_cdrom($drive)) {
+ $blockdev->{discard} = $drive->{discard} && $drive->{discard} eq 'on' ? 'unmap' : 'ignore';
+ $blockdev->{'detect-zeroes'} = PVE::QemuServer::Drive::detect_zeroes_cmdline_option($drive);
+ }
+
+ $blockdev->{'node-name'} =
+ get_node_name('file', $drive_id, $drive->{file}, $options->{'snapshot-name'});
+
+ $blockdev->{'read-only'} = read_only_json_option($drive, $options);
+
+ return $blockdev;
+}
+
+sub generate_format_blockdev {
+ my ($storecfg, $drive, $child, $options) = @_;
+
+ die "generate_format_blockdev called without volid/path\n" if !$drive->{file};
+ die "generate_format_blockdev called with 'none'\n" if $drive->{file} eq 'none';
+
+ my $scfg;
+ my $format;
+ my $volid = $drive->{file};
+ my $drive_id = PVE::QemuServer::Drive::get_drive_id($drive);
+ my ($storeid) = PVE::Storage::parse_volume_id($volid, 1);
+
+ # For PVE-managed volumes, use the format from the storage layer and prevent overrides via the
+ # drive's 'format' option. For unmanaged volumes, fallback to 'raw' to avoid auto-detection by
+ # QEMU.
+ if ($storeid) {
+ $scfg = PVE::Storage::storage_config($storecfg, $storeid);
+ $format = PVE::QemuServer::Drive::checked_volume_format($storecfg, $volid);
+ if ($drive->{format} && $drive->{format} ne $format) {
+ die "drive '$drive->{interface}$drive->{index}' - volume '$volid'"
+ . " - 'format=$drive->{format}' option different from storage format '$format'\n";
+ }
+ } else {
+ $format = $drive->{format} // 'raw';
+ }
+
+ # define cache option on both format && file node like libvirt does
+ my $cache = generate_blockdev_drive_cache($drive, $scfg);
+
+ my $node_name = get_node_name('fmt', $drive_id, $drive->{file}, $options->{'snapshot-name'});
+
+ return {
+ 'node-name' => "$node_name",
+ driver => "$format",
+ file => $child,
+ cache => $cache,
+ 'read-only' => read_only_json_option($drive, $options),
+ };
+}
+
+sub generate_drive_blockdev {
+ my ($storecfg, $drive, $options) = @_;
+
+ my $drive_id = PVE::QemuServer::Drive::get_drive_id($drive);
+
+ die "generate_drive_blockdev called without volid/path\n" if !$drive->{file};
+ die "generate_drive_blockdev called with 'none'\n" if $drive->{file} eq 'none';
+
+ my $child = generate_file_blockdev($storecfg, $drive, $options);
+ $child = generate_format_blockdev($storecfg, $drive, $child, $options);
+
+ # this is the top filter entry point, use $drive-drive_id as nodename
+ return {
+ driver => "throttle",
+ 'node-name' => "drive-$drive_id",
+ 'throttle-group' => "throttle-drive-$drive_id",
+ file => $child,
+ };
+}
+
1;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [RFC qemu-server v2 31/32] blockdev: add support for NBD paths
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (29 preceding siblings ...)
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 30/32] blockdev: add helpers to generate blockdev commandline Fiona Ebner
@ 2025-06-18 13:02 ` Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [RFC qemu-server v2 32/32] command line: switch to blockdev starting with machine version 10.0 Fiona Ebner
2025-06-20 13:03 ` [pve-devel] partially-applied: [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fabian Grünbichler
32 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:02 UTC (permalink / raw)
To: pve-devel
Co-developed-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
RFC, still didn't come around to testing it (will be done with
blockdev-mirror).
Changes in v2:
* Also support unix socket NBD paths.
src/PVE/QemuServer/Blockdev.pm | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/src/PVE/QemuServer/Blockdev.pm b/src/PVE/QemuServer/Blockdev.pm
index 76a00383..75fe40ec 100644
--- a/src/PVE/QemuServer/Blockdev.pm
+++ b/src/PVE/QemuServer/Blockdev.pm
@@ -12,6 +12,18 @@ use PVE::Storage;
use PVE::QemuServer::Drive qw(drive_is_cdrom);
+# gives ($host, $port, $export)
+my $NBD_TCP_PATH_RE_3 = qr/nbd:(\S+):(\d+):exportname=(\S+)/;
+my $NBD_UNIX_PATH_RE_2 = qr/nbd:unix:(\S+):exportname=(\S+)/;
+
+my sub is_nbd {
+ my ($drive) = @_;
+
+ return 1 if $drive->{file} =~ $NBD_TCP_PATH_RE_3;
+ return 1 if $drive->{file} =~ $NBD_UNIX_PATH_RE_2;
+ return 0;
+}
+
my sub get_node_name {
my ($type, $drive_id, $volid, $snap) = @_;
@@ -98,7 +110,13 @@ sub generate_file_blockdev {
my $drive_id = PVE::QemuServer::Drive::get_drive_id($drive);
- if ($drive->{file} eq 'cdrom') {
+ if ($drive->{file} =~ m/^$NBD_UNIX_PATH_RE_2$/) {
+ my $server = { type => 'unix', path => "$1" };
+ $blockdev = { driver => 'nbd', server => $server, export => "$2" };
+ } elsif ($drive->{file} =~ m/^$NBD_TCP_PATH_RE_3$/) {
+ my $server = { type => 'inet', host => "$1", port => "$2" }; # port is also a string in QAPI
+ $blockdev = { driver => 'nbd', server => $server, export => "$3" };
+ } elsif ($drive->{file} eq 'cdrom') {
my $path = PVE::QemuServer::Drive::get_iso_path($storecfg, $drive->{file});
$blockdev = { driver => 'host_cdrom', filename => "$path" };
} elsif ($drive->{file} =~ m|^/|) {
@@ -155,6 +173,7 @@ sub generate_format_blockdev {
die "generate_format_blockdev called without volid/path\n" if !$drive->{file};
die "generate_format_blockdev called with 'none'\n" if $drive->{file} eq 'none';
+ die "generate_format_blockdev called with NBD path\n" if is_nbd($drive);
my $scfg;
my $format;
@@ -199,7 +218,9 @@ sub generate_drive_blockdev {
die "generate_drive_blockdev called with 'none'\n" if $drive->{file} eq 'none';
my $child = generate_file_blockdev($storecfg, $drive, $options);
- $child = generate_format_blockdev($storecfg, $drive, $child, $options);
+ if (!is_nbd($drive)) {
+ $child = generate_format_blockdev($storecfg, $drive, $child, $options);
+ }
# this is the top filter entry point, use $drive-drive_id as nodename
return {
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] [RFC qemu-server v2 32/32] command line: switch to blockdev starting with machine version 10.0
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (30 preceding siblings ...)
2025-06-18 13:02 ` [pve-devel] [RFC qemu-server v2 31/32] blockdev: add support for NBD paths Fiona Ebner
@ 2025-06-18 13:02 ` Fiona Ebner
2025-06-23 9:12 ` DERUMIER, Alexandre via pve-devel
2025-06-20 13:03 ` [pve-devel] partially-applied: [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fabian Grünbichler
32 siblings, 1 reply; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 13:02 UTC (permalink / raw)
To: pve-devel
Co-developed-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
RFC, because this should come together with supporting all operations.
No changes in v2.
src/PVE/QemuServer.pm | 131 ++++++++++++++++++++++++++----------------
1 file changed, 81 insertions(+), 50 deletions(-)
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 1b8d42a3..b9d18899 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -4104,15 +4104,30 @@ sub config_to_command {
if (min_version($machine_version, 10, 0)) { # for the switch to -blockdev
my $throttle_group = PVE::QemuServer::Blockdev::generate_throttle_group($drive);
push @$cmd, '-object', to_json($throttle_group, { canonical => 1 });
+
+ die "FIXME: blockdev for live restore not yet implemented"
+ if $live_blockdev_name;
+
+ my $extra_blockdev_options = {};
+ # extra protection for templates, but SATA and IDE don't support it..
+ $extra_blockdev_options->{'read-only'} = 1 if drive_is_read_only($conf, $drive);
+
+ if ($drive->{file} ne 'none') {
+ my $blockdev = PVE::QemuServer::Blockdev::generate_drive_blockdev(
+ $storecfg, $drive, $extra_blockdev_options,
+ );
+ push @$devices, '-blockdev', to_json($blockdev, { canonical => 1 });
+ }
+ } else {
+ my $drive_cmd =
+ print_drive_commandline_full($storecfg, $vmid, $drive, $live_blockdev_name);
+
+ # extra protection for templates, but SATA and IDE don't support it..
+ $drive_cmd .= ',readonly=on' if drive_is_read_only($conf, $drive);
+
+ push @$devices, '-drive', $drive_cmd;
}
- my $drive_cmd =
- print_drive_commandline_full($storecfg, $vmid, $drive, $live_blockdev_name);
-
- # extra protection for templates, but SATA and IDE don't support it..
- $drive_cmd .= ',readonly=on' if drive_is_read_only($conf, $drive);
-
- push @$devices, '-drive', $drive_cmd;
push @$devices, '-device',
print_drivedevice_full(
$storecfg, $conf, $vmid, $drive, $bridges, $arch, $machine_type,
@@ -4522,16 +4537,21 @@ sub qemu_driveadd {
if (PVE::QemuServer::Machine::is_machine_version_at_least($machine_type, 10, 0)) {
my $throttle_group = PVE::QemuServer::Blockdev::generate_throttle_group($device);
mon_cmd($vmid, 'object-add', %$throttle_group);
+
+ my $blockdev = PVE::QemuServer::Blockdev::generate_drive_blockdev($storecfg, $device, {});
+ mon_cmd($vmid, 'blockdev-add', %$blockdev, timeout => 60);
+
+ return 1;
+ } else {
+ my $drive = print_drive_commandline_full($storecfg, $vmid, $device, undef);
+ $drive =~ s/\\/\\\\/g;
+ my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_add auto \"$drive\"", 60);
+
+ # If the command succeeds qemu prints: "OK"
+ return 1 if $ret =~ m/OK/s;
+
+ die "adding drive failed: $ret\n";
}
-
- my $drive = print_drive_commandline_full($storecfg, $vmid, $device, undef);
- $drive =~ s/\\/\\\\/g;
- my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_add auto \"$drive\"", 60);
-
- # If the command succeeds qemu prints: "OK"
- return 1 if $ret =~ m/OK/s;
-
- die "adding drive failed: $ret\n";
}
sub qemu_drivedel {
@@ -4541,18 +4561,30 @@ sub qemu_drivedel {
# for the switch to -blockdev
if (PVE::QemuServer::Machine::is_machine_version_at_least($machine_type, 10, 0)) {
+ # QEMU recursively auto-removes the file children, i.e. file and format node below the top
+ # node and also implicit backing children referenced by a qcow2 image.
+ eval {
+ mon_cmd(
+ $vmid, 'blockdev-del',
+ 'node-name' => "drive-$deviceid",
+ timeout => 10 * 60,
+ );
+ };
+ die "deleting blockdev $deviceid failed : $@\n" if $@;
+ # FIXME ignore already removed scenario like below?
+
mon_cmd($vmid, 'object-del', id => "throttle-drive-$deviceid");
+ } else {
+ my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_del drive-$deviceid", 10 * 60);
+ $ret =~ s/^\s+//;
+
+ return 1 if $ret eq "";
+
+ # NB: device not found errors mean the drive was auto-deleted and we ignore the error
+ return 1 if $ret =~ m/Device \'.*?\' not found/s;
+
+ die "deleting drive $deviceid failed : $ret\n";
}
-
- my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_del drive-$deviceid", 10 * 60);
- $ret =~ s/^\s+//;
-
- return 1 if $ret eq "";
-
- # NB: device not found errors mean the drive was auto-deleted and we ignore the error
- return 1 if $ret =~ m/Device \'.*?\' not found/s;
-
- die "deleting drive $deviceid failed : $ret\n";
}
sub qemu_deviceaddverify {
@@ -4838,31 +4870,30 @@ sub qemu_block_set_io_throttle {
'iops-write-max-length' => int($iops_wr_max_length),
},
);
+ } else {
+ mon_cmd(
+ $vmid, "block_set_io_throttle",
+ device => $deviceid,
+ bps => int($bps),
+ bps_rd => int($bps_rd),
+ bps_wr => int($bps_wr),
+ iops => int($iops),
+ iops_rd => int($iops_rd),
+ iops_wr => int($iops_wr),
+ bps_max => int($bps_max),
+ bps_rd_max => int($bps_rd_max),
+ bps_wr_max => int($bps_wr_max),
+ iops_max => int($iops_max),
+ iops_rd_max => int($iops_rd_max),
+ iops_wr_max => int($iops_wr_max),
+ bps_max_length => int($bps_max_length),
+ bps_rd_max_length => int($bps_rd_max_length),
+ bps_wr_max_length => int($bps_wr_max_length),
+ iops_max_length => int($iops_max_length),
+ iops_rd_max_length => int($iops_rd_max_length),
+ iops_wr_max_length => int($iops_wr_max_length),
+ );
}
-
- mon_cmd(
- $vmid, "block_set_io_throttle",
- device => $deviceid,
- bps => int($bps),
- bps_rd => int($bps_rd),
- bps_wr => int($bps_wr),
- iops => int($iops),
- iops_rd => int($iops_rd),
- iops_wr => int($iops_wr),
- bps_max => int($bps_max),
- bps_rd_max => int($bps_rd_max),
- bps_wr_max => int($bps_wr_max),
- iops_max => int($iops_max),
- iops_rd_max => int($iops_rd_max),
- iops_wr_max => int($iops_wr_max),
- bps_max_length => int($bps_max_length),
- bps_rd_max_length => int($bps_rd_max_length),
- bps_wr_max_length => int($bps_wr_max_length),
- iops_max_length => int($iops_max_length),
- iops_rd_max_length => int($iops_rd_max_length),
- iops_wr_max_length => int($iops_wr_max_length),
- );
-
}
sub qemu_block_resize {
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [pve-devel] [PATCH qemu-server v2 22/32] pci: code cleanup: remove superfluous machine type paramater from print_pci_addr
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 22/32] pci: code cleanup: remove superfluous machine type paramater from print_pci_addr Fiona Ebner
@ 2025-06-18 15:19 ` Fiona Ebner
0 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-18 15:19 UTC (permalink / raw)
To: pve-devel
Typo paramater -> parameter in subject
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [pve-devel] [PATCH common v2 01/32] schema: parse property string: support skipping keys
2025-06-18 13:01 ` [pve-devel] [PATCH common v2 01/32] schema: parse property string: support skipping keys Fiona Ebner
@ 2025-06-20 11:00 ` Fabian Grünbichler
0 siblings, 0 replies; 42+ messages in thread
From: Fabian Grünbichler @ 2025-06-20 11:00 UTC (permalink / raw)
To: Proxmox VE development discussion
On June 18, 2025 3:01 pm, Fiona Ebner wrote:
> In certain situations like restoring a backup, it can be useful to
> skip certain properties. This allows to drop outdated properties from
> the schema while still being able to parse property strings that
> contain them, but without allowing all additional properties.
>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>
> New in v2.
>
> src/PVE/JSONSchema.pm | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
> index 8c90986..e617fc1 100644
> --- a/src/PVE/JSONSchema.pm
> +++ b/src/PVE/JSONSchema.pm
> @@ -1033,7 +1033,7 @@ sub parse_boolean {
> }
>
> sub parse_property_string {
> - my ($format, $data, $path, $additional_properties) = @_;
> + my ($format, $data, $path, $additional_properties, $options) = @_;
>
> # In property strings we default to not allowing additional properties
> $additional_properties = 0 if !defined($additional_properties);
> @@ -1057,6 +1057,7 @@ sub parse_property_string {
> }
>
> my $default_key;
> + my $skip = $options->{skip} ? { map { $_ => 1 } $options->{skip}->@* } : {};
>
> my $res = {};
> foreach my $part (split(/,/, $data)) {
> @@ -1064,6 +1065,7 @@ sub parse_property_string {
>
> if ($part =~ /^([^=]+)=(.+)$/) {
> my ($k, $v) = ($1, $2);
> + next if $skip->{$k};
should we log this? chances are this is a one-time operation to make an
old config compatible with a new schema without dropping whole property
strings, so it might be nice to inform the user that this happened, just
like we do if we drop the whole line?
> die "duplicate key in comma-separated list property: $k\n" if defined($res->{$k});
> my $schema = $format->{$k};
> if (my $alias = $schema->{alias}) {
> --
> 2.39.5
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [pve-devel] [PATCH qemu-server v2 08/32] drive: remove geometry options gone since QEMU 3.1
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 08/32] drive: remove geometry options gone since QEMU 3.1 Fiona Ebner
@ 2025-06-20 11:03 ` Fabian Grünbichler
2025-06-20 11:20 ` Fiona Ebner
0 siblings, 1 reply; 42+ messages in thread
From: Fabian Grünbichler @ 2025-06-20 11:03 UTC (permalink / raw)
To: Proxmox VE development discussion
On June 18, 2025 3:01 pm, Fiona Ebner wrote:
> It was not possible to start a QEMU instance with these options set
> since QEMU version 3.1, QEMU commit b24ec3c462 ("block: Remove
> deprecated -drive geometry options") and thus also not to take a
> backup. It is still possible to restore an old backup with these
> options set.
we could do this unconditionally on parsing a drive line, it doesn't
really matter whether the context is regular parsing or restore parsing,
it's better to just drop the offending properties rather than the whole
drive in both cases.
while it is not very likely such old configs are still around, it would
also allow us to drop the previous two patches and keep the interface
simpler..
unrelated to this series and thus not high priority - it might be nice
to think whether we have other properties that we could drop from the
schema(s) for 9.0 thanks to this new option..
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>
> Changes in v2:
> * Different approach, skip dropped keys, rather than allowing all
> additional properties.
>
> src/PVE/QemuServer.pm | 2 +-
> src/PVE/QemuServer/Drive.pm | 28 +++-------------------------
> 2 files changed, 4 insertions(+), 26 deletions(-)
>
> diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
> index cfeb8949..95a84c56 100644
> --- a/src/PVE/QemuServer.pm
> +++ b/src/PVE/QemuServer.pm
> @@ -1543,7 +1543,7 @@ sub print_drive_commandline_full {
> my $is_rbd = $path =~ m/^rbd:/;
>
> my $opts = '';
> - my @qemu_drive_options = qw(heads secs cyls trans media cache rerror werror aio discard);
> + my @qemu_drive_options = qw(media cache rerror werror aio discard);
> foreach my $o (@qemu_drive_options) {
> $opts .= ",$o=$drive->{$o}" if defined($drive->{$o});
> }
> diff --git a/src/PVE/QemuServer/Drive.pm b/src/PVE/QemuServer/Drive.pm
> index 0e99257b..96bb12c4 100644
> --- a/src/PVE/QemuServer/Drive.pm
> +++ b/src/PVE/QemuServer/Drive.pm
> @@ -26,7 +26,7 @@ our @EXPORT_OK = qw(
> print_drive
> );
>
> -my $DROPPED_PROPERTIES = [];
> +my $DROPPED_PROPERTIES = ['cyls', 'heads', 'secs', 'trans'];
>
> our $QEMU_FORMAT_RE = qr/raw|qcow|qcow2|qed|vmdk|cloop/;
>
> @@ -176,27 +176,6 @@ my %drivedesc_base = (
> default => 'disk',
> optional => 1,
> },
> - cyls => {
> - type => 'integer',
> - description => "Force the drive's physical geometry to have a specific cylinder count.",
> - optional => 1,
> - },
> - heads => {
> - type => 'integer',
> - description => "Force the drive's physical geometry to have a specific head count.",
> - optional => 1,
> - },
> - secs => {
> - type => 'integer',
> - description => "Force the drive's physical geometry to have a specific sector count.",
> - optional => 1,
> - },
> - trans => {
> - type => 'string',
> - enum => [qw(none lba auto)],
> - description => "Force disk geometry bios translation mode.",
> - optional => 1,
> - },
> snapshot => {
> type => 'boolean',
> description => "Controls qemu's snapshot mode feature."
> @@ -765,7 +744,7 @@ sub drive_is_read_only {
> return $drive->{interface} ne 'sata' && $drive->{interface} ne 'ide';
> }
>
> -# ideX = [volume=]volume-id[,media=d][,cyls=c,heads=h,secs=s[,trans=t]]
> +# ideX = [volume=]volume-id[,media=d]
> # [,snapshot=on|off][,cache=on|off][,format=f][,backup=yes|no]
> # [,rerror=ignore|report|stop][,werror=enospc|ignore|report|stop]
> # [,aio=native|threads][,discard=ignore|on][,detect_zeroes=on|off]
> @@ -843,8 +822,7 @@ sub parse_drive {
> return if $res->{iops_wr} && $res->{iops};
>
> if ($res->{media} && ($res->{media} eq 'cdrom')) {
> - return if $res->{snapshot} || $res->{trans} || $res->{format};
> - return if $res->{heads} || $res->{secs} || $res->{cyls};
> + return if $res->{snapshot} || $res->{format};
> return if $res->{interface} eq 'virtio';
> }
>
> --
> 2.39.5
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [pve-devel] [PATCH qemu-server v2 08/32] drive: remove geometry options gone since QEMU 3.1
2025-06-20 11:03 ` Fabian Grünbichler
@ 2025-06-20 11:20 ` Fiona Ebner
0 siblings, 0 replies; 42+ messages in thread
From: Fiona Ebner @ 2025-06-20 11:20 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
Am 20.06.25 um 13:03 schrieb Fabian Grünbichler:
> On June 18, 2025 3:01 pm, Fiona Ebner wrote:
>> It was not possible to start a QEMU instance with these options set
>> since QEMU version 3.1, QEMU commit b24ec3c462 ("block: Remove
>> deprecated -drive geometry options") and thus also not to take a
>> backup. It is still possible to restore an old backup with these
>> options set.
>
> we could do this unconditionally on parsing a drive line, it doesn't
> really matter whether the context is regular parsing or restore parsing,
> it's better to just drop the offending properties rather than the whole
> drive in both cases.
I didn't want to make it accidentally possible to still write e.g. qm
set 123 --scsi0 dir:1,cyls=3 after this. But actually, this is caught
already earlier even if parse_drive() would support it. And yes, not
being quite like you suggested in the other reply will help too!
> while it is not very likely such old configs are still around, it would
> also allow us to drop the previous two patches and keep the interface
> simpler..
Ack.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [pve-devel] [PATCH qemu-server v2 18/32] vm start/commandline: activate volumes before config_to_command()
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 18/32] vm start/commandline: activate volumes before config_to_command() Fiona Ebner
@ 2025-06-20 11:33 ` Fabian Grünbichler
0 siblings, 0 replies; 42+ messages in thread
From: Fabian Grünbichler @ 2025-06-20 11:33 UTC (permalink / raw)
To: Proxmox VE development discussion
On June 18, 2025 3:01 pm, Fiona Ebner wrote:
> With '-blockdev', it is necessary to activate the volumes to generate
> the command line, because it can be necessary to check whether the
> volume is a block device or a regular file.
>
> Do not deactivate after commandline generation for 'qm showcmd',
> because there can be concurrent operations acting on the volumes.
>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>
> Changes in v2:
> * Do not deactivate after showcmd, because it might not be safe.
>
> src/PVE/QemuServer.pm | 60 +++++++++++++++++++++-------
> src/test/run_config2command_tests.pl | 10 +++++
> 2 files changed, 55 insertions(+), 15 deletions(-)
>
> diff --git a/src/test/run_config2command_tests.pl b/src/test/run_config2command_tests.pl
> index 590f80f5..cbd8cd8b 100755
> --- a/src/test/run_config2command_tests.pl
> +++ b/src/test/run_config2command_tests.pl
> @@ -256,6 +256,16 @@ $qemu_server_module->mock(
> },
> );
>
> +my $storage_module = Test::MockModule->new("PVE::Storage");
> +$storage_module->mock(
> + activate_volumes => sub {
> + return;
> + },
> + deactivate_volumes => sub {
> + return;
> + },
> +);
> +
potential for some follow ups here - we could actually track what the
tests trigger here, and thus that
- only actively referenced volumes are activated, and unused volumes for
example are not
- backing chains are properly activated (once those are a thing)
- showcmd doesn't deactivate
- ..?
> my $zfsplugin_module = Test::MockModule->new("PVE::Storage::ZFSPlugin");
> $zfsplugin_module->mock(
> zfs_get_lu_name => sub {
> --
> 2.39.5
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [pve-devel] partially-applied: [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
` (31 preceding siblings ...)
2025-06-18 13:02 ` [pve-devel] [RFC qemu-server v2 32/32] command line: switch to blockdev starting with machine version 10.0 Fiona Ebner
@ 2025-06-20 13:03 ` Fabian Grünbichler
32 siblings, 0 replies; 42+ messages in thread
From: Fabian Grünbichler @ 2025-06-20 13:03 UTC (permalink / raw)
To: Proxmox VE development discussion
applied most of the patches, except for 6, 28/29 and 31/32.
6 is dropped altogether, the other 4 can be included in the switch to
blockdev to avoid affecting the block graph before that happens.
folded in a small fixup for 7/8, and committed the changed expected
output for tests.
thanks a lot for driving this forward!
On June 18, 2025 3:01 pm, Fiona Ebner wrote:
> Changes in v2 (I hope I remembered everything, because of the
> indentation changes, I couldn't use my usual git range-diff O:)):
>
> * Rebase + tidy all patches (this is not noted on individual patches).
> * Indicate that version guards are for the switch to -blockdev via
> comments.
> * Fix deleting throttle group, ID string was wrongly quoted.
> * For backup restore, only skip dropped keys when parsing drive
> property string, rather than allowing all additional properties.
> * Order Perl file names in Makefile.
> * Add patch to drop superfluous fallack for migration type.
> * For templates, keep using minimized config for timeout calculation.
> * Also support unix socket NBD paths.
> * Coerce some more strings by quoting for JSON, just to be sure.
> * Introduce json_bool() helper.
> The following (i.e. patches 19/32 to 24/32) are orthogonal:
> * Add patches for PCI module improvements.
> * Never reserve PCI usage for 'qm showcmd', add dedicated test for
> checking that reservation is respected.
>
> This is a preparatory series for the switch to -blockdev with Proxmox
> VE 9, QEMU machine version 10.0, based in part on Alexandre's series
> here [0].
>
> While the last patch actually does the switch, many operations are not
> yet supported. It is included to show what changes I made there. It
> should not yet be applied and supporting everything is the goal for a
> following series based on the rest of Alexandre's patches. It was time
> to send this however, as it's already gotten large enough (but most
> patches are quite small, so don't worry reviewers ;)).
>
> There are mostly cleanups, moving and defining helpers, as well as
> doing non-intrusive preparation for the switch with machine version
> 10.0. Quite a large chunk of the series is for having
> activate_volumes() be called before config_to_command() with proper
> cleanup handling.
>
> A breaking change is the removal of the gone-since-QEMU-3.1 drive
> geometry options. It was not possible to even start or backup a VM
> with those options set since then. Still, restore of old backups with
> such an option is made explicitly supported.
>
> NOTE: Dependency bump qemu-server -> libpve-common-perl needed.
>
> [0]: https://lore.proxmox.com/pve-devel/mailman.347.1749728041.395.pve-devel@lists.proxmox.com/T/
>
>
> common:
>
> Fiona Ebner (2):
> schema: parse property string: support skipping keys
> json schema: add helper to convert to JSON boolean
>
> src/PVE/JSONSchema.pm | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
>
> qemu-server:
>
> Alexandre Derumier via pve-devel (1):
> vm devices list: prepare querying block device names for -blockdev
>
> Fiona Ebner (29):
> buildsys: order Perl source files in QemuServer/Makefile
> drive: code cleanup: drop unused $vmid parameter from
> get_path_and_format()
> cfg2cmd: require at least QEMU binary version 6.0
> drive: parse: use hash argument for optional parameters
> drive: parse: properly handle dropped properties for restore
> drive: remove geometry options gone since QEMU 3.1
> clone disk: io uring check: fix call to determine cache direct
> drive: move storage_allows_io_uring_default() and
> drive_uses_cache_direct() helpers to drive module
> drive: introduce aio_cmdline_option() helper
> drive: introduce detect_zeroes_cmdline_option() helper
> vm start: assert that migration type is set for 'tcp' migration
> introduce StateFile module for state file related helpers
> vm start: move state file handling to dedicated module
> vm start: move config_to_command() call further down
> vm start/commandline: also clean up pci reservation when
> config_to_command() fails
> vm start/commandline: activate volumes before config_to_command()
> pci: add missing includes
> test: add tests for PCI reservations
> cfg2cmd: print vga: fix call to print_pcie_addr()
> pci: code cleanup: remove superfluous machine type paramater from
> print_pci_addr
> cfg2cmd: collect optional parameters as a hash
> qm: showcmd: never reserve PCI devices
> print drive device: explicitly set write-cache starting with machine
> version 10.0
> print drive device: set {r,w}error front-end properties starting with
> machine version 10.0
> print drive device: don't reference any drive for 'none' starting with
> machine version 10.0
> drive: create a throttle group for each drive starting with machine
> version 10.0
> blockdev: add helpers to generate blockdev commandline
> blockdev: add support for NBD paths
> command line: switch to blockdev starting with machine version 10.0
>
> src/PVE/API2/Qemu.pm | 15 +-
> src/PVE/QemuServer.pm | 605 +++++++++---------
> src/PVE/QemuServer/Blockdev.pm | 234 +++++++
> src/PVE/QemuServer/Drive.pm | 108 +++-
> src/PVE/QemuServer/Makefile | 26 +-
> src/PVE/QemuServer/PCI.pm | 30 +-
> src/PVE/QemuServer/RNG.pm | 4 +-
> src/PVE/QemuServer/StateFile.pm | 79 +++
> src/PVE/QemuServer/USB.pm | 8 +-
> src/test/Makefile | 5 +-
> src/test/cfg2cmd/aio.conf.cmd | 16 +-
> src/test/cfg2cmd/old-qemu.conf | 4 +-
> .../q35-linux-hostpci-mapping.conf.cmd | 2 +-
> src/test/run_config2command_tests.pl | 24 +-
> src/test/run_pci_reservation_tests.pl | 175 +++++
> 15 files changed, 950 insertions(+), 385 deletions(-)
> create mode 100644 src/PVE/QemuServer/Blockdev.pm
> create mode 100644 src/PVE/QemuServer/StateFile.pm
> create mode 100755 src/test/run_pci_reservation_tests.pl
>
>
> Summary over all repositories:
> 16 files changed, 960 insertions(+), 386 deletions(-)
>
> --
> Generated by git-murpp 0.5.0
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [pve-devel] [RFC qemu-server v2 32/32] command line: switch to blockdev starting with machine version 10.0
2025-06-18 13:02 ` [pve-devel] [RFC qemu-server v2 32/32] command line: switch to blockdev starting with machine version 10.0 Fiona Ebner
@ 2025-06-23 9:12 ` DERUMIER, Alexandre via pve-devel
2025-06-23 9:31 ` Fiona Ebner
0 siblings, 1 reply; 42+ messages in thread
From: DERUMIER, Alexandre via pve-devel @ 2025-06-23 9:12 UTC (permalink / raw)
To: pve-devel; +Cc: DERUMIER, Alexandre
[-- Attachment #1: Type: message/rfc822, Size: 12420 bytes --]
From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [RFC qemu-server v2 32/32] command line: switch to blockdev starting with machine version 10.0
Date: Mon, 23 Jun 2025 09:12:43 +0000
Message-ID: <d105eff412538c94deb41c93a30c0018d9d015c5.camel@groupe-cyllene.com>
>>+ my $blockdev =
>>PVE::QemuServer::Blockdev::generate_drive_blockdev($storecfg,
>>$device, {});
>>+ mon_cmd($vmid, 'blockdev-add', %$blockdev, timeout => 60);
>>+
>>+ return 1;
should we handle error here ? (I don't known if a blockdev-add can
fail , and if it need 60s timeout like drive-add, as I think that the
blockdev open is done in device-add)
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [pve-devel] [RFC qemu-server v2 32/32] command line: switch to blockdev starting with machine version 10.0
2025-06-23 9:12 ` DERUMIER, Alexandre via pve-devel
@ 2025-06-23 9:31 ` Fiona Ebner
2025-06-23 13:06 ` DERUMIER, Alexandre via pve-devel
0 siblings, 1 reply; 42+ messages in thread
From: Fiona Ebner @ 2025-06-23 9:31 UTC (permalink / raw)
To: Proxmox VE development discussion
Am 23.06.25 um 11:12 schrieb DERUMIER, Alexandre via pve-devel:
>>> + my $blockdev =
>>> PVE::QemuServer::Blockdev::generate_drive_blockdev($storecfg,
>>> $device, {});
>>> + mon_cmd($vmid, 'blockdev-add', %$blockdev, timeout => 60);
>>> +
>>> + return 1;
>
> should we handle error here ? (I don't known if a blockdev-add can
> fail , and if it need 60s timeout like drive-add, as I think that the
> blockdev open is done in device-add)
Yes, we could remove the throttle group again if it fails.
qmp_blockdev_add() calls bds_tree_init() which calls bdrv_open(). So I
think using 60 seconds here like for "drive_add" is a sensible choice.
Note that "device_add" currently only uses the default timeout of 5
seconds. I'm not aware about reports about failure there directly. I had
sent a patch to increase that a while ago [0] because of a report with
"netdev_add". But probably would be good to take in too.
[0]:
https://lore.proxmox.com/pve-devel/20241212100247.20926-1-f.ebner@proxmox.com/
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [pve-devel] [RFC qemu-server v2 32/32] command line: switch to blockdev starting with machine version 10.0
2025-06-23 9:31 ` Fiona Ebner
@ 2025-06-23 13:06 ` DERUMIER, Alexandre via pve-devel
0 siblings, 0 replies; 42+ messages in thread
From: DERUMIER, Alexandre via pve-devel @ 2025-06-23 13:06 UTC (permalink / raw)
To: pve-devel, f.ebner; +Cc: DERUMIER, Alexandre
[-- Attachment #1: Type: message/rfc822, Size: 14803 bytes --]
From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>, "f.ebner@proxmox.com" <f.ebner@proxmox.com>
Subject: Re: [pve-devel] [RFC qemu-server v2 32/32] command line: switch to blockdev starting with machine version 10.0
Date: Mon, 23 Jun 2025 13:06:54 +0000
Message-ID: <f53d6f29acfd8a5722825b7f448453577041a91d.camel@groupe-cyllene.com>
-------- Message initial --------
De: Fiona Ebner <f.ebner@proxmox.com>
À: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Cc: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
Objet: Re: [pve-devel] [RFC qemu-server v2 32/32] command line: switch
to blockdev starting with machine version 10.0
Date: 23/06/2025 11:31:02
Am 23.06.25 um 11:12 schrieb DERUMIER, Alexandre via pve-devel:
> > > + my $blockdev =
> > > PVE::QemuServer::Blockdev::generate_drive_blockdev($storecfg,
> > > $device, {});
> > > + mon_cmd($vmid, 'blockdev-add', %$blockdev, timeout =>
> > > 60);
> > > +
> > > + return 1;
>
> should we handle error here ? (I don't known if a blockdev-add can
> fail , and if it need 60s timeout like drive-add, as I think that the
> blockdev open is done in device-add)
Yes, we could remove the throttle group again if it fails.
>>qmp_blockdev_add() calls bds_tree_init() which calls bdrv_open(). So
>>I
>>think using 60 seconds here like for "drive_add" is a sensible
choice.
ah ok, indeed if the bdrv_open is done here (I was not sure), this make
sense
>>Note that "device_add" currently only uses the default timeout of 5
>>seconds. I'm not aware about reports about failure there directly. I
>>had
>>sent a patch to increase that a while ago [0] because of a report
>>with
>>"netdev_add". But probably would be good to take in too.
Never have seen problem of device_add, but yet, maybe with some os (we
look a t you windows ;), it's possible.
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2025-06-23 13:07 UTC | newest]
Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-06-18 13:01 [pve-devel] [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH common v2 01/32] schema: parse property string: support skipping keys Fiona Ebner
2025-06-20 11:00 ` Fabian Grünbichler
2025-06-18 13:01 ` [pve-devel] [PATCH common v2 02/32] json schema: add helper to convert to JSON boolean Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 03/32] buildsys: order Perl source files in QemuServer/Makefile Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 04/32] drive: code cleanup: drop unused $vmid parameter from get_path_and_format() Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 05/32] cfg2cmd: require at least QEMU binary version 6.0 Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 06/32] drive: parse: use hash argument for optional parameters Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 07/32] drive: parse: properly handle dropped properties for restore Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 08/32] drive: remove geometry options gone since QEMU 3.1 Fiona Ebner
2025-06-20 11:03 ` Fabian Grünbichler
2025-06-20 11:20 ` Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 09/32] clone disk: io uring check: fix call to determine cache direct Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 10/32] drive: move storage_allows_io_uring_default() and drive_uses_cache_direct() helpers to drive module Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 11/32] drive: introduce aio_cmdline_option() helper Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 12/32] drive: introduce detect_zeroes_cmdline_option() helper Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 13/32] vm start: assert that migration type is set for 'tcp' migration Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 14/32] introduce StateFile module for state file related helpers Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 15/32] vm start: move state file handling to dedicated module Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 16/32] vm start: move config_to_command() call further down Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 17/32] vm start/commandline: also clean up pci reservation when config_to_command() fails Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 18/32] vm start/commandline: activate volumes before config_to_command() Fiona Ebner
2025-06-20 11:33 ` Fabian Grünbichler
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 19/32] pci: add missing includes Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 20/32] test: add tests for PCI reservations Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 21/32] cfg2cmd: print vga: fix call to print_pcie_addr() Fiona Ebner
2025-06-18 13:01 ` [pve-devel] [PATCH qemu-server v2 22/32] pci: code cleanup: remove superfluous machine type paramater from print_pci_addr Fiona Ebner
2025-06-18 15:19 ` Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 23/32] cfg2cmd: collect optional parameters as a hash Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 24/32] qm: showcmd: never reserve PCI devices Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 25/32] vm devices list: prepare querying block device names for -blockdev Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 26/32] print drive device: explicitly set write-cache starting with machine version 10.0 Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 27/32] print drive device: set {r, w}error front-end properties " Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 28/32] print drive device: don't reference any drive for 'none' " Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 29/32] drive: create a throttle group for each drive " Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [PATCH qemu-server v2 30/32] blockdev: add helpers to generate blockdev commandline Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [RFC qemu-server v2 31/32] blockdev: add support for NBD paths Fiona Ebner
2025-06-18 13:02 ` [pve-devel] [RFC qemu-server v2 32/32] command line: switch to blockdev starting with machine version 10.0 Fiona Ebner
2025-06-23 9:12 ` DERUMIER, Alexandre via pve-devel
2025-06-23 9:31 ` Fiona Ebner
2025-06-23 13:06 ` DERUMIER, Alexandre via pve-devel
2025-06-20 13:03 ` [pve-devel] partially-applied: [PATCH-SERIES common/qemu-server v2 00/32] preparation for switch to blockdev Fabian Grünbichler
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