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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.