From: Friedrich Weber <f.weber@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH qemu-server 1/1] fix #5997: qemu: pass guest-ostype hint when activating volumes
Date: Fri, 24 Oct 2025 14:25:55 +0200 [thread overview]
Message-ID: <20251024122705.93761-4-f.weber@proxmox.com> (raw)
In-Reply-To: <20251024122705.93761-1-f.weber@proxmox.com>
KRBD can use this hint to pass rxbounce only when mapping disks of
Windows VMs, which should avoid "bad crc/signature" errors in the
host journal, retransmits as well as degraded performance.
Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
---
src/PVE/API2/Qemu.pm | 18 ++++++++++---
src/PVE/QemuConfig.pm | 1 +
src/PVE/QemuServer.pm | 44 ++++++++++++++++++++++++--------
src/PVE/QemuServer/Cloudinit.pm | 3 ++-
src/PVE/QemuServer/ImportDisk.pm | 4 ++-
src/PVE/QemuServer/OVMF.pm | 4 ++-
src/PVE/QemuServer/QemuImage.pm | 5 ++--
src/PVE/QemuServer/RunState.pm | 3 ++-
src/PVE/VZDump/QemuServer.pm | 6 +++--
9 files changed, 66 insertions(+), 22 deletions(-)
diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index 71bedc1e..296d73a3 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -446,6 +446,8 @@ my sub create_disks : prototype($$$$$$$$$$$) {
my $volid = $disk->{file};
my ($storeid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
+ my $storage_hints = PVE::QemuServer::generate_storage_hints($conf);
+
if (!$volid || $volid eq 'none' || $volid eq 'cdrom') {
delete $disk->{size};
$res->{$ds} = PVE::QemuServer::print_drive($disk);
@@ -548,6 +550,7 @@ my sub create_disks : prototype($$$$$$$$$$$) {
drivename => $ds,
storage => $storeid,
format => $disk->{format},
+ storage_hints => $storage_hints,
};
$dest_info->{efisize} = PVE::QemuServer::get_efivars_size($conf, $disk)
@@ -681,7 +684,8 @@ my sub create_disks : prototype($$$$$$$$$$$) {
}
}
- PVE::Storage::activate_volumes($storecfg, [$volid]) if $storeid;
+ PVE::Storage::activate_volumes($storecfg, [$volid], undef, $storage_hints)
+ if $storeid;
my $size = PVE::Storage::volume_size_info($storecfg, $volid);
die "volume $volid does not exist\n" if !$size;
@@ -4487,11 +4491,13 @@ __PACKAGE__->register_method({
my $newvollist = [];
my $jobs = {};
+ my $storage_hints = PVE::QemuServer::generate_storage_hints($newconf);
+
eval {
local $SIG{INT} = local $SIG{TERM} = local $SIG{QUIT} = local $SIG{HUP} =
sub { die "interrupted by signal\n"; };
- PVE::Storage::activate_volumes($storecfg, $vollist, $snapname);
+ PVE::Storage::activate_volumes($storecfg, $vollist, $snapname, $storage_hints);
my $bwlimit = extract_param($param, 'bwlimit');
@@ -4522,6 +4528,7 @@ __PACKAGE__->register_method({
drivename => $opt,
storage => $storage,
format => $format,
+ storage_hints => $storage_hints,
};
$dest_info->{efisize} = PVE::QemuServer::get_efivars_size($oldconf)
@@ -4768,7 +4775,8 @@ __PACKAGE__->register_method({
my $running = PVE::QemuServer::check_running($vmid);
- PVE::Storage::activate_volumes($storecfg, [$drive->{file}]);
+ my $storage_hints = PVE::QemuServer::generate_storage_hints($conf);
+ PVE::Storage::activate_volumes($storecfg, [$drive->{file}], undef, $storage_hints);
my $newvollist = [];
@@ -4797,6 +4805,7 @@ __PACKAGE__->register_method({
drivename => $disk,
storage => $storeid,
format => $format,
+ storage_hints => $storage_hints,
};
$dest_info->{efisize} = PVE::QemuServer::get_efivars_size($conf)
@@ -5847,7 +5856,8 @@ __PACKAGE__->register_method({
$rpcenv->check($authuser, "/storage/$storeid", ['Datastore.AllocateSpace']);
- PVE::Storage::activate_volumes($storecfg, [$volid]);
+ my $storage_hints = PVE::QemuServer::generate_storage_hints($conf);
+ PVE::Storage::activate_volumes($storecfg, [$volid], undef, $storage_hints);
my $size = PVE::Storage::volume_size_info($storecfg, $volid, 5);
die "Could not determine current size of volume '$volid'\n" if !defined($size);
diff --git a/src/PVE/QemuConfig.pm b/src/PVE/QemuConfig.pm
index ad6ce6b0..ef6c3edd 100644
--- a/src/PVE/QemuConfig.pm
+++ b/src/PVE/QemuConfig.pm
@@ -326,6 +326,7 @@ sub __snapshot_create_vol_snapshots_hook {
if ($hook eq "before") {
if ($snap->{vmstate}) {
my $path = PVE::Storage::path($storecfg, $snap->{vmstate});
+ # TODO: no guest hints needed for VM state?
PVE::Storage::activate_volumes($storecfg, [$snap->{vmstate}]);
my $state_storage_id = PVE::Storage::parse_volume_id($snap->{vmstate});
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index cf195ccc..c7b6dc63 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5248,7 +5248,9 @@ sub vmconfig_update_disk {
die "skip\n" if !$hotplug || $opt =~ m/(ide|sata)(\d+)/;
# hotplug new disks
- PVE::Storage::activate_volumes($storecfg, [$drive->{file}]) if $drive->{file} !~ m|^/dev/.+|;
+ my $storage_hints = generate_storage_hints($conf);
+ PVE::Storage::activate_volumes($storecfg, [$drive->{file}], undef, $storage_hints)
+ if $drive->{file} !~ m|^/dev/.+|;
vm_deviceplug($storecfg, $conf, $vmid, $opt, $drive, $arch, $machine_type);
}
@@ -5363,6 +5365,15 @@ my sub remove_left_over_vmstate_opts {
PVE::QemuConfig->write_config($vmid, $conf) if $found;
}
+sub generate_storage_hints {
+ my ($conf) = @_;
+
+ my $hints = {};
+ $hints->{'guest-ostype'} = $conf->{ostype} if PVE::Storage::is_hint_supported('guest-ostype');
+
+ return $hints;
+}
+
# see vm_start_nolock for parameters, additionally:
# migrate_opts:
# storagemap = parsed storage map for allocating NBD disks
@@ -5526,11 +5537,13 @@ sub vm_start_nolock {
my $vollist = get_current_vm_volumes($storecfg, $conf);
push $vollist->@*, $statefile if $statefile_is_a_volume;
+ my $storage_hints = generate_storage_hints($conf);
+
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);
+ PVE::Storage::activate_volumes($storecfg, $vollist, undef, $storage_hints);
# 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
@@ -5901,7 +5914,8 @@ sub vm_commandline {
# 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 $storage_hints = generate_storage_hints($conf);
+ PVE::Storage::activate_volumes($storecfg, $volumes, undef, $storage_hints);
}
# There might be concurrent operations on the volumes, so do not deactivate.
@@ -6474,7 +6488,7 @@ my $parse_backup_hints = sub {
#
# Returns: { $virtdev => $volid }
my $restore_allocate_devices = sub {
- my ($storecfg, $virtdev_hash, $vmid) = @_;
+ my ($storecfg, $virtdev_hash, $vmid, $storage_hints) = @_;
my $map = {};
foreach my $virtdev (sort keys %$virtdev_hash) {
@@ -6502,7 +6516,7 @@ my $restore_allocate_devices = sub {
print STDERR "new volume ID is '$volid'\n";
$d->{volid} = $volid;
- PVE::Storage::activate_volumes($storecfg, [$volid]);
+ PVE::Storage::activate_volumes($storecfg, [$volid], undef, $storage_hints);
$map->{$virtdev} = $volid;
}
@@ -6867,7 +6881,10 @@ sub restore_proxmox_backup_archive {
$restore_cleanup_oldconf->($storecfg, $vmid, $oldconf, $virtdev_hash) if $oldconf;
# allocate volumes
- my $map = $restore_allocate_devices->($storecfg, $virtdev_hash, $vmid);
+ my $storage_hints = {};
+ # TODO: we should also generate storage hints here for live-restore, but we do not have
+ # the config yet -- build the config here already?
+ my $map = $restore_allocate_devices->($storecfg, $virtdev_hash, $vmid, $storage_hints);
foreach my $virtdev (sort keys %$virtdev_hash) {
my $d = $virtdev_hash->{$virtdev};
@@ -7037,7 +7054,9 @@ sub restore_external_archive {
$restore_cleanup_oldconf->($storecfg, $vmid, $oldconf, $virtdev_hash) if $oldconf;
# allocate volumes
- my $map = $restore_allocate_devices->($storecfg, $virtdev_hash, $vmid);
+ my $storage_hints = {};
+ # TODO: in order to generate storage hints, we'd need to build the config already here
+ my $map = $restore_allocate_devices->($storecfg, $virtdev_hash, $vmid, $storage_hints);
for my $virtdev (sort keys $virtdev_hash->%*) {
my $d = $virtdev_hash->{$virtdev};
@@ -7062,6 +7081,7 @@ sub restore_external_archive {
'is-zero-initialized' => $sparseinit,
'source-path-format' => $source_format,
};
+ # no need to pass source-storage-hints for the source
PVE::QemuServer::QemuImage::convert(
$source_path,
$d->{volid},
@@ -7456,7 +7476,9 @@ sub restore_vma_archive {
}
# allocate volumes
- my $map = $restore_allocate_devices->($cfg, $virtdev_hash, $vmid);
+ my $storage_hints = {};
+ # TODO: here we do not necessarily have to pass storage hints, as we deactivate when we're done?
+ my $map = $restore_allocate_devices->($cfg, $virtdev_hash, $vmid, $storage_hints);
# print restore information to $fifofh
foreach my $virtdev (sort keys %$virtdev_hash) {
@@ -7754,7 +7776,7 @@ sub clone_disk {
my ($src_drivename, $drive, $snapname) = $source->@{qw(drivename drive snapname)};
my ($newvmid, $dst_drivename, $efisize) = $dest->@{qw(vmid drivename efisize)};
- my ($storage, $format) = $dest->@{qw(storage format)};
+ my ($storage, $format, $storage_hints) = $dest->@{qw(storage format storage_hints)};
my $unused = defined($src_drivename) && $src_drivename =~ /^unused/;
my $use_drive_mirror = $full && $running && $src_drivename && !$snapname && !$unused;
@@ -7820,7 +7842,7 @@ sub clone_disk {
);
push @$newvollist, $newvolid;
- PVE::Storage::activate_volumes($storecfg, [$newvolid]);
+ PVE::Storage::activate_volumes($storecfg, [$newvolid], undef, $storage_hints);
if (drive_is_cloudinit($drive)) {
# when cloning multiple disks (e.g. during clone_vm) it might be the last disk
@@ -7877,6 +7899,8 @@ sub clone_disk {
bwlimit => $bwlimit,
'is-zero-initialized' => $sparseinit,
snapname => $snapname,
+ # TODO: double-check it's OK to pass the dest's storage hints here
+ 'source-storage-hints' => $storage_hints,
};
PVE::QemuServer::QemuImage::convert($drive->{file}, $newvolid, $size, $opts);
}
diff --git a/src/PVE/QemuServer/Cloudinit.pm b/src/PVE/QemuServer/Cloudinit.pm
index 349cf90b..f42047e0 100644
--- a/src/PVE/QemuServer/Cloudinit.pm
+++ b/src/PVE/QemuServer/Cloudinit.pm
@@ -48,7 +48,8 @@ sub commit_cloudinit_disk {
$size *= 1024; # vdisk alloc takes KB, qemu-img dd's osize takes byte
}
my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
- $plugin->activate_volume($storeid, $scfg, $volname);
+ my $storage_hints = PVE::QemuServer::generate_storage_hints($conf);
+ $plugin->activate_volume($storeid, $scfg, $volname, $storage_hints);
print "generating cloud-init ISO\n";
eval {
diff --git a/src/PVE/QemuServer/ImportDisk.pm b/src/PVE/QemuServer/ImportDisk.pm
index 75a54aa2..030cc4a1 100755
--- a/src/PVE/QemuServer/ImportDisk.pm
+++ b/src/PVE/QemuServer/ImportDisk.pm
@@ -84,7 +84,9 @@ sub do_import {
local $SIG{INT} = local $SIG{TERM} = local $SIG{QUIT} = local $SIG{HUP} =
local $SIG{PIPE} = sub { die "interrupted by signal $!\n"; };
- PVE::Storage::activate_volumes($storecfg, [$dst_volid]);
+ # TODO: do we need storage hints here if we deactivate the volumes immediately?
+ PVE::Storage::activate_volumes($storecfg, [$dst_volid], undef, undef);
+ # TODO: do we need source-storage-hints here?
PVE::QemuServer::QemuImage::convert(
$src_path,
$dst_volid,
diff --git a/src/PVE/QemuServer/OVMF.pm b/src/PVE/QemuServer/OVMF.pm
index 08134e30..717253b2 100644
--- a/src/PVE/QemuServer/OVMF.pm
+++ b/src/PVE/QemuServer/OVMF.pm
@@ -136,8 +136,10 @@ sub create_efidisk($$$$$$$$) {
my $vars_size_b = -s $ovmf_vars;
my $vars_size = PVE::Tools::convert_size($vars_size_b, 'b' => 'kb');
my $volid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, $fmt, undef, $vars_size);
- PVE::Storage::activate_volumes($storecfg, [$volid]);
+ # TODO: storage hints needed for EFI disks?
+ PVE::Storage::activate_volumes($storecfg, [$volid], undef, undef);
+ # no source-storage-hints passed here, because the source is not a volume
PVE::QemuServer::QemuImage::convert($ovmf_vars, $volid, $vars_size_b);
my $size = PVE::Storage::volume_size_info($storecfg, $volid, 3);
diff --git a/src/PVE/QemuServer/QemuImage.pm b/src/PVE/QemuServer/QemuImage.pm
index 71be3abb..0adfbee1 100644
--- a/src/PVE/QemuServer/QemuImage.pm
+++ b/src/PVE/QemuServer/QemuImage.pm
@@ -97,10 +97,11 @@ my sub qcow2_target_image_opts {
# snapname - Use this snapshot of the source image.
# source-path-format - Indicate the format of the source when the source is a path. For PVE-managed
# volumes, the format from the storage layer is always used.
+# source-storage-hints - storage hints to pass when activating the source volume
sub convert {
my ($src_volid, $dst_volid, $size, $opts) = @_;
- my ($bwlimit, $snapname) = $opts->@{qw(bwlimit snapname)};
+ my ($bwlimit, $snapname, $storage_hints) = $opts->@{qw(bwlimit snapname source-storage-hints)};
my $storecfg = PVE::Storage::config();
my ($src_storeid) = PVE::Storage::parse_volume_id($src_volid, 1);
@@ -114,7 +115,7 @@ sub convert {
my $src_format;
if ($src_storeid) {
- PVE::Storage::activate_volumes($storecfg, [$src_volid], $snapname);
+ PVE::Storage::activate_volumes($storecfg, [$src_volid], $snapname, $storage_hints);
my $src_scfg = PVE::Storage::storage_config($storecfg, $src_storeid);
$src_format = checked_volume_format($storecfg, $src_volid);
$src_path = PVE::Storage::path($storecfg, $src_volid, $snapname);
diff --git a/src/PVE/QemuServer/RunState.pm b/src/PVE/QemuServer/RunState.pm
index 6a5fdbd7..fd2771c1 100644
--- a/src/PVE/QemuServer/RunState.pm
+++ b/src/PVE/QemuServer/RunState.pm
@@ -69,7 +69,8 @@ sub vm_suspend {
if ($includestate) {
# save vm state
- PVE::Storage::activate_volumes($storecfg, [$vmstate]);
+ # TODO: need hints for VM state?
+ PVE::Storage::activate_volumes($storecfg, [$vmstate], undef, undef);
eval {
PVE::QemuMigrate::Helpers::set_migration_caps($vmid, 1);
diff --git a/src/PVE/VZDump/QemuServer.pm b/src/PVE/VZDump/QemuServer.pm
index dd789652..4f387731 100644
--- a/src/PVE/VZDump/QemuServer.pm
+++ b/src/PVE/VZDump/QemuServer.pm
@@ -118,7 +118,8 @@ sub prepare {
$drivehash->{$name} = $volume->{volume_config};
}
- PVE::Storage::activate_volumes($self->{storecfg}, $vollist);
+ my $storage_hints = PVE::QemuServer::generate_storage_hints($conf);
+ PVE::Storage::activate_volumes($self->{storecfg}, $vollist, undef, $storage_hints);
foreach my $ds (sort keys %$drivehash) {
my $drive = $drivehash->{$ds};
@@ -643,7 +644,8 @@ my sub attach_fleecing_images {
detach_fleecing_images($disks, $vmid);
my $vollist = [map { $_->{'fleece-volid'} } grep { $_->{'fleece-volid'} } $disks->@*];
- PVE::Storage::activate_volumes($self->{storecfg}, $vollist);
+ # TODO: storage hints needed for fleecing images?
+ PVE::Storage::activate_volumes($self->{storecfg}, $vollist, undef, undef);
for my $di ($disks->@*) {
if (my $volid = $di->{'fleece-volid'}) {
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
prev parent reply other threads:[~2025-10-24 12:27 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-24 12:25 [pve-devel] [RFC qemu-server/storage 0/3] fix #5779: introduce guest hints to pass rxbounce flag to KRBD Friedrich Weber
2025-10-24 12:25 ` [pve-devel] [PATCH pve-storage 1/2] plugin: map/activate volume: allow callers to pass hints Friedrich Weber
2025-10-24 12:25 ` [pve-devel] [PATCH pve-storage 2/2] plugin: rbd: pass rxbounce when mapping Windows VM disks Friedrich Weber
2025-10-24 12:25 ` Friedrich Weber [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251024122705.93761-4-f.weber@proxmox.com \
--to=f.weber@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox