From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 995161FF16B for ; Fri, 24 Oct 2025 14:27:39 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 1E78D200E3; Fri, 24 Oct 2025 14:27:53 +0200 (CEST) From: Friedrich Weber To: pve-devel@lists.proxmox.com Date: Fri, 24 Oct 2025 14:25:55 +0200 Message-ID: <20251024122705.93761-4-f.weber@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20251024122705.93761-1-f.weber@proxmox.com> References: <20251024122705.93761-1-f.weber@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1761308827146 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.013 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH qemu-server 1/1] fix #5997: qemu: pass guest-ostype hint when activating volumes X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" 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 --- 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