* [pve-devel] [PATCH qemu-server 0/8] disable S3/S4 power states by default @ 2025-03-06 10:44 Dominik Csapak 2025-03-06 10:44 ` [pve-devel] [RFC PATCH qemu-server 1/8] tests: cfg2cmd: pin QEMU version Dominik Csapak ` (8 more replies) 0 siblings, 9 replies; 35+ messages in thread From: Dominik Csapak @ 2025-03-06 10:44 UTC (permalink / raw) To: pve-devel since they make some problems (e.g. windows hybrid shutdown is enabled by default then -> which makes vGPU problem). Libvirt/virsh also disables that by default (and tries preventing enabling it.) This series introduces a new pve1 version for 9.2 machine versions, and pins new windows guests to that. For this, we need to save the current pve version in the meta info too now. Additionally I introduce 'enable-s{3,4}' settings so that users can still manually enable that (in case it's needed), or disable it for older machine types. (non hotpluggable). I deliberatly did not introduce a GUI option for the enabling of S3/S4 power states, since I don't think it'll come up often. (If it does, we can still add something to the UI) The only ugly thing left, is that one cannot select the 9.2+pve1 variant in the gui manually (e.g. for windows), since we don't have anything in the UI for that. The question here is if I should introduce a pve version selector. (I personally think it's not a great idea, because it does not really convey any meaning to the user if e.g. pve0 or pve1 is better...) Note that patch 1 is only an RFC and has only tangentially to do with the series. I sent it along, because the way we test will change the tests again if we apply the rest of this series and bump to the next qemu version (due to the +pve1 -> +pve0 change). Dominik Csapak (8): tests: cfg2cmd: pin QEMU version config to command: add one '-global' option for each flag meta info: also add current pve-machine version machine: correctly select pve machine version for non pinned windows guests machine: incorporate pve machine version when pinning windows guests machine: add S3/S4 power state properties machine: bump pve machine version and reverse the s3/s4 defaults tests: cfg2cmd: add test for windows machine pinning from meta info PVE/API2/Qemu.pm | 4 +- PVE/QemuServer.pm | 7 +- PVE/QemuServer/Machine.pm | 71 +++++++++++++++++-- PVE/QemuServer/MetaInfo.pm | 28 +++++--- test/cfg2cmd/bootorder-empty.conf.cmd | 4 +- test/cfg2cmd/bootorder-legacy.conf.cmd | 4 +- test/cfg2cmd/bootorder.conf.cmd | 4 +- ...putype-icelake-client-deprecation.conf.cmd | 4 +- .../custom-cpu-model-defaults.conf.cmd | 4 +- test/cfg2cmd/efi-raw-template.conf.cmd | 4 +- test/cfg2cmd/efi-raw.conf.cmd | 4 +- test/cfg2cmd/efi-secboot-and-tpm-q35.conf.cmd | 4 +- test/cfg2cmd/efi-secboot-and-tpm.conf.cmd | 4 +- test/cfg2cmd/efidisk-on-rbd.conf.cmd | 4 +- test/cfg2cmd/i440fx-viommu-virtio.conf.cmd | 4 +- test/cfg2cmd/ide.conf.cmd | 4 +- .../cfg2cmd/memory-hotplug-hugepages.conf.cmd | 4 +- test/cfg2cmd/memory-hotplug.conf.cmd | 4 +- test/cfg2cmd/memory-hugepages-1g.conf.cmd | 4 +- test/cfg2cmd/memory-hugepages-2m.conf.cmd | 4 +- test/cfg2cmd/minimal-defaults.conf.cmd | 4 +- test/cfg2cmd/netdev-7.1-multiqueues.conf.cmd | 4 +- test/cfg2cmd/netdev-7.1.conf.cmd | 4 +- test/cfg2cmd/q35-ide.conf.cmd | 4 +- .../q35-linux-hostpci-mapping.conf.cmd | 4 +- .../q35-linux-hostpci-multifunction.conf.cmd | 4 +- .../q35-linux-hostpci-template.conf.cmd | 4 +- ...q35-linux-hostpci-x-pci-overrides.conf.cmd | 4 +- test/cfg2cmd/q35-linux-hostpci.conf.cmd | 4 +- test/cfg2cmd/q35-simple.conf.cmd | 4 +- test/cfg2cmd/q35-viommu-intel.conf.cmd | 4 +- test/cfg2cmd/q35-viommu-virtio.conf.cmd | 4 +- test/cfg2cmd/q35-windows-pinning-pvever.conf | 5 ++ .../q35-windows-pinning-pvever.conf.cmd | 26 +++++++ test/cfg2cmd/q35-windows-pinning.conf | 5 ++ test/cfg2cmd/q35-windows-pinning.conf.cmd | 24 +++++++ test/cfg2cmd/seabios_serial.conf.cmd | 4 +- test/cfg2cmd/simple-btrfs.conf.cmd | 4 +- test/cfg2cmd/simple-virtio-blk.conf.cmd | 4 +- test/cfg2cmd/simple1-template.conf.cmd | 4 +- test/cfg2cmd/simple1.conf.cmd | 4 +- test/cfg2cmd/vnc-clipboard-spice.conf.cmd | 4 +- test/cfg2cmd/vnc-clipboard-std.conf.cmd | 4 +- test/run_config2command_tests.pl | 23 +++++- 44 files changed, 281 insertions(+), 52 deletions(-) create mode 100644 test/cfg2cmd/q35-windows-pinning-pvever.conf create mode 100644 test/cfg2cmd/q35-windows-pinning-pvever.conf.cmd create mode 100644 test/cfg2cmd/q35-windows-pinning.conf create mode 100644 test/cfg2cmd/q35-windows-pinning.conf.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] 35+ messages in thread
* [pve-devel] [RFC PATCH qemu-server 1/8] tests: cfg2cmd: pin QEMU version 2025-03-06 10:44 [pve-devel] [PATCH qemu-server 0/8] disable S3/S4 power states by default Dominik Csapak @ 2025-03-06 10:44 ` Dominik Csapak 2025-03-06 12:00 ` Fiona Ebner 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 2/8] config to command: add one '-global' option for each flag Dominik Csapak ` (7 subsequent siblings) 8 siblings, 1 reply; 35+ messages in thread From: Dominik Csapak @ 2025-03-06 10:44 UTC (permalink / raw) To: pve-devel but warn when we're out of date compared to the installed one, and die when we're one major (+1 minor) release behind. (the warning is not very visible when running tests or when building) We don't want to depend on the installed QEMU version for such tests, otherwise a developer might need to adapt tests because the installed QEMU version is different to what is e.g. in the build environment for our packaging. Also, this way, we have to show intent for bumping this and it will be obvious that we need to adapt tests because of a changed QEMU version. Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- test/run_config2command_tests.pl | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl index 7e3d10e6..5fa6f2de 100755 --- a/test/run_config2command_tests.pl +++ b/test/run_config2command_tests.pl @@ -20,6 +20,10 @@ use PVE::QemuServer::Monitor; use PVE::QemuServer::QMPHelpers; use PVE::QemuServer::CPUConfig; +# bump when QEMU version changes +my $tested_version_major = 9; +my $tested_version_minor = 2; + my $base_env = { storage_config => { ids => { @@ -80,6 +84,7 @@ my $base_env = { } }, vmid => 8006, + tested_qemu_version => "$tested_version_major.$tested_version_minor", real_qemu_version => PVE::QemuServer::Helpers::kvm_user_version(), # not yet mocked }; @@ -184,7 +189,7 @@ sub parse_test($) { } sub get_test_qemu_version { - $current_test->{qemu_version} // $base_env->{real_qemu_version} // '2.12'; + $current_test->{qemu_version} // $base_env->{tested_qemu_version} // '2.12'; } my $qemu_server_module; @@ -528,3 +533,19 @@ if (my $file = shift) { } done_testing(); + +# reset warn +$SIG{__WARN__} = undef; +if ($base_env->{real_qemu_version} =~ m/^(\d+.\d+)/) { + if (PVE::QemuServer::Helpers::version_cmp($1, $tested_version_major, $2, $tested_version_minor) < 0) { + warn "\nWARNING: installed QEMU version bigger than tested one, please bump!\n"; + } + + # if we did not bump since the last major QEMU (+1 minor) release fail the test + if (PVE::QemuServer::Helpers::version_cmp($1, $tested_version_major + 1, $2, 1) >= 0) { + die "\nERROR: installed QEMU version one major release bigger than tested one, please bump!\n"; + } + +} else { + die "\nERROR: invalid QEMU version\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] 35+ messages in thread
* Re: [pve-devel] [RFC PATCH qemu-server 1/8] tests: cfg2cmd: pin QEMU version 2025-03-06 10:44 ` [pve-devel] [RFC PATCH qemu-server 1/8] tests: cfg2cmd: pin QEMU version Dominik Csapak @ 2025-03-06 12:00 ` Fiona Ebner 2025-03-06 12:07 ` Dominik Csapak 0 siblings, 1 reply; 35+ messages in thread From: Fiona Ebner @ 2025-03-06 12:00 UTC (permalink / raw) To: Proxmox VE development discussion, Dominik Csapak Am 06.03.25 um 11:44 schrieb Dominik Csapak: > but warn when we're out of date compared to the installed one, and die > when we're one major (+1 minor) release behind. > (the warning is not very visible when running tests or when building) > > We don't want to depend on the installed QEMU version for such tests, > otherwise a developer might need to adapt tests because the installed > QEMU version is different to what is e.g. in the build environment for > our packaging. > > Also, this way, we have to show intent for bumping this and it will be > obvious that we need to adapt tests because of a changed QEMU version. > > Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> > --- > test/run_config2command_tests.pl | 23 ++++++++++++++++++++++- > 1 file changed, 22 insertions(+), 1 deletion(-) > > diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl > index 7e3d10e6..5fa6f2de 100755 > --- a/test/run_config2command_tests.pl > +++ b/test/run_config2command_tests.pl > @@ -20,6 +20,10 @@ use PVE::QemuServer::Monitor; > use PVE::QemuServer::QMPHelpers; > use PVE::QemuServer::CPUConfig; > > +# bump when QEMU version changes > +my $tested_version_major = 9; > +my $tested_version_minor = 2; > + > my $base_env = { > storage_config => { > ids => { > @@ -80,6 +84,7 @@ my $base_env = { > } > }, > vmid => 8006, > + tested_qemu_version => "$tested_version_major.$tested_version_minor", > real_qemu_version => PVE::QemuServer::Helpers::kvm_user_version(), # not yet mocked I'd not keep the real_qemu_version in base_env anymore, but have it as a stand-alone variable for the version comparison check. > }; > > @@ -184,7 +189,7 @@ sub parse_test($) { > } > > sub get_test_qemu_version { > - $current_test->{qemu_version} // $base_env->{real_qemu_version} // '2.12'; > + $current_test->{qemu_version} // $base_env->{tested_qemu_version} // '2.12'; > } > > my $qemu_server_module; > @@ -528,3 +533,19 @@ if (my $file = shift) { > } > > done_testing(); Nit: Since the check below can die, I'd put it at the very beginning rather than at the end. > + > +# reset warn IMHO "Why is the reset needed?" is the interesting part worth commenting here ;) > +$SIG{__WARN__} = undef; > +if ($base_env->{real_qemu_version} =~ m/^(\d+.\d+)/) { > + if (PVE::QemuServer::Helpers::version_cmp($1, $tested_version_major, $2, $tested_version_minor) < 0) { > + warn "\nWARNING: installed QEMU version bigger than tested one, please bump!\n"; > + } > + > + # if we did not bump since the last major QEMU (+1 minor) release fail the test > + if (PVE::QemuServer::Helpers::version_cmp($1, $tested_version_major + 1, $2, 1) >= 0) { I'd rather have a fixed difference between versions trigger the die. Your check behaves the same when the tested version is 10.0 and when the tested version is 10.2. In both cases, having the real version 11.1 will trigger the die. > + die "\nERROR: installed QEMU version one major release bigger than tested one, please bump!\n"; > + } > + > +} else { > + die "\nERROR: invalid QEMU version\n"; > +} _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [RFC PATCH qemu-server 1/8] tests: cfg2cmd: pin QEMU version 2025-03-06 12:00 ` Fiona Ebner @ 2025-03-06 12:07 ` Dominik Csapak 2025-03-06 12:43 ` Fiona Ebner 0 siblings, 1 reply; 35+ messages in thread From: Dominik Csapak @ 2025-03-06 12:07 UTC (permalink / raw) To: Fiona Ebner, Proxmox VE development discussion On 3/6/25 13:00, Fiona Ebner wrote: > Am 06.03.25 um 11:44 schrieb Dominik Csapak: >> but warn when we're out of date compared to the installed one, and die >> when we're one major (+1 minor) release behind. >> (the warning is not very visible when running tests or when building) >> >> We don't want to depend on the installed QEMU version for such tests, >> otherwise a developer might need to adapt tests because the installed >> QEMU version is different to what is e.g. in the build environment for >> our packaging. >> >> Also, this way, we have to show intent for bumping this and it will be >> obvious that we need to adapt tests because of a changed QEMU version. >> >> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> >> --- >> test/run_config2command_tests.pl | 23 ++++++++++++++++++++++- >> 1 file changed, 22 insertions(+), 1 deletion(-) >> >> diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl >> index 7e3d10e6..5fa6f2de 100755 >> --- a/test/run_config2command_tests.pl >> +++ b/test/run_config2command_tests.pl >> @@ -20,6 +20,10 @@ use PVE::QemuServer::Monitor; >> use PVE::QemuServer::QMPHelpers; >> use PVE::QemuServer::CPUConfig; >> >> +# bump when QEMU version changes >> +my $tested_version_major = 9; >> +my $tested_version_minor = 2; >> + >> my $base_env = { >> storage_config => { >> ids => { >> @@ -80,6 +84,7 @@ my $base_env = { >> } >> }, >> vmid => 8006, >> + tested_qemu_version => "$tested_version_major.$tested_version_minor", >> real_qemu_version => PVE::QemuServer::Helpers::kvm_user_version(), # not yet mocked > > I'd not keep the real_qemu_version in base_env anymore, but have it as a > stand-alone variable for the version comparison check. makes sense > >> }; >> >> @@ -184,7 +189,7 @@ sub parse_test($) { >> } >> >> sub get_test_qemu_version { >> - $current_test->{qemu_version} // $base_env->{real_qemu_version} // '2.12'; >> + $current_test->{qemu_version} // $base_env->{tested_qemu_version} // '2.12'; >> } >> >> my $qemu_server_module; >> @@ -528,3 +533,19 @@ if (my $file = shift) { >> } >> >> done_testing(); > > Nit: Since the check below can die, I'd put it at the very beginning > rather than at the end. mhmm ok, the rationale was that when letting the tests run manually, the warning can be seen when it's at the end, but if it's printed at the start, it'll most likely be overlooked... i played around with requiring input from the user to continue, but that didn't work at all when building (and is probably not a good idea in general when running tests..) Not sure how we can make the warning more visible while not failing the tests at the same time... > >> + >> +# reset warn > > IMHO "Why is the reset needed?" is the interesting part worth commenting > here ;) true ;), we override the warn handler above to check the warning for the expected ones, we could of course add some code there to handle this case here, but it seemed shorter/more elegant to just reset the handler completely when we're done with testing... > >> +$SIG{__WARN__} = undef; >> +if ($base_env->{real_qemu_version} =~ m/^(\d+.\d+)/) { >> + if (PVE::QemuServer::Helpers::version_cmp($1, $tested_version_major, $2, $tested_version_minor) < 0) { >> + warn "\nWARNING: installed QEMU version bigger than tested one, please bump!\n"; >> + } >> + >> + # if we did not bump since the last major QEMU (+1 minor) release fail the test >> + if (PVE::QemuServer::Helpers::version_cmp($1, $tested_version_major + 1, $2, 1) >= 0) { > > I'd rather have a fixed difference between versions trigger the die. > Your check behaves the same when the tested version is 10.0 and when the > tested version is 10.2. In both cases, having the real version 11.1 will > trigger the die. > not exactly sure what you mean here: IIUC, in the scenario where we pinned X.Y, you want the test to fail when the real version is: X+1.Y ? or when X+1.Y+1 ? or when X+1.Y-1 ? my thought here was that we want to update at least once per major release (not sure if that means anything for qemu though), but leave a wiggle room until the first point release >> + die "\nERROR: installed QEMU version one major release bigger than tested one, please bump!\n"; >> + } >> + >> +} else { >> + die "\nERROR: invalid QEMU version\n"; >> +} > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [RFC PATCH qemu-server 1/8] tests: cfg2cmd: pin QEMU version 2025-03-06 12:07 ` Dominik Csapak @ 2025-03-06 12:43 ` Fiona Ebner 0 siblings, 0 replies; 35+ messages in thread From: Fiona Ebner @ 2025-03-06 12:43 UTC (permalink / raw) To: Dominik Csapak, Proxmox VE development discussion Am 06.03.25 um 13:07 schrieb Dominik Csapak: > On 3/6/25 13:00, Fiona Ebner wrote: >> Am 06.03.25 um 11:44 schrieb Dominik Csapak: >>> @@ -528,3 +533,19 @@ if (my $file = shift) { >>> } >>> done_testing(); >> >> Nit: Since the check below can die, I'd put it at the very beginning >> rather than at the end. > > mhmm ok, the rationale was that when letting the tests run manually, > the warning can be seen when it's at the end, but if it's printed at the > start, it'll most likely be overlooked... > > i played around with requiring input from the user to continue, > but that didn't work at all when building (and is probably not > a good idea in general when running tests..) > > Not sure how we can make the warning more visible while not failing the > tests at the same time... Thinking again about the whole pinning, I'm not sure we really want to. If there are changes based on the binary version (rare), I do want to have the actual tests be run immediately. Tests should not change between binary versions without intent. The exception is here, with the bumped pve version. And I don't mean the bumping itself, that has intent and is fine. If we bump the pve version, that QEMU version is already public (or we could've still made the change based on the QEMU version rather than pve version ;)). The actual issue is with QEMU 10.0 where we will need to adapt the tests for un-bumping the pve version again. That does not have intent and that will temporarily break build. Maybe we can still use the installed version for running tests by default. We can add an environment variable or similar for overriding the test version. Additionally, record the expected version in the test script and die if it doesn't match the installed binary, suggesting to use the override. Then the package can still be built by setting the environment variable as an escape hatch. While usual builds will use and test the current version that will be running on people's systems. > >> >>> + >>> +# reset warn >> >> IMHO "Why is the reset needed?" is the interesting part worth commenting >> here ;) > > true ;), we override the warn handler above to check the warning for the > expected > ones, we could of course add some code there to handle this case here, > but it seemed shorter/more elegant to just reset the handler completely > when we're done with testing... > >> >>> +$SIG{__WARN__} = undef; >>> +if ($base_env->{real_qemu_version} =~ m/^(\d+.\d+)/) { >>> + if (PVE::QemuServer::Helpers::version_cmp($1, >>> $tested_version_major, $2, $tested_version_minor) < 0) { >>> + warn "\nWARNING: installed QEMU version bigger than tested >>> one, please bump!\n"; >>> + } >>> + >>> + # if we did not bump since the last major QEMU (+1 minor) >>> release fail the test >>> + if (PVE::QemuServer::Helpers::version_cmp($1, >>> $tested_version_major + 1, $2, 1) >= 0) { >> >> I'd rather have a fixed difference between versions trigger the die. >> Your check behaves the same when the tested version is 10.0 and when the >> tested version is 10.2. In both cases, having the real version 11.1 will >> trigger the die. >> > > not exactly sure what you mean here: > > IIUC, in the scenario where we pinned X.Y, you want the test to fail > when the real version is: > > X+1.Y ? Yes, for example. I don't think we should wait longer. > > or when > > X+1.Y+1 ? That release might not exist, e.g. 10.3 will never exist. Of course you can make what you want work by distinguishing based on major version difference. > > or when X+1.Y-1 ? Similar here. > > my thought here was that we want to update at least once per major > release (not sure > if that means anything for qemu though), but leave a wiggle room until > the first point release Yes, but I'd prefer being reminded after a fixed difference. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* [pve-devel] [PATCH qemu-server 2/8] config to command: add one '-global' option for each flag 2025-03-06 10:44 [pve-devel] [PATCH qemu-server 0/8] disable S3/S4 power states by default Dominik Csapak 2025-03-06 10:44 ` [pve-devel] [RFC PATCH qemu-server 1/8] tests: cfg2cmd: pin QEMU version Dominik Csapak @ 2025-03-06 10:44 ` Dominik Csapak 2025-03-06 12:13 ` Fiona Ebner 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 3/8] meta info: also add current pve-machine version Dominik Csapak ` (6 subsequent siblings) 8 siblings, 1 reply; 35+ messages in thread From: Dominik Csapak @ 2025-03-06 10:44 UTC (permalink / raw) To: pve-devel If we have multiple 'globalFlags', we have to encode each one separately on the commandline with '-global OPTION', since QEMU does not allow to have multiple options here. We currently only have one such flag that used the 'globalFlags' list, so it never popped up. (All other uses directly add an option to the commandline) Avoid future bugs by fixing it now. Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- PVE/QemuServer.pm | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index b6fc1f17..13af495d 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -4001,7 +4001,9 @@ sub config_to_command { push @$cmd, @$devices; push @$cmd, '-rtc', join(',', @$rtcFlags) if scalar(@$rtcFlags); push @$cmd, '-machine', join(',', @$machineFlags) if scalar(@$machineFlags); - push @$cmd, '-global', join(',', @$globalFlags) if scalar(@$globalFlags); + for my $flag ($globalFlags->@*) { + push @$cmd, '-global', $flag; + } if (my $vmstate = $conf->{vmstate}) { my $statepath = PVE::Storage::path($storecfg, $vmstate); -- 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] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 2/8] config to command: add one '-global' option for each flag 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 2/8] config to command: add one '-global' option for each flag Dominik Csapak @ 2025-03-06 12:13 ` Fiona Ebner 2025-03-06 12:15 ` Dominik Csapak 0 siblings, 1 reply; 35+ messages in thread From: Fiona Ebner @ 2025-03-06 12:13 UTC (permalink / raw) To: Proxmox VE development discussion, Dominik Csapak Am 06.03.25 um 11:44 schrieb Dominik Csapak: > If we have multiple 'globalFlags', we have to encode each one separately > on the commandline with '-global OPTION', since QEMU does not allow to > have multiple options here. > > We currently only have one such flag that used the 'globalFlags' list, > so it never popped up. (All other uses directly add an option to the > commandline) > > Avoid future bugs by fixing it now. > So there is no real point to collecting the flags in the first place? I.e. we could also get rid of the variable and have the single current user of the variable add the flag directly on the commandline too. Or otherwise, we could change the other users and collect all flags with this variable. Pre-existing of course, but ideally, we could avoid the mishmash. > Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com> > --- > PVE/QemuServer.pm | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm > index b6fc1f17..13af495d 100644 > --- a/PVE/QemuServer.pm > +++ b/PVE/QemuServer.pm > @@ -4001,7 +4001,9 @@ sub config_to_command { > push @$cmd, @$devices; > push @$cmd, '-rtc', join(',', @$rtcFlags) if scalar(@$rtcFlags); > push @$cmd, '-machine', join(',', @$machineFlags) if scalar(@$machineFlags); > - push @$cmd, '-global', join(',', @$globalFlags) if scalar(@$globalFlags); > + for my $flag ($globalFlags->@*) { > + push @$cmd, '-global', $flag; > + } > > if (my $vmstate = $conf->{vmstate}) { > my $statepath = PVE::Storage::path($storecfg, $vmstate); _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 2/8] config to command: add one '-global' option for each flag 2025-03-06 12:13 ` Fiona Ebner @ 2025-03-06 12:15 ` Dominik Csapak 2025-03-06 12:55 ` Fiona Ebner 0 siblings, 1 reply; 35+ messages in thread From: Dominik Csapak @ 2025-03-06 12:15 UTC (permalink / raw) To: Fiona Ebner, Proxmox VE development discussion On 3/6/25 13:13, Fiona Ebner wrote: > Am 06.03.25 um 11:44 schrieb Dominik Csapak: >> If we have multiple 'globalFlags', we have to encode each one separately >> on the commandline with '-global OPTION', since QEMU does not allow to >> have multiple options here. >> >> We currently only have one such flag that used the 'globalFlags' list, >> so it never popped up. (All other uses directly add an option to the >> commandline) >> >> Avoid future bugs by fixing it now. >> > > So there is no real point to collecting the flags in the first place? > I.e. we could also get rid of the variable and have the single current > user of the variable add the flag directly on the commandline too. Or > otherwise, we could change the other users and collect all flags with > this variable. Pre-existing of course, but ideally, we could avoid the > mishmash. > Sorry this could have been more clear here: I add to the flags in one of the following patches, so i sent this in preparation of that (could possibly be squashed) I did not want to touch the other places, since that in turn changes the order of the qemu commandline (which sometimes has unintended side effects, e.g. in combination with the 'args' parameter) >> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> > > Reviewed-by: Fiona Ebner <f.ebner@proxmox.com> > >> --- >> PVE/QemuServer.pm | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm >> index b6fc1f17..13af495d 100644 >> --- a/PVE/QemuServer.pm >> +++ b/PVE/QemuServer.pm >> @@ -4001,7 +4001,9 @@ sub config_to_command { >> push @$cmd, @$devices; >> push @$cmd, '-rtc', join(',', @$rtcFlags) if scalar(@$rtcFlags); >> push @$cmd, '-machine', join(',', @$machineFlags) if scalar(@$machineFlags); >> - push @$cmd, '-global', join(',', @$globalFlags) if scalar(@$globalFlags); >> + for my $flag ($globalFlags->@*) { >> + push @$cmd, '-global', $flag; >> + } >> >> if (my $vmstate = $conf->{vmstate}) { >> my $statepath = PVE::Storage::path($storecfg, $vmstate); > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 2/8] config to command: add one '-global' option for each flag 2025-03-06 12:15 ` Dominik Csapak @ 2025-03-06 12:55 ` Fiona Ebner 2025-03-07 9:54 ` Dominik Csapak 0 siblings, 1 reply; 35+ messages in thread From: Fiona Ebner @ 2025-03-06 12:55 UTC (permalink / raw) To: Dominik Csapak, Proxmox VE development discussion Am 06.03.25 um 13:15 schrieb Dominik Csapak: > On 3/6/25 13:13, Fiona Ebner wrote: >> Am 06.03.25 um 11:44 schrieb Dominik Csapak: >>> If we have multiple 'globalFlags', we have to encode each one separately >>> on the commandline with '-global OPTION', since QEMU does not allow to >>> have multiple options here. >>> >>> We currently only have one such flag that used the 'globalFlags' list, >>> so it never popped up. (All other uses directly add an option to the >>> commandline) >>> >>> Avoid future bugs by fixing it now. >>> >> >> So there is no real point to collecting the flags in the first place? >> I.e. we could also get rid of the variable and have the single current >> user of the variable add the flag directly on the commandline too. Or >> otherwise, we could change the other users and collect all flags with >> this variable. Pre-existing of course, but ideally, we could avoid the >> mishmash. >> > > Sorry this could have been more clear here: > I add to the flags in one of the following patches, so i sent this > in preparation of that (could possibly be squashed) Yes, I understand that. I still think the status quo with mixing two different approaches might not be best. It's not going to be a blocker for the series, but I wanted to mention it, if you want to go for avoiding it. > I did not want to touch the other places, since that in turn changes > the order of the qemu commandline (which sometimes has unintended side > effects, e.g. in combination with the 'args' parameter) Are you sure? Custom 'args' are always added last so that shouldn't matter. The only thing that would change by removing the global flags variable is having "-global kvm-pit.lost_tick_policy=discard" earlier in the commandline. I think that should be fine. In particular QEMU's qemu_init() function has a call to user_register_global_props() which handles all global properties at the same time, so I think changing the order should be fine in (almost?) all cases. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 2/8] config to command: add one '-global' option for each flag 2025-03-06 12:55 ` Fiona Ebner @ 2025-03-07 9:54 ` Dominik Csapak 2025-03-07 10:00 ` Fiona Ebner 0 siblings, 1 reply; 35+ messages in thread From: Dominik Csapak @ 2025-03-07 9:54 UTC (permalink / raw) To: Fiona Ebner, Proxmox VE development discussion On 3/6/25 13:55, Fiona Ebner wrote: > Am 06.03.25 um 13:15 schrieb Dominik Csapak: >> On 3/6/25 13:13, Fiona Ebner wrote: >>> Am 06.03.25 um 11:44 schrieb Dominik Csapak: >>>> If we have multiple 'globalFlags', we have to encode each one separately >>>> on the commandline with '-global OPTION', since QEMU does not allow to >>>> have multiple options here. >>>> >>>> We currently only have one such flag that used the 'globalFlags' list, >>>> so it never popped up. (All other uses directly add an option to the >>>> commandline) >>>> >>>> Avoid future bugs by fixing it now. >>>> >>> >>> So there is no real point to collecting the flags in the first place? >>> I.e. we could also get rid of the variable and have the single current >>> user of the variable add the flag directly on the commandline too. Or >>> otherwise, we could change the other users and collect all flags with >>> this variable. Pre-existing of course, but ideally, we could avoid the >>> mishmash. >>> >> >> Sorry this could have been more clear here: >> I add to the flags in one of the following patches, so i sent this >> in preparation of that (could possibly be squashed) > > Yes, I understand that. I still think the status quo with mixing two > different approaches might not be best. It's not going to be a blocker > for the series, but I wanted to mention it, if you want to go for > avoiding it. > >> I did not want to touch the other places, since that in turn changes >> the order of the qemu commandline (which sometimes has unintended side >> effects, e.g. in combination with the 'args' parameter) > > Are you sure? Custom 'args' are always added last so that shouldn't matter. > > The only thing that would change by removing the global flags variable > is having "-global kvm-pit.lost_tick_policy=discard" earlier in the > commandline. I think that should be fine. In particular QEMU's > qemu_init() function has a call to user_register_global_props() which > handles all global properties at the same time, so I think changing the > order should be fine in (almost?) all cases. I'll test that, but imho it would better to do the reverse here? So don't interject '-gloabl' parameters throughout config2command, but add them to the globalFlags and output them together at the end? we'd have to touch the same number of tests i think, but it seems less confusing to me (also in the resulting commandline we'd have all global options together then) Or is there a better argument for injecting the global parameters in the middle? _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 2/8] config to command: add one '-global' option for each flag 2025-03-07 9:54 ` Dominik Csapak @ 2025-03-07 10:00 ` Fiona Ebner 2025-03-07 10:05 ` Dominik Csapak 0 siblings, 1 reply; 35+ messages in thread From: Fiona Ebner @ 2025-03-07 10:00 UTC (permalink / raw) To: Dominik Csapak, Proxmox VE development discussion Am 07.03.25 um 10:54 schrieb Dominik Csapak: > On 3/6/25 13:55, Fiona Ebner wrote: >> Am 06.03.25 um 13:15 schrieb Dominik Csapak: >>> On 3/6/25 13:13, Fiona Ebner wrote: >>>> Am 06.03.25 um 11:44 schrieb Dominik Csapak: >>>>> If we have multiple 'globalFlags', we have to encode each one >>>>> separately >>>>> on the commandline with '-global OPTION', since QEMU does not allow to >>>>> have multiple options here. >>>>> >>>>> We currently only have one such flag that used the 'globalFlags' list, >>>>> so it never popped up. (All other uses directly add an option to the >>>>> commandline) >>>>> >>>>> Avoid future bugs by fixing it now. >>>>> >>>> >>>> So there is no real point to collecting the flags in the first place? >>>> I.e. we could also get rid of the variable and have the single current >>>> user of the variable add the flag directly on the commandline too. Or >>>> otherwise, we could change the other users and collect all flags with >>>> this variable. Pre-existing of course, but ideally, we could avoid the >>>> mishmash. >>>> >>> >>> Sorry this could have been more clear here: >>> I add to the flags in one of the following patches, so i sent this >>> in preparation of that (could possibly be squashed) >> >> Yes, I understand that. I still think the status quo with mixing two >> different approaches might not be best. It's not going to be a blocker >> for the series, but I wanted to mention it, if you want to go for >> avoiding it. >> >>> I did not want to touch the other places, since that in turn changes >>> the order of the qemu commandline (which sometimes has unintended side >>> effects, e.g. in combination with the 'args' parameter) >> >> Are you sure? Custom 'args' are always added last so that shouldn't >> matter. >> >> The only thing that would change by removing the global flags variable >> is having "-global kvm-pit.lost_tick_policy=discard" earlier in the >> commandline. I think that should be fine. In particular QEMU's >> qemu_init() function has a call to user_register_global_props() which >> handles all global properties at the same time, so I think changing the >> order should be fine in (almost?) all cases. > > I'll test that, but imho it would better to do the reverse here? > So don't interject '-gloabl' parameters throughout config2command, but > add them to the globalFlags and output them together at the end? > > we'd have to touch the same number of tests i think, but it seems less > confusing to me (also in the resulting commandline we'd have all > global options together then) > > Or is there a better argument for injecting the global parameters > in the middle? It avoids the need for the variable to collect and passing it around and to remember adding future ones to that variable too. It doesn't make a difference from QEMUs perspective, but would slightly improve readability for humans looking at the commandline. Note that I already suggested collecting all in the variable as an approach above. I just want to avoid the mishmash. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 2/8] config to command: add one '-global' option for each flag 2025-03-07 10:00 ` Fiona Ebner @ 2025-03-07 10:05 ` Dominik Csapak 2025-03-07 10:14 ` Fiona Ebner 0 siblings, 1 reply; 35+ messages in thread From: Dominik Csapak @ 2025-03-07 10:05 UTC (permalink / raw) To: Fiona Ebner, Proxmox VE development discussion On 3/7/25 11:00, Fiona Ebner wrote: > Am 07.03.25 um 10:54 schrieb Dominik Csapak: >> On 3/6/25 13:55, Fiona Ebner wrote: >>> Am 06.03.25 um 13:15 schrieb Dominik Csapak: >>>> On 3/6/25 13:13, Fiona Ebner wrote: >>>>> Am 06.03.25 um 11:44 schrieb Dominik Csapak: >>>>>> If we have multiple 'globalFlags', we have to encode each one >>>>>> separately >>>>>> on the commandline with '-global OPTION', since QEMU does not allow to >>>>>> have multiple options here. >>>>>> >>>>>> We currently only have one such flag that used the 'globalFlags' list, >>>>>> so it never popped up. (All other uses directly add an option to the >>>>>> commandline) >>>>>> >>>>>> Avoid future bugs by fixing it now. >>>>>> >>>>> >>>>> So there is no real point to collecting the flags in the first place? >>>>> I.e. we could also get rid of the variable and have the single current >>>>> user of the variable add the flag directly on the commandline too. Or >>>>> otherwise, we could change the other users and collect all flags with >>>>> this variable. Pre-existing of course, but ideally, we could avoid the >>>>> mishmash. >>>>> >>>> >>>> Sorry this could have been more clear here: >>>> I add to the flags in one of the following patches, so i sent this >>>> in preparation of that (could possibly be squashed) >>> >>> Yes, I understand that. I still think the status quo with mixing two >>> different approaches might not be best. It's not going to be a blocker >>> for the series, but I wanted to mention it, if you want to go for >>> avoiding it. >>> >>>> I did not want to touch the other places, since that in turn changes >>>> the order of the qemu commandline (which sometimes has unintended side >>>> effects, e.g. in combination with the 'args' parameter) >>> >>> Are you sure? Custom 'args' are always added last so that shouldn't >>> matter. >>> >>> The only thing that would change by removing the global flags variable >>> is having "-global kvm-pit.lost_tick_policy=discard" earlier in the >>> commandline. I think that should be fine. In particular QEMU's >>> qemu_init() function has a call to user_register_global_props() which >>> handles all global properties at the same time, so I think changing the >>> order should be fine in (almost?) all cases. >> >> I'll test that, but imho it would better to do the reverse here? >> So don't interject '-gloabl' parameters throughout config2command, but >> add them to the globalFlags and output them together at the end? >> >> we'd have to touch the same number of tests i think, but it seems less >> confusing to me (also in the resulting commandline we'd have all >> global options together then) >> >> Or is there a better argument for injecting the global parameters >> in the middle? > > It avoids the need for the variable to collect and passing it around and > to remember adding future ones to that variable too. > > It doesn't make a difference from QEMUs perspective, but would slightly > improve readability for humans looking at the commandline. > > Note that I already suggested collecting all in the variable as an > approach above. I just want to avoid the mishmash. OK, which approach would you prefer? (I don't have a strong feeling in either direction and doing the cleanup now seems to not be a lot of work) _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 2/8] config to command: add one '-global' option for each flag 2025-03-07 10:05 ` Dominik Csapak @ 2025-03-07 10:14 ` Fiona Ebner 0 siblings, 0 replies; 35+ messages in thread From: Fiona Ebner @ 2025-03-07 10:14 UTC (permalink / raw) To: Dominik Csapak, Proxmox VE development discussion Am 07.03.25 um 11:05 schrieb Dominik Csapak: > On 3/7/25 11:00, Fiona Ebner wrote: >> Am 07.03.25 um 10:54 schrieb Dominik Csapak: >>> On 3/6/25 13:55, Fiona Ebner wrote: >>>> Am 06.03.25 um 13:15 schrieb Dominik Csapak: >>>>> On 3/6/25 13:13, Fiona Ebner wrote: >>>>>> Am 06.03.25 um 11:44 schrieb Dominik Csapak: >>>>>>> If we have multiple 'globalFlags', we have to encode each one >>>>>>> separately >>>>>>> on the commandline with '-global OPTION', since QEMU does not >>>>>>> allow to >>>>>>> have multiple options here. >>>>>>> >>>>>>> We currently only have one such flag that used the 'globalFlags' >>>>>>> list, >>>>>>> so it never popped up. (All other uses directly add an option to the >>>>>>> commandline) >>>>>>> >>>>>>> Avoid future bugs by fixing it now. >>>>>>> >>>>>> >>>>>> So there is no real point to collecting the flags in the first place? >>>>>> I.e. we could also get rid of the variable and have the single >>>>>> current >>>>>> user of the variable add the flag directly on the commandline too. Or >>>>>> otherwise, we could change the other users and collect all flags with >>>>>> this variable. Pre-existing of course, but ideally, we could avoid >>>>>> the >>>>>> mishmash. >>>>>> >>>>> >>>>> Sorry this could have been more clear here: >>>>> I add to the flags in one of the following patches, so i sent this >>>>> in preparation of that (could possibly be squashed) >>>> >>>> Yes, I understand that. I still think the status quo with mixing two >>>> different approaches might not be best. It's not going to be a blocker >>>> for the series, but I wanted to mention it, if you want to go for >>>> avoiding it. >>>> >>>>> I did not want to touch the other places, since that in turn changes >>>>> the order of the qemu commandline (which sometimes has unintended side >>>>> effects, e.g. in combination with the 'args' parameter) >>>> >>>> Are you sure? Custom 'args' are always added last so that shouldn't >>>> matter. >>>> >>>> The only thing that would change by removing the global flags variable >>>> is having "-global kvm-pit.lost_tick_policy=discard" earlier in the >>>> commandline. I think that should be fine. In particular QEMU's >>>> qemu_init() function has a call to user_register_global_props() which >>>> handles all global properties at the same time, so I think changing the >>>> order should be fine in (almost?) all cases. >>> >>> I'll test that, but imho it would better to do the reverse here? >>> So don't interject '-gloabl' parameters throughout config2command, but >>> add them to the globalFlags and output them together at the end? >>> >>> we'd have to touch the same number of tests i think, but it seems less >>> confusing to me (also in the resulting commandline we'd have all >>> global options together then) >>> >>> Or is there a better argument for injecting the global parameters >>> in the middle? >> >> It avoids the need for the variable to collect and passing it around and >> to remember adding future ones to that variable too. >> >> It doesn't make a difference from QEMUs perspective, but would slightly >> improve readability for humans looking at the commandline. >> >> Note that I already suggested collecting all in the variable as an >> approach above. I just want to avoid the mishmash. > > OK, which approach would you prefer? (I don't have a strong feeling in > either direction > and doing the cleanup now seems to not be a lot of work) > I don't have a strong opinion either, but slightly prefer the no-variable/collect approach. Just because cfg2cmd is already quite bloated. We can always switch to collecting again, if we continue adding global flags and think it's worth for readability on the cmdline. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* [pve-devel] [PATCH qemu-server 3/8] meta info: also add current pve-machine version 2025-03-06 10:44 [pve-devel] [PATCH qemu-server 0/8] disable S3/S4 power states by default Dominik Csapak 2025-03-06 10:44 ` [pve-devel] [RFC PATCH qemu-server 1/8] tests: cfg2cmd: pin QEMU version Dominik Csapak 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 2/8] config to command: add one '-global' option for each flag Dominik Csapak @ 2025-03-06 10:44 ` Dominik Csapak 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 4/8] machine: correctly select pve machine version for non pinned windows guests Dominik Csapak ` (5 subsequent siblings) 8 siblings, 0 replies; 35+ messages in thread From: Dominik Csapak @ 2025-03-06 10:44 UTC (permalink / raw) To: pve-devel when we bump the pve machine version, we also want to include that info in the meta creation info. With that we can pin guests to the specific version they were created on. Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- we could also give the kvm_version as parameter directly here, since we already need it... PVE/API2/Qemu.pm | 4 +++- PVE/QemuServer/MetaInfo.pm | 28 +++++++++++++++++++--------- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm index dc8915a7..0249c0d0 100644 --- a/PVE/API2/Qemu.pm +++ b/PVE/API2/Qemu.pm @@ -1207,7 +1207,9 @@ __PACKAGE__->register_method({ assert_scsi_feature_compatibility($opt, $conf, $storecfg, $param->{$opt}); } - $conf->{meta} = PVE::QemuServer::MetaInfo::new_meta_info_string(); + my $kvm_version = PVE::QemuServer::Helpers::kvm_user_version(); + my $pve_machine = PVE::QemuServer::Machine::get_pve_version($kvm_version); + $conf->{meta} = PVE::QemuServer::MetaInfo::new_meta_info_string($pve_machine); my $vollist = []; eval { diff --git a/PVE/QemuServer/MetaInfo.pm b/PVE/QemuServer/MetaInfo.pm index a8cb6c5e..78526eb2 100644 --- a/PVE/QemuServer/MetaInfo.pm +++ b/PVE/QemuServer/MetaInfo.pm @@ -20,6 +20,13 @@ our $meta_info_fmt = { pattern => '\d+(\.\d+)+', optional => 1, }, + 'creation-pve-machine' => { + type => 'integer', + description => "The PVE machine version current at the time this VM was created.", + default => 0, + minimum => 1, + optional => 1, + }, }; sub parse_meta_info { @@ -33,15 +40,18 @@ sub parse_meta_info { } sub new_meta_info_string { - my () = @_; # for now do not allow to override any value - - return PVE::JSONSchema::print_property_string( - { - 'creation-qemu' => PVE::QemuServer::Helpers::kvm_user_version(), - ctime => "". int(time()), - }, - $meta_info_fmt, - ); + my ($pve_machine) = @_; + + my $data = { + 'creation-qemu' => PVE::QemuServer::Helpers::kvm_user_version(), + ctime => "". int(time()), + }; + + if (defined($pve_machine) && $pve_machine > 0) { + $data->{'creation-pve-machine'} = $pve_machine; + } + + return PVE::JSONSchema::print_property_string($data, $meta_info_fmt); } 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] 35+ messages in thread
* [pve-devel] [PATCH qemu-server 4/8] machine: correctly select pve machine version for non pinned windows guests 2025-03-06 10:44 [pve-devel] [PATCH qemu-server 0/8] disable S3/S4 power states by default Dominik Csapak ` (2 preceding siblings ...) 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 3/8] meta info: also add current pve-machine version Dominik Csapak @ 2025-03-06 10:44 ` Dominik Csapak 2025-03-06 13:10 ` Fiona Ebner 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 5/8] machine: incorporate pve machine version when pinning " Dominik Csapak ` (4 subsequent siblings) 8 siblings, 1 reply; 35+ messages in thread From: Dominik Csapak @ 2025-03-06 10:44 UTC (permalink / raw) To: pve-devel when we don't have a specific machine version on a windows guest, we use the creation meta info to pin the machine version. Currently we always append the pve machine version from the current installed kvm version, which is not necessarily the version we pinned the guest to. Instead, use either the info from the creation meta info if it exists, or use 'pve0'. For non-windows machines, we used the current QEMU machine version so we should use the pve machine version from that too. Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- PVE/QemuServer/Machine.pm | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm index f1acde8f..e3da8e21 100644 --- a/PVE/QemuServer/Machine.pm +++ b/PVE/QemuServer/Machine.pm @@ -237,14 +237,19 @@ sub get_vm_machine { if (PVE::QemuServer::Helpers::min_version($meta->{'creation-qemu'}, 9, 1)) { # need only major.minor ($base_version) = ($meta->{'creation-qemu'} =~ m/^(\d+.\d+)/); + # append pve machine version if we have one + if (my $pvever = $meta->{'creation-pve-machine'}) { + $base_version .= "+pve$pvever" + } } } $machine = windows_get_pinned_machine_version($machine, $base_version, $kvmversion); + } else{ + $arch //= 'x86_64'; + $machine ||= default_machine_for_arch($arch); + my $pvever = get_pve_version($kvmversion); + $machine .= "+pve$pvever"; } - $arch //= 'x86_64'; - $machine ||= default_machine_for_arch($arch); - my $pvever = get_pve_version($kvmversion); - $machine .= "+pve$pvever"; } if ($machine !~ m/\+pve\d+?(?:\.pxe)?$/) { -- 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] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 4/8] machine: correctly select pve machine version for non pinned windows guests 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 4/8] machine: correctly select pve machine version for non pinned windows guests Dominik Csapak @ 2025-03-06 13:10 ` Fiona Ebner 2025-03-06 13:36 ` Dominik Csapak 0 siblings, 1 reply; 35+ messages in thread From: Fiona Ebner @ 2025-03-06 13:10 UTC (permalink / raw) To: Proxmox VE development discussion, Dominik Csapak Am 06.03.25 um 11:44 schrieb Dominik Csapak: > when we don't have a specific machine version on a windows guest, we use > the creation meta info to pin the machine version. Currently we always > append the pve machine version from the current installed kvm version, > which is not necessarily the version we pinned the guest to. > > Instead, use either the info from the creation meta info if it exists, > or use 'pve0'. > > For non-windows machines, we used the current QEMU machine version so we > should use the pve machine version from that too. > > Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> > --- > PVE/QemuServer/Machine.pm | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm > index f1acde8f..e3da8e21 100644 > --- a/PVE/QemuServer/Machine.pm > +++ b/PVE/QemuServer/Machine.pm > @@ -237,14 +237,19 @@ sub get_vm_machine { > if (PVE::QemuServer::Helpers::min_version($meta->{'creation-qemu'}, 9, 1)) { > # need only major.minor > ($base_version) = ($meta->{'creation-qemu'} =~ m/^(\d+.\d+)/); > + # append pve machine version if we have one > + if (my $pvever = $meta->{'creation-pve-machine'}) { > + $base_version .= "+pve$pvever" Since this is only the fallback handling for the rare edge case where no explicit machine version is set for a Windows guest, not sure if it's even worth doing this. I.e. can we just avoid the additional meta property and always use pve0 here? Did you intend any other use for the creation-pve-machine? Further below we already have: > # for version-pinned machines that do not include a pve-version (e.g. > # pc-q35-4.1), we assume 0 to keep them stable in case we bump > $machine .= '+pve0'; so let's just follow this here too? > + } > } > } > $machine = windows_get_pinned_machine_version($machine, $base_version, $kvmversion); > + } else{ > + $arch //= 'x86_64'; > + $machine ||= default_machine_for_arch($arch); > + my $pvever = get_pve_version($kvmversion); > + $machine .= "+pve$pvever"; > } > - $arch //= 'x86_64'; > - $machine ||= default_machine_for_arch($arch); > - my $pvever = get_pve_version($kvmversion); > - $machine .= "+pve$pvever"; > } > > if ($machine !~ m/\+pve\d+?(?:\.pxe)?$/) { _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 4/8] machine: correctly select pve machine version for non pinned windows guests 2025-03-06 13:10 ` Fiona Ebner @ 2025-03-06 13:36 ` Dominik Csapak 2025-03-06 14:10 ` Fiona Ebner 0 siblings, 1 reply; 35+ messages in thread From: Dominik Csapak @ 2025-03-06 13:36 UTC (permalink / raw) To: Fiona Ebner, Proxmox VE development discussion On 3/6/25 14:10, Fiona Ebner wrote: > Am 06.03.25 um 11:44 schrieb Dominik Csapak: >> when we don't have a specific machine version on a windows guest, we use >> the creation meta info to pin the machine version. Currently we always >> append the pve machine version from the current installed kvm version, >> which is not necessarily the version we pinned the guest to. >> >> Instead, use either the info from the creation meta info if it exists, >> or use 'pve0'. >> >> For non-windows machines, we used the current QEMU machine version so we >> should use the pve machine version from that too. >> >> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> >> --- >> PVE/QemuServer/Machine.pm | 13 +++++++++---- >> 1 file changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm >> index f1acde8f..e3da8e21 100644 >> --- a/PVE/QemuServer/Machine.pm >> +++ b/PVE/QemuServer/Machine.pm >> @@ -237,14 +237,19 @@ sub get_vm_machine { >> if (PVE::QemuServer::Helpers::min_version($meta->{'creation-qemu'}, 9, 1)) { >> # need only major.minor >> ($base_version) = ($meta->{'creation-qemu'} =~ m/^(\d+.\d+)/); >> + # append pve machine version if we have one >> + if (my $pvever = $meta->{'creation-pve-machine'}) { >> + $base_version .= "+pve$pvever" > > Since this is only the fallback handling for the rare edge case where no > explicit machine version is set for a Windows guest, not sure if it's > even worth doing this. I.e. can we just avoid the additional meta > property and always use pve0 here? Did you intend any other use for the > creation-pve-machine? > I though the intention of the code was to pin the guest to that version where it was created. If we omit the machine version, this is not true anymore, since I'd then create a windows vm with e.g. pc-q35-9.2+pve1 then set it to q35, and would get pc-q35-9.2+pve0 and while i don't have anymore use cases for the current case, wouldn't this approach here correct also when we decide to bump again in the future? also recording with which pve version the vm was created could help in debugging issues at some point.. (my patch only adds the version in the meta info if it's > 0 anyway) > Further below we already have: > >> # for version-pinned machines that do not include a pve-version (e.g. >> # pc-q35-4.1), we assume 0 to keep them stable in case we bump >> $machine .= '+pve0'; > > so let's just follow this here too? this here make sense IMHO, since the user wanted a specific version but without pve versions info, so default to pve0 sounds fine to me? > > >> + } >> } >> } >> $machine = windows_get_pinned_machine_version($machine, $base_version, $kvmversion); >> + } else{ >> + $arch //= 'x86_64'; >> + $machine ||= default_machine_for_arch($arch); >> + my $pvever = get_pve_version($kvmversion); >> + $machine .= "+pve$pvever"; >> } >> - $arch //= 'x86_64'; >> - $machine ||= default_machine_for_arch($arch); >> - my $pvever = get_pve_version($kvmversion); >> - $machine .= "+pve$pvever"; >> } >> >> if ($machine !~ m/\+pve\d+?(?:\.pxe)?$/) { > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 4/8] machine: correctly select pve machine version for non pinned windows guests 2025-03-06 13:36 ` Dominik Csapak @ 2025-03-06 14:10 ` Fiona Ebner 2025-03-06 14:14 ` Fiona Ebner 2025-03-06 14:15 ` Dominik Csapak 0 siblings, 2 replies; 35+ messages in thread From: Fiona Ebner @ 2025-03-06 14:10 UTC (permalink / raw) To: Dominik Csapak, Proxmox VE development discussion Am 06.03.25 um 14:36 schrieb Dominik Csapak: > On 3/6/25 14:10, Fiona Ebner wrote: >> Am 06.03.25 um 11:44 schrieb Dominik Csapak: >>> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm >>> index f1acde8f..e3da8e21 100644 >>> --- a/PVE/QemuServer/Machine.pm >>> +++ b/PVE/QemuServer/Machine.pm >>> @@ -237,14 +237,19 @@ sub get_vm_machine { >>> if (PVE::QemuServer::Helpers::min_version($meta- >>> >{'creation-qemu'}, 9, 1)) { >>> # need only major.minor >>> ($base_version) = ($meta->{'creation-qemu'} =~ m/^(\d+. >>> \d+)/); >>> + # append pve machine version if we have one >>> + if (my $pvever = $meta->{'creation-pve-machine'}) { >>> + $base_version .= "+pve$pvever" >> >> Since this is only the fallback handling for the rare edge case where no >> explicit machine version is set for a Windows guest, not sure if it's >> even worth doing this. I.e. can we just avoid the additional meta >> property and always use pve0 here? Did you intend any other use for the >> creation-pve-machine? >> > > I though the intention of the code was to pin the guest to that version > where it > was created. If we omit the machine version, this is not true anymore, > since I'd then create a windows vm with e.g. pc-q35-9.2+pve1 then set it > to q35, and would get pc-q35-9.2+pve0 Fair enough. There might be people manually changing windows guests to use the latest machine even if we don't recommend it or allow it in the UI. Still, it would just mean a pve machine version downgrade for subsequent boots, which also can happen if you offline migrate to a node that does not yet support the newest pve machine version. > > and while i don't have anymore use cases for the current case, wouldn't > this approach here correct also when we decide to bump again in the future? Correct what? > > also recording with which pve version the vm was created could help in > debugging > issues at some point.. (my patch only adds the version in the meta info > if it's > 0 anyway) > In principle, yes. But honestly, it's much less informative than the creation QEMU version, and even that is rather rarely useful for debugging in my experience. So not a huge fan, since I'd find it nicer to keep the meta info slick, but we can go for it if you really think it's worth it. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 4/8] machine: correctly select pve machine version for non pinned windows guests 2025-03-06 14:10 ` Fiona Ebner @ 2025-03-06 14:14 ` Fiona Ebner 2025-03-06 14:15 ` Dominik Csapak 1 sibling, 0 replies; 35+ messages in thread From: Fiona Ebner @ 2025-03-06 14:14 UTC (permalink / raw) To: Dominik Csapak, Proxmox VE development discussion Am 06.03.25 um 15:10 schrieb Fiona Ebner: > Am 06.03.25 um 14:36 schrieb Dominik Csapak: >> On 3/6/25 14:10, Fiona Ebner wrote: >>> Am 06.03.25 um 11:44 schrieb Dominik Csapak: >>>> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm >>>> index f1acde8f..e3da8e21 100644 >>>> --- a/PVE/QemuServer/Machine.pm >>>> +++ b/PVE/QemuServer/Machine.pm >>>> @@ -237,14 +237,19 @@ sub get_vm_machine { >>>> if (PVE::QemuServer::Helpers::min_version($meta- >>>>> {'creation-qemu'}, 9, 1)) { >>>> # need only major.minor >>>> ($base_version) = ($meta->{'creation-qemu'} =~ m/^(\d+. >>>> \d+)/); >>>> + # append pve machine version if we have one >>>> + if (my $pvever = $meta->{'creation-pve-machine'}) { >>>> + $base_version .= "+pve$pvever" >>> >>> Since this is only the fallback handling for the rare edge case where no >>> explicit machine version is set for a Windows guest, not sure if it's >>> even worth doing this. I.e. can we just avoid the additional meta >>> property and always use pve0 here? Did you intend any other use for the >>> creation-pve-machine? >>> >> >> I though the intention of the code was to pin the guest to that version >> where it >> was created. If we omit the machine version, this is not true anymore, >> since I'd then create a windows vm with e.g. pc-q35-9.2+pve1 then set it >> to q35, and would get pc-q35-9.2+pve0 > > Fair enough. There might be people manually changing windows guests to > use the latest machine even if we don't recommend it or allow it in the > UI. Still, it would just mean a pve machine version downgrade for > subsequent boots, which also can happen if you offline migrate to a node > that does not yet support the newest pve machine version. No wait, if you set it to q35 it will mean "use the fallback" not "use the latest". So that is not even a valid use case I had in mind. And we can define the fallback to be just with pve0 always. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 4/8] machine: correctly select pve machine version for non pinned windows guests 2025-03-06 14:10 ` Fiona Ebner 2025-03-06 14:14 ` Fiona Ebner @ 2025-03-06 14:15 ` Dominik Csapak 2025-03-06 14:20 ` Fiona Ebner 1 sibling, 1 reply; 35+ messages in thread From: Dominik Csapak @ 2025-03-06 14:15 UTC (permalink / raw) To: Fiona Ebner, Proxmox VE development discussion On 3/6/25 15:10, Fiona Ebner wrote: > Am 06.03.25 um 14:36 schrieb Dominik Csapak: >> On 3/6/25 14:10, Fiona Ebner wrote: >>> Am 06.03.25 um 11:44 schrieb Dominik Csapak: >>>> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm >>>> index f1acde8f..e3da8e21 100644 >>>> --- a/PVE/QemuServer/Machine.pm >>>> +++ b/PVE/QemuServer/Machine.pm >>>> @@ -237,14 +237,19 @@ sub get_vm_machine { >>>> if (PVE::QemuServer::Helpers::min_version($meta- >>>>> {'creation-qemu'}, 9, 1)) { >>>> # need only major.minor >>>> ($base_version) = ($meta->{'creation-qemu'} =~ m/^(\d+. >>>> \d+)/); >>>> + # append pve machine version if we have one >>>> + if (my $pvever = $meta->{'creation-pve-machine'}) { >>>> + $base_version .= "+pve$pvever" >>> >>> Since this is only the fallback handling for the rare edge case where no >>> explicit machine version is set for a Windows guest, not sure if it's >>> even worth doing this. I.e. can we just avoid the additional meta >>> property and always use pve0 here? Did you intend any other use for the >>> creation-pve-machine? >>> >> >> I though the intention of the code was to pin the guest to that version >> where it >> was created. If we omit the machine version, this is not true anymore, >> since I'd then create a windows vm with e.g. pc-q35-9.2+pve1 then set it >> to q35, and would get pc-q35-9.2+pve0 > > Fair enough. There might be people manually changing windows guests to > use the latest machine even if we don't recommend it or allow it in the > UI. Still, it would just mean a pve machine version downgrade for > subsequent boots, which also can happen if you offline migrate to a node > that does not yet support the newest pve machine version. > >> >> and while i don't have anymore use cases for the current case, wouldn't >> this approach here correct also when we decide to bump again in the future? > > Correct what? > i wanted to write 'wouldn't this approach here be correct [..]' >> >> also recording with which pve version the vm was created could help in >> debugging >> issues at some point.. (my patch only adds the version in the meta info >> if it's > 0 anyway) >> > > In principle, yes. But honestly, it's much less informative than the > creation QEMU version, and even that is rather rarely useful for > debugging in my experience. > > So not a huge fan, since I'd find it nicer to keep the meta info slick, > but we can go for it if you really think it's worth it. mhmm i get what you mean, would it be an ok compromise to record the pve version with the creation machine maybe? i'd have to touch the places where we read that ofc but that shouldn't be too many _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 4/8] machine: correctly select pve machine version for non pinned windows guests 2025-03-06 14:15 ` Dominik Csapak @ 2025-03-06 14:20 ` Fiona Ebner 2025-03-07 9:55 ` Dominik Csapak 0 siblings, 1 reply; 35+ messages in thread From: Fiona Ebner @ 2025-03-06 14:20 UTC (permalink / raw) To: Dominik Csapak, Proxmox VE development discussion Am 06.03.25 um 15:15 schrieb Dominik Csapak: > On 3/6/25 15:10, Fiona Ebner wrote: >> Am 06.03.25 um 14:36 schrieb Dominik Csapak: >>> On 3/6/25 14:10, Fiona Ebner wrote: >>>> Am 06.03.25 um 11:44 schrieb Dominik Csapak: >>>>> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm >>>>> index f1acde8f..e3da8e21 100644 >>>>> --- a/PVE/QemuServer/Machine.pm >>>>> +++ b/PVE/QemuServer/Machine.pm >>>>> @@ -237,14 +237,19 @@ sub get_vm_machine { >>>>> if (PVE::QemuServer::Helpers::min_version($meta- >>>>>> {'creation-qemu'}, 9, 1)) { >>>>> # need only major.minor >>>>> ($base_version) = ($meta->{'creation-qemu'} =~ m/^(\d+. >>>>> \d+)/); >>>>> + # append pve machine version if we have one >>>>> + if (my $pvever = $meta->{'creation-pve-machine'}) { >>>>> + $base_version .= "+pve$pvever" >>>> >>>> Since this is only the fallback handling for the rare edge case >>>> where no >>>> explicit machine version is set for a Windows guest, not sure if it's >>>> even worth doing this. I.e. can we just avoid the additional meta >>>> property and always use pve0 here? Did you intend any other use for the >>>> creation-pve-machine? >>>> >>> >>> I though the intention of the code was to pin the guest to that version >>> where it >>> was created. If we omit the machine version, this is not true anymore, >>> since I'd then create a windows vm with e.g. pc-q35-9.2+pve1 then set it >>> to q35, and would get pc-q35-9.2+pve0 >> >> Fair enough. There might be people manually changing windows guests to >> use the latest machine even if we don't recommend it or allow it in the >> UI. Still, it would just mean a pve machine version downgrade for >> subsequent boots, which also can happen if you offline migrate to a node >> that does not yet support the newest pve machine version. >> >>> >>> and while i don't have anymore use cases for the current case, wouldn't >>> this approach here correct also when we decide to bump again in the >>> future? >> >> Correct what? >> > > i wanted to write 'wouldn't this approach here be correct [..]' I never said it's not correct. Just questioning if it's worth it. >>> also recording with which pve version the vm was created could help in >>> debugging >>> issues at some point.. (my patch only adds the version in the meta info >>> if it's > 0 anyway) >>> >> >> In principle, yes. But honestly, it's much less informative than the >> creation QEMU version, and even that is rather rarely useful for >> debugging in my experience. >> >> So not a huge fan, since I'd find it nicer to keep the meta info slick, >> but we can go for it if you really think it's worth it. > > > mhmm i get what you mean, would it be an ok compromise to record the pve > version with > the creation machine maybe? > > i'd have to touch the places where we read that ofc but that shouldn't > be too many > The creation-qemu is the binary version, but the latest pve version is related to the qemu-server version/machine version, so while I get where you're coming from, I'd rather keep them separate. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 4/8] machine: correctly select pve machine version for non pinned windows guests 2025-03-06 14:20 ` Fiona Ebner @ 2025-03-07 9:55 ` Dominik Csapak 0 siblings, 0 replies; 35+ messages in thread From: Dominik Csapak @ 2025-03-07 9:55 UTC (permalink / raw) To: Fiona Ebner, Proxmox VE development discussion On 3/6/25 15:20, Fiona Ebner wrote: > Am 06.03.25 um 15:15 schrieb Dominik Csapak: >> On 3/6/25 15:10, Fiona Ebner wrote: >>> Am 06.03.25 um 14:36 schrieb Dominik Csapak: >>>> On 3/6/25 14:10, Fiona Ebner wrote: >>>>> Am 06.03.25 um 11:44 schrieb Dominik Csapak: >>>>>> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm >>>>>> index f1acde8f..e3da8e21 100644 >>>>>> --- a/PVE/QemuServer/Machine.pm >>>>>> +++ b/PVE/QemuServer/Machine.pm >>>>>> @@ -237,14 +237,19 @@ sub get_vm_machine { >>>>>> if (PVE::QemuServer::Helpers::min_version($meta- >>>>>>> {'creation-qemu'}, 9, 1)) { >>>>>> # need only major.minor >>>>>> ($base_version) = ($meta->{'creation-qemu'} =~ m/^(\d+. >>>>>> \d+)/); >>>>>> + # append pve machine version if we have one >>>>>> + if (my $pvever = $meta->{'creation-pve-machine'}) { >>>>>> + $base_version .= "+pve$pvever" >>>>> >>>>> Since this is only the fallback handling for the rare edge case >>>>> where no >>>>> explicit machine version is set for a Windows guest, not sure if it's >>>>> even worth doing this. I.e. can we just avoid the additional meta >>>>> property and always use pve0 here? Did you intend any other use for the >>>>> creation-pve-machine? >>>>> >>>> >>>> I though the intention of the code was to pin the guest to that version >>>> where it >>>> was created. If we omit the machine version, this is not true anymore, >>>> since I'd then create a windows vm with e.g. pc-q35-9.2+pve1 then set it >>>> to q35, and would get pc-q35-9.2+pve0 >>> >>> Fair enough. There might be people manually changing windows guests to >>> use the latest machine even if we don't recommend it or allow it in the >>> UI. Still, it would just mean a pve machine version downgrade for >>> subsequent boots, which also can happen if you offline migrate to a node >>> that does not yet support the newest pve machine version. >>> >>>> >>>> and while i don't have anymore use cases for the current case, wouldn't >>>> this approach here correct also when we decide to bump again in the >>>> future? >>> >>> Correct what? >>> >> >> i wanted to write 'wouldn't this approach here be correct [..]' > > I never said it's not correct. Just questioning if it's worth it. > >>>> also recording with which pve version the vm was created could help in >>>> debugging >>>> issues at some point.. (my patch only adds the version in the meta info >>>> if it's > 0 anyway) >>>> >>> >>> In principle, yes. But honestly, it's much less informative than the >>> creation QEMU version, and even that is rather rarely useful for >>> debugging in my experience. >>> >>> So not a huge fan, since I'd find it nicer to keep the meta info slick, >>> but we can go for it if you really think it's worth it. >> >> >> mhmm i get what you mean, would it be an ok compromise to record the pve >> version with >> the creation machine maybe? >> >> i'd have to touch the places where we read that ofc but that shouldn't >> be too many >> > > The creation-qemu is the binary version, but the latest pve version is > related to the qemu-server version/machine version, so while I get where > you're coming from, I'd rather keep them separate. Ok, I'll send a new version without recording the PVE version _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* [pve-devel] [PATCH qemu-server 5/8] machine: incorporate pve machine version when pinning windows guests 2025-03-06 10:44 [pve-devel] [PATCH qemu-server 0/8] disable S3/S4 power states by default Dominik Csapak ` (3 preceding siblings ...) 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 4/8] machine: correctly select pve machine version for non pinned windows guests Dominik Csapak @ 2025-03-06 10:44 ` Dominik Csapak 2025-03-06 14:32 ` Fiona Ebner 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 6/8] machine: add S3/S4 power state properties Dominik Csapak ` (3 subsequent siblings) 8 siblings, 1 reply; 35+ messages in thread From: Dominik Csapak @ 2025-03-06 10:44 UTC (permalink / raw) To: pve-devel When creating or updating guests with ostype windows, we want to pin the machine version to a specific one. Since introduction of that feature, we never bumped the pve machine version, so this was missing. Append the pve machine version if it's not 0 so we don't add that unnecessarily. Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- PVE/QemuServer/Machine.pm | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm index e3da8e21..ebaf2dcc 100644 --- a/PVE/QemuServer/Machine.pm +++ b/PVE/QemuServer/Machine.pm @@ -274,7 +274,17 @@ sub check_and_pin_machine_string { if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) { # always pin Windows' machine version on create, they get confused too easily if (PVE::QemuServer::Helpers::windows_version($ostype)) { - $machine_conf->{type} = windows_get_pinned_machine_version($machine); + my $kvmversion = PVE::QemuServer::Helpers::kvm_user_version(); + my $pin_version = get_installed_machine_version($kvmversion); + + # pin to the current pveX version to make use of most current features if > 0 + my $pvever = get_pve_version($kvmversion); + if ($pvever > 0) { + $pin_version .= "+pve$pvever"; + } + + $machine_conf->{type} = windows_get_pinned_machine_version($machine, $pin_version, $kvmversion); + print "pinning machine type to '$machine_conf->{type}' for Windows guest OS\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] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 5/8] machine: incorporate pve machine version when pinning windows guests 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 5/8] machine: incorporate pve machine version when pinning " Dominik Csapak @ 2025-03-06 14:32 ` Fiona Ebner 2025-03-07 9:58 ` Dominik Csapak 0 siblings, 1 reply; 35+ messages in thread From: Fiona Ebner @ 2025-03-06 14:32 UTC (permalink / raw) To: Proxmox VE development discussion, Dominik Csapak Am 06.03.25 um 11:44 schrieb Dominik Csapak: > When creating or updating guests with ostype windows, we want to pin the > machine version to a specific one. Since introduction of that feature, > we never bumped the pve machine version, so this was missing. > > Append the pve machine version if it's not 0 so we don't add that > unnecessarily. > > Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> > --- > PVE/QemuServer/Machine.pm | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm > index e3da8e21..ebaf2dcc 100644 > --- a/PVE/QemuServer/Machine.pm > +++ b/PVE/QemuServer/Machine.pm > @@ -274,7 +274,17 @@ sub check_and_pin_machine_string { > if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) { > # always pin Windows' machine version on create, they get confused too easily > if (PVE::QemuServer::Helpers::windows_version($ostype)) { > - $machine_conf->{type} = windows_get_pinned_machine_version($machine); > + my $kvmversion = PVE::QemuServer::Helpers::kvm_user_version(); > + my $pin_version = get_installed_machine_version($kvmversion); Nit: I'd call this $base_version like the argument it's passed-in as. It's not necessarily the version that is going to be pinned. > + > + # pin to the current pveX version to make use of most current features if > 0 > + my $pvever = get_pve_version($kvmversion); > + if ($pvever > 0) { > + $pin_version .= "+pve$pvever"; > + } I feel this logic should go into windows_get_pinned_machine_version() itself. > + > + $machine_conf->{type} = windows_get_pinned_machine_version($machine, $pin_version, $kvmversion); > + > print "pinning machine type to '$machine_conf->{type}' for Windows guest OS\n"; > } > } _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 5/8] machine: incorporate pve machine version when pinning windows guests 2025-03-06 14:32 ` Fiona Ebner @ 2025-03-07 9:58 ` Dominik Csapak 2025-03-07 10:06 ` Fiona Ebner 0 siblings, 1 reply; 35+ messages in thread From: Dominik Csapak @ 2025-03-07 9:58 UTC (permalink / raw) To: Fiona Ebner, Proxmox VE development discussion On 3/6/25 15:32, Fiona Ebner wrote: > Am 06.03.25 um 11:44 schrieb Dominik Csapak: >> When creating or updating guests with ostype windows, we want to pin the >> machine version to a specific one. Since introduction of that feature, >> we never bumped the pve machine version, so this was missing. >> >> Append the pve machine version if it's not 0 so we don't add that >> unnecessarily. >> >> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> >> --- >> PVE/QemuServer/Machine.pm | 12 +++++++++++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm >> index e3da8e21..ebaf2dcc 100644 >> --- a/PVE/QemuServer/Machine.pm >> +++ b/PVE/QemuServer/Machine.pm >> @@ -274,7 +274,17 @@ sub check_and_pin_machine_string { >> if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) { >> # always pin Windows' machine version on create, they get confused too easily >> if (PVE::QemuServer::Helpers::windows_version($ostype)) { >> - $machine_conf->{type} = windows_get_pinned_machine_version($machine); >> + my $kvmversion = PVE::QemuServer::Helpers::kvm_user_version(); >> + my $pin_version = get_installed_machine_version($kvmversion); > > Nit: I'd call this $base_version like the argument it's passed-in as. > It's not necessarily the version that is going to be pinned. sure> >> + >> + # pin to the current pveX version to make use of most current features if > 0 >> + my $pvever = get_pve_version($kvmversion); >> + if ($pvever > 0) { >> + $pin_version .= "+pve$pvever"; >> + } > > I feel this logic should go into windows_get_pinned_machine_version() > itself. > if we'd do that, we'd automatically get the newest pve version for the windows workaround in get_vm_machine, which is the reverse you suggested IIUC ? (e.g. use pve0 in that case) and using a parameter to control that seem weird for this one call... >> + >> + $machine_conf->{type} = windows_get_pinned_machine_version($machine, $pin_version, $kvmversion); >> + >> print "pinning machine type to '$machine_conf->{type}' for Windows guest OS\n"; >> } >> } > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 5/8] machine: incorporate pve machine version when pinning windows guests 2025-03-07 9:58 ` Dominik Csapak @ 2025-03-07 10:06 ` Fiona Ebner 0 siblings, 0 replies; 35+ messages in thread From: Fiona Ebner @ 2025-03-07 10:06 UTC (permalink / raw) To: Dominik Csapak, Proxmox VE development discussion Am 07.03.25 um 10:58 schrieb Dominik Csapak: > On 3/6/25 15:32, Fiona Ebner wrote: >> Am 06.03.25 um 11:44 schrieb Dominik Csapak: >>> When creating or updating guests with ostype windows, we want to pin the >>> machine version to a specific one. Since introduction of that feature, >>> we never bumped the pve machine version, so this was missing. >>> >>> Append the pve machine version if it's not 0 so we don't add that >>> unnecessarily. >>> >>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> >>> --- >>> PVE/QemuServer/Machine.pm | 12 +++++++++++- >>> 1 file changed, 11 insertions(+), 1 deletion(-) >>> >>> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm >>> index e3da8e21..ebaf2dcc 100644 >>> --- a/PVE/QemuServer/Machine.pm >>> +++ b/PVE/QemuServer/Machine.pm >>> @@ -274,7 +274,17 @@ sub check_and_pin_machine_string { >>> if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) { >>> # always pin Windows' machine version on create, they get >>> confused too easily >>> if (PVE::QemuServer::Helpers::windows_version($ostype)) { >>> - $machine_conf->{type} = >>> windows_get_pinned_machine_version($machine); >>> + my $kvmversion = PVE::QemuServer::Helpers::kvm_user_version(); >>> + my $pin_version = get_installed_machine_version($kvmversion); >> >> Nit: I'd call this $base_version like the argument it's passed-in as. >> It's not necessarily the version that is going to be pinned. > > sure> >>> + >>> + # pin to the current pveX version to make use of most >>> current features if > 0 >>> + my $pvever = get_pve_version($kvmversion); >>> + if ($pvever > 0) { >>> + $pin_version .= "+pve$pvever"; >>> + } >> >> I feel this logic should go into windows_get_pinned_machine_version() >> itself. >> > > if we'd do that, we'd automatically get the newest pve version for the > windows workaround in > get_vm_machine, which is the reverse you suggested IIUC ? (e.g. use pve0 > in that case) > and using a parameter to control that seem weird for this one call... > Depends on where exactly you do it. If you do it in the branch when you don't have an explicitly passed-in base version then you don't affect that caller (except if too old QEMU binary, but that is even more of an edge case and I don't see a reason not to use the newest pve version then if we already need to use a pre-creation version). _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* [pve-devel] [PATCH qemu-server 6/8] machine: add S3/S4 power state properties 2025-03-06 10:44 [pve-devel] [PATCH qemu-server 0/8] disable S3/S4 power states by default Dominik Csapak ` (4 preceding siblings ...) 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 5/8] machine: incorporate pve machine version when pinning " Dominik Csapak @ 2025-03-06 10:44 ` Dominik Csapak 2025-03-06 14:52 ` Fiona Ebner 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 7/8] machine: bump pve machine version and reverse the s3/s4 defaults Dominik Csapak ` (2 subsequent siblings) 8 siblings, 1 reply; 35+ messages in thread From: Dominik Csapak @ 2025-03-06 10:44 UTC (permalink / raw) To: pve-devel So users can disable them (they're enabled by default in QEMU) Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- This patch may make sense, regardless if we'll apply the reversal of the default... PVE/QemuServer.pm | 2 ++ PVE/QemuServer/Machine.pm | 40 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index 13af495d..b8ce4750 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -3975,6 +3975,8 @@ sub config_to_command { push @$machineFlags, 'accel=tcg'; } + PVE::QemuServer::Machine::check_and_set_power_state_flags($globalFlags, $machine_conf); + push @$machineFlags, 'smm=off' if should_disable_smm($conf, $vga, $machine_type); my $machine_type_min = $machine_type; diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm index ebaf2dcc..377abc8a 100644 --- a/PVE/QemuServer/Machine.pm +++ b/PVE/QemuServer/Machine.pm @@ -31,6 +31,16 @@ my $machine_fmt = { enum => ['intel', 'virtio'], optional => 1, }, + 'enable-s3' => { + type => 'boolean', + description => "Enables S3 power state. Defaults to true.", + optional => 1, + }, + 'enable-s4' => { + type => 'boolean', + description => "Enables S4 power state. Defaults to true.", + optional => 1, + }, }; PVE::JSONSchema::register_format('pve-qemu-machine-fmt', $machine_fmt); @@ -293,4 +303,34 @@ sub check_and_pin_machine_string { return print_machine($machine_conf); } +# disable s3/s4 by default for 9.2+pve1 machine types +sub check_and_set_power_state_flags { + my ($globalFlags, $machine_conf) = @_; + + my $get_flag = sub { + my ($q35, $type, $value) = @_; + + if ($q35) { + return "ICH9-LPC.disable_${type}=${value}"; + } else { + return "PIIX4_PM.disable_${type}=${value}"; + } + }; + + my $q35 = $machine_conf->{type} && ($machine_conf->{type} =~ m/q35/) ? 1 : 0; + + my $s3 = $machine_conf->{'enable-s3'} // 1; + my $s4 = $machine_conf->{'enable-s4'} // 1; + + # they're enabled by default in QEMU, so only add the flags to disable them + if (!$s3) { + push $globalFlags->@*, $get_flag->($q35, 's3', 1) + } + if (!$s4) { + push $globalFlags->@*, $get_flag->($q35, 's4', 1) + } + + return; +} + 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] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 6/8] machine: add S3/S4 power state properties 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 6/8] machine: add S3/S4 power state properties Dominik Csapak @ 2025-03-06 14:52 ` Fiona Ebner 2025-03-07 10:02 ` Dominik Csapak 0 siblings, 1 reply; 35+ messages in thread From: Fiona Ebner @ 2025-03-06 14:52 UTC (permalink / raw) To: Proxmox VE development discussion, Dominik Csapak Am 06.03.25 um 11:44 schrieb Dominik Csapak: > So users can disable them (they're enabled by default in QEMU) > > Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> > --- > This patch may make sense, regardless if we'll apply the reversal of the > default... > > PVE/QemuServer.pm | 2 ++ > PVE/QemuServer/Machine.pm | 40 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 42 insertions(+) > > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm > index 13af495d..b8ce4750 100644 > --- a/PVE/QemuServer.pm > +++ b/PVE/QemuServer.pm > @@ -3975,6 +3975,8 @@ sub config_to_command { > push @$machineFlags, 'accel=tcg'; > } > > + PVE::QemuServer::Machine::check_and_set_power_state_flags($globalFlags, $machine_conf); > + > push @$machineFlags, 'smm=off' if should_disable_smm($conf, $vga, $machine_type); > > my $machine_type_min = $machine_type; > diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm > index ebaf2dcc..377abc8a 100644 > --- a/PVE/QemuServer/Machine.pm > +++ b/PVE/QemuServer/Machine.pm > @@ -31,6 +31,16 @@ my $machine_fmt = { > enum => ['intel', 'virtio'], > optional => 1, > }, > + 'enable-s3' => { > + type => 'boolean', > + description => "Enables S3 power state. Defaults to true.", > + optional => 1, > + }, > + 'enable-s4' => { > + type => 'boolean', > + description => "Enables S4 power state. Defaults to true.", > + optional => 1, > + }, > }; > > PVE::JSONSchema::register_format('pve-qemu-machine-fmt', $machine_fmt); Note that an UI patch is needed, because editing the machine there will not preserve these values currently. > @@ -293,4 +303,34 @@ sub check_and_pin_machine_string { > return print_machine($machine_conf); > } > > +# disable s3/s4 by default for 9.2+pve1 machine types > +sub check_and_set_power_state_flags { > + my ($globalFlags, $machine_conf) = @_; > + > + my $get_flag = sub { > + my ($q35, $type, $value) = @_; Style nit: $value is always 1 > + > + if ($q35) { > + return "ICH9-LPC.disable_${type}=${value}"; > + } else { > + return "PIIX4_PM.disable_${type}=${value}"; > + } > + }; Rather than this closure, could just be $object = $q35 ? 'ICH9-LPC' : 'PIIX4_PM' and then use that for constructing the string when pusing. > + > + my $q35 = $machine_conf->{type} && ($machine_conf->{type} =~ m/q35/) ? 1 : 0; > + > + my $s3 = $machine_conf->{'enable-s3'} // 1; > + my $s4 = $machine_conf->{'enable-s4'} // 1; > + > + # they're enabled by default in QEMU, so only add the flags to disable them > + if (!$s3) { > + push $globalFlags->@*, $get_flag->($q35, 's3', 1) Style nit: missing semicolon > + } > + if (!$s4) { > + push $globalFlags->@*, $get_flag->($q35, 's4', 1) Style nit: missing semicolon > + } > + > + return; > +} > + > 1; _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 6/8] machine: add S3/S4 power state properties 2025-03-06 14:52 ` Fiona Ebner @ 2025-03-07 10:02 ` Dominik Csapak 2025-03-07 10:10 ` Fiona Ebner 0 siblings, 1 reply; 35+ messages in thread From: Dominik Csapak @ 2025-03-07 10:02 UTC (permalink / raw) To: Fiona Ebner, Proxmox VE development discussion On 3/6/25 15:52, Fiona Ebner wrote: > Am 06.03.25 um 11:44 schrieb Dominik Csapak: >> So users can disable them (they're enabled by default in QEMU) >> >> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> >> --- >> This patch may make sense, regardless if we'll apply the reversal of the >> default... >> >> PVE/QemuServer.pm | 2 ++ >> PVE/QemuServer/Machine.pm | 40 +++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 42 insertions(+) >> >> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm >> index 13af495d..b8ce4750 100644 >> --- a/PVE/QemuServer.pm >> +++ b/PVE/QemuServer.pm >> @@ -3975,6 +3975,8 @@ sub config_to_command { >> push @$machineFlags, 'accel=tcg'; >> } >> >> + PVE::QemuServer::Machine::check_and_set_power_state_flags($globalFlags, $machine_conf); >> + >> push @$machineFlags, 'smm=off' if should_disable_smm($conf, $vga, $machine_type); >> >> my $machine_type_min = $machine_type; >> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm >> index ebaf2dcc..377abc8a 100644 >> --- a/PVE/QemuServer/Machine.pm >> +++ b/PVE/QemuServer/Machine.pm >> @@ -31,6 +31,16 @@ my $machine_fmt = { >> enum => ['intel', 'virtio'], >> optional => 1, >> }, >> + 'enable-s3' => { >> + type => 'boolean', >> + description => "Enables S3 power state. Defaults to true.", >> + optional => 1, >> + }, >> + 'enable-s4' => { >> + type => 'boolean', >> + description => "Enables S4 power state. Defaults to true.", >> + optional => 1, >> + }, >> }; >> >> PVE::JSONSchema::register_format('pve-qemu-machine-fmt', $machine_fmt); > > Note that an UI patch is needed, because editing the machine there will > not preserve these values currently. > alternative suggestions: I'd prefer to return the +pveX machines (where X > 0) also in the 'capabilities/qemu/machine' api call listing. This would 1. allow the users of the api to see that there are variants 2. allows the UI to work automagically (since it uses that api call) - shows the correct version without losing info - makes it possible to select e.g. a +pve1 variant what do you say? >> @@ -293,4 +303,34 @@ sub check_and_pin_machine_string { >> return print_machine($machine_conf); >> } >> >> +# disable s3/s4 by default for 9.2+pve1 machine types >> +sub check_and_set_power_state_flags { >> + my ($globalFlags, $machine_conf) = @_; >> + >> + my $get_flag = sub { >> + my ($q35, $type, $value) = @_; > > Style nit: $value is always 1 > >> + >> + if ($q35) { >> + return "ICH9-LPC.disable_${type}=${value}"; >> + } else { >> + return "PIIX4_PM.disable_${type}=${value}"; >> + } >> + }; > > Rather than this closure, could just be $object = $q35 ? 'ICH9-LPC' : > 'PIIX4_PM' and then use that for constructing the string when pusing. ok makes sense, in an early version the code was structured differently where this closure made more sense, and i didn't clean it up... > >> + >> + my $q35 = $machine_conf->{type} && ($machine_conf->{type} =~ m/q35/) ? 1 : 0; >> + >> + my $s3 = $machine_conf->{'enable-s3'} // 1; >> + my $s4 = $machine_conf->{'enable-s4'} // 1; >> + >> + # they're enabled by default in QEMU, so only add the flags to disable them >> + if (!$s3) { >> + push $globalFlags->@*, $get_flag->($q35, 's3', 1) > > Style nit: missing semicolon > >> + } >> + if (!$s4) { >> + push $globalFlags->@*, $get_flag->($q35, 's4', 1) > > Style nit: missing semicolon > >> + } >> + >> + return; >> +} >> + >> 1; > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 6/8] machine: add S3/S4 power state properties 2025-03-07 10:02 ` Dominik Csapak @ 2025-03-07 10:10 ` Fiona Ebner 0 siblings, 0 replies; 35+ messages in thread From: Fiona Ebner @ 2025-03-07 10:10 UTC (permalink / raw) To: Dominik Csapak, Proxmox VE development discussion Am 07.03.25 um 11:02 schrieb Dominik Csapak: > On 3/6/25 15:52, Fiona Ebner wrote: >> Am 06.03.25 um 11:44 schrieb Dominik Csapak: >>> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm >>> index ebaf2dcc..377abc8a 100644 >>> --- a/PVE/QemuServer/Machine.pm >>> +++ b/PVE/QemuServer/Machine.pm >>> @@ -31,6 +31,16 @@ my $machine_fmt = { >>> enum => ['intel', 'virtio'], >>> optional => 1, >>> }, >>> + 'enable-s3' => { >>> + type => 'boolean', >>> + description => "Enables S3 power state. Defaults to true.", >>> + optional => 1, >>> + }, >>> + 'enable-s4' => { >>> + type => 'boolean', >>> + description => "Enables S4 power state. Defaults to true.", >>> + optional => 1, >>> + }, >>> }; >>> PVE::JSONSchema::register_format('pve-qemu-machine-fmt', >>> $machine_fmt); >> >> Note that an UI patch is needed, because editing the machine there will >> not preserve these values currently. >> > > alternative suggestions: I'd prefer to return the +pveX machines (where > X > 0) > also in the 'capabilities/qemu/machine' api call listing. This would > 1. allow the users of the api to see that there are variants > 2. allows the UI to work automagically (since it uses that api call) > - shows the correct version without losing info > - makes it possible to select e.g. a +pve1 variant > > what do you say? Sounds good. We should also add something to the docs to describe what +pveN versions are. That would've been useful even before, but if we expose it in the UI it's high time to do so. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* [pve-devel] [PATCH qemu-server 7/8] machine: bump pve machine version and reverse the s3/s4 defaults 2025-03-06 10:44 [pve-devel] [PATCH qemu-server 0/8] disable S3/S4 power states by default Dominik Csapak ` (5 preceding siblings ...) 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 6/8] machine: add S3/S4 power state properties Dominik Csapak @ 2025-03-06 10:44 ` Dominik Csapak 2025-03-06 15:04 ` Fiona Ebner 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 8/8] tests: cfg2cmd: add test for windows machine pinning from meta info Dominik Csapak 2025-03-07 14:46 ` [pve-devel] [PATCH qemu-server 0/8] disable S3/S4 power states by default Dominik Csapak 8 siblings, 1 reply; 35+ messages in thread From: Dominik Csapak @ 2025-03-06 10:44 UTC (permalink / raw) To: pve-devel so new guests (or guests with the 'latest' machine type) have that setting automatically disabled. The previous default (enabling S3/S4), does not make too much sense in a virtual environment, and sometimes makes problems, e.g. Windows defaults to using 'hybrid shutdown' and 'fast startup' when S4 is enabled, which leads to NVIDIA vGPU being broken on the boot after that. Since the tests don't pin the pve version themselves, we have to update all the ones where the machine versions are derived from the installed QEMU version. Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- PVE/QemuServer.pm | 3 ++- PVE/QemuServer/Machine.pm | 16 +++++++++++----- test/cfg2cmd/bootorder-empty.conf.cmd | 4 +++- test/cfg2cmd/bootorder-legacy.conf.cmd | 4 +++- test/cfg2cmd/bootorder.conf.cmd | 4 +++- .../cputype-icelake-client-deprecation.conf.cmd | 4 +++- test/cfg2cmd/custom-cpu-model-defaults.conf.cmd | 4 +++- test/cfg2cmd/efi-raw-template.conf.cmd | 4 +++- test/cfg2cmd/efi-raw.conf.cmd | 4 +++- test/cfg2cmd/efi-secboot-and-tpm-q35.conf.cmd | 4 +++- test/cfg2cmd/efi-secboot-and-tpm.conf.cmd | 4 +++- test/cfg2cmd/efidisk-on-rbd.conf.cmd | 4 +++- test/cfg2cmd/i440fx-viommu-virtio.conf.cmd | 4 +++- test/cfg2cmd/ide.conf.cmd | 4 +++- test/cfg2cmd/memory-hotplug-hugepages.conf.cmd | 4 +++- test/cfg2cmd/memory-hotplug.conf.cmd | 4 +++- test/cfg2cmd/memory-hugepages-1g.conf.cmd | 4 +++- test/cfg2cmd/memory-hugepages-2m.conf.cmd | 4 +++- test/cfg2cmd/minimal-defaults.conf.cmd | 4 +++- test/cfg2cmd/netdev-7.1-multiqueues.conf.cmd | 4 +++- test/cfg2cmd/netdev-7.1.conf.cmd | 4 +++- test/cfg2cmd/q35-ide.conf.cmd | 4 +++- test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd | 4 +++- .../q35-linux-hostpci-multifunction.conf.cmd | 4 +++- test/cfg2cmd/q35-linux-hostpci-template.conf.cmd | 4 +++- .../q35-linux-hostpci-x-pci-overrides.conf.cmd | 4 +++- test/cfg2cmd/q35-linux-hostpci.conf.cmd | 4 +++- test/cfg2cmd/q35-simple.conf.cmd | 4 +++- test/cfg2cmd/q35-viommu-intel.conf.cmd | 4 +++- test/cfg2cmd/q35-viommu-virtio.conf.cmd | 4 +++- test/cfg2cmd/seabios_serial.conf.cmd | 4 +++- test/cfg2cmd/simple-btrfs.conf.cmd | 4 +++- test/cfg2cmd/simple-virtio-blk.conf.cmd | 4 +++- test/cfg2cmd/simple1-template.conf.cmd | 4 +++- test/cfg2cmd/simple1.conf.cmd | 4 +++- test/cfg2cmd/vnc-clipboard-spice.conf.cmd | 4 +++- test/cfg2cmd/vnc-clipboard-std.conf.cmd | 4 +++- 37 files changed, 118 insertions(+), 41 deletions(-) diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index b8ce4750..07fcc97d 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -3975,7 +3975,8 @@ sub config_to_command { push @$machineFlags, 'accel=tcg'; } - PVE::QemuServer::Machine::check_and_set_power_state_flags($globalFlags, $machine_conf); + PVE::QemuServer::Machine::check_and_set_power_state_flags( + $globalFlags, $machine_conf, $version_guard); push @$machineFlags, 'smm=off' if should_disable_smm($conf, $vga, $machine_type); diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm index 377abc8a..20117675 100644 --- a/PVE/QemuServer/Machine.pm +++ b/PVE/QemuServer/Machine.pm @@ -12,6 +12,7 @@ use PVE::JSONSchema qw(get_standard_option parse_property_string print_property_ # version stays the same) our $PVE_MACHINE_VERSION = { '4.1' => 2, + '9.2' => 1, }; my $machine_fmt = { @@ -33,12 +34,12 @@ my $machine_fmt = { }, 'enable-s3' => { type => 'boolean', - description => "Enables S3 power state. Defaults to true.", + description => "Enables S3 power state. Defaults to false beginning with machine types 9.2+pve1, true before.", optional => 1, }, 'enable-s4' => { type => 'boolean', - description => "Enables S4 power state. Defaults to true.", + description => "Enables S4 power state. Defaults to false beginning with machine types 9.2+pve1, true before.", optional => 1, }, }; @@ -305,7 +306,7 @@ sub check_and_pin_machine_string { # disable s3/s4 by default for 9.2+pve1 machine types sub check_and_set_power_state_flags { - my ($globalFlags, $machine_conf) = @_; + my ($globalFlags, $machine_conf, $version_guard) = @_; my $get_flag = sub { my ($q35, $type, $value) = @_; @@ -319,8 +320,13 @@ sub check_and_set_power_state_flags { my $q35 = $machine_conf->{type} && ($machine_conf->{type} =~ m/q35/) ? 1 : 0; - my $s3 = $machine_conf->{'enable-s3'} // 1; - my $s4 = $machine_conf->{'enable-s4'} // 1; + my $default = 1; + if ($version_guard->(9, 2, 1)) { + $default = 0; + } + + my $s3 = $machine_conf->{'enable-s3'} // $default; + my $s4 = $machine_conf->{'enable-s4'} // $default; # they're enabled by default in QEMU, so only add the flags to disable them if (!$s3) { diff --git a/test/cfg2cmd/bootorder-empty.conf.cmd b/test/cfg2cmd/bootorder-empty.conf.cmd index 87fa6c28..9710a70f 100644 --- a/test/cfg2cmd/bootorder-empty.conf.cmd +++ b/test/cfg2cmd/bootorder-empty.conf.cmd @@ -36,4 +36,6 @@ -device 'virtio-blk-pci,drive=drive-virtio1,id=virtio1,bus=pci.0,addr=0xb,iothread=iothread-virtio1' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=A2:C0:43:77:08:A0,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=256' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/bootorder-legacy.conf.cmd b/test/cfg2cmd/bootorder-legacy.conf.cmd index a4c3f050..e0dbb174 100644 --- a/test/cfg2cmd/bootorder-legacy.conf.cmd +++ b/test/cfg2cmd/bootorder-legacy.conf.cmd @@ -36,4 +36,6 @@ -device 'virtio-blk-pci,drive=drive-virtio1,id=virtio1,bus=pci.0,addr=0xb,iothread=iothread-virtio1,bootindex=302' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=A2:C0:43:77:08:A0,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=256,bootindex=100' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/bootorder.conf.cmd b/test/cfg2cmd/bootorder.conf.cmd index 76bd55d7..a0d1e05b 100644 --- a/test/cfg2cmd/bootorder.conf.cmd +++ b/test/cfg2cmd/bootorder.conf.cmd @@ -36,4 +36,6 @@ -device 'virtio-blk-pci,drive=drive-virtio1,id=virtio1,bus=pci.0,addr=0xb,iothread=iothread-virtio1,bootindex=100' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=A2:C0:43:77:08:A0,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=256,bootindex=101' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/cputype-icelake-client-deprecation.conf.cmd b/test/cfg2cmd/cputype-icelake-client-deprecation.conf.cmd index bf084432..95cbe8f9 100644 --- a/test/cfg2cmd/cputype-icelake-client-deprecation.conf.cmd +++ b/test/cfg2cmd/cputype-icelake-client-deprecation.conf.cmd @@ -28,4 +28,6 @@ -device 'virtio-scsi-pci,id=scsihw0,bus=pci.0,addr=0x5' \ -drive 'file=/var/lib/vz/images/8006/base-8006-disk-0.qcow2,if=none,id=drive-scsi0,discard=on,format=qcow2,cache=none,aio=io_uring,detect-zeroes=unmap' \ -device 'scsi-hd,bus=scsihw0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0,id=scsi0,bootindex=100' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/custom-cpu-model-defaults.conf.cmd b/test/cfg2cmd/custom-cpu-model-defaults.conf.cmd index 15b31fb0..16f04bd0 100644 --- a/test/cfg2cmd/custom-cpu-model-defaults.conf.cmd +++ b/test/cfg2cmd/custom-cpu-model-defaults.conf.cmd @@ -22,4 +22,6 @@ -device 'VGA,id=vga,bus=pci.0,addr=0x2' \ -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' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/efi-raw-template.conf.cmd b/test/cfg2cmd/efi-raw-template.conf.cmd index 3e90c335..0d22095e 100644 --- a/test/cfg2cmd/efi-raw-template.conf.cmd +++ b/test/cfg2cmd/efi-raw-template.conf.cmd @@ -23,5 +23,7 @@ -device 'usb-tablet,id=tablet,bus=uhci.0,port=1' \ -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' \ - -machine 'accel=tcg,type=pc+pve0' \ + -machine 'accel=tcg,type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' \ -snapshot diff --git a/test/cfg2cmd/efi-raw.conf.cmd b/test/cfg2cmd/efi-raw.conf.cmd index cf9804bc..94351e4e 100644 --- a/test/cfg2cmd/efi-raw.conf.cmd +++ b/test/cfg2cmd/efi-raw.conf.cmd @@ -24,4 +24,6 @@ -device 'VGA,id=vga,bus=pci.0,addr=0x2' \ -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' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/efi-secboot-and-tpm-q35.conf.cmd b/test/cfg2cmd/efi-secboot-and-tpm-q35.conf.cmd index 911ead05..538a3062 100644 --- a/test/cfg2cmd/efi-secboot-and-tpm-q35.conf.cmd +++ b/test/cfg2cmd/efi-secboot-and-tpm-q35.conf.cmd @@ -25,4 +25,6 @@ -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' \ - -machine 'type=q35+pve0' + -machine 'type=q35+pve1' \ + -global 'ICH9-LPC.disable_s3=1' \ + -global 'ICH9-LPC.disable_s4=1' diff --git a/test/cfg2cmd/efi-secboot-and-tpm.conf.cmd b/test/cfg2cmd/efi-secboot-and-tpm.conf.cmd index 68a85ea0..4b4f80f6 100644 --- a/test/cfg2cmd/efi-secboot-and-tpm.conf.cmd +++ b/test/cfg2cmd/efi-secboot-and-tpm.conf.cmd @@ -27,4 +27,6 @@ -device 'VGA,id=vga,bus=pci.0,addr=0x2' \ -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' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/efidisk-on-rbd.conf.cmd b/test/cfg2cmd/efidisk-on-rbd.conf.cmd index f02039a1..02738217 100644 --- a/test/cfg2cmd/efidisk-on-rbd.conf.cmd +++ b/test/cfg2cmd/efidisk-on-rbd.conf.cmd @@ -29,4 +29,6 @@ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=256,bootindex=300' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/i440fx-viommu-virtio.conf.cmd b/test/cfg2cmd/i440fx-viommu-virtio.conf.cmd index 0352354f..8dc8c129 100644 --- a/test/cfg2cmd/i440fx-viommu-virtio.conf.cmd +++ b/test/cfg2cmd/i440fx-viommu-virtio.conf.cmd @@ -22,4 +22,6 @@ -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 virtio-iommu-pci \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/ide.conf.cmd b/test/cfg2cmd/ide.conf.cmd index 33c6aadc..86920bc7 100644 --- a/test/cfg2cmd/ide.conf.cmd +++ b/test/cfg2cmd/ide.conf.cmd @@ -36,4 +36,6 @@ -device 'scsi-hd,bus=scsihw0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0,id=scsi0,bootindex=100' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=256,bootindex=300' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/memory-hotplug-hugepages.conf.cmd b/test/cfg2cmd/memory-hotplug-hugepages.conf.cmd index f8a8bcb7..64a1d8e5 100644 --- a/test/cfg2cmd/memory-hotplug-hugepages.conf.cmd +++ b/test/cfg2cmd/memory-hotplug-hugepages.conf.cmd @@ -59,4 +59,6 @@ -device 'VGA,id=vga,bus=pci.0,addr=0x2' \ -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' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/memory-hotplug.conf.cmd b/test/cfg2cmd/memory-hotplug.conf.cmd index 859c889d..66834031 100644 --- a/test/cfg2cmd/memory-hotplug.conf.cmd +++ b/test/cfg2cmd/memory-hotplug.conf.cmd @@ -171,4 +171,6 @@ -device 'VGA,id=vga,bus=pci.0,addr=0x2' \ -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' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/memory-hugepages-1g.conf.cmd b/test/cfg2cmd/memory-hugepages-1g.conf.cmd index 352242c4..29187c18 100644 --- a/test/cfg2cmd/memory-hugepages-1g.conf.cmd +++ b/test/cfg2cmd/memory-hugepages-1g.conf.cmd @@ -27,4 +27,6 @@ -device 'VGA,id=vga,bus=pci.0,addr=0x2' \ -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' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/memory-hugepages-2m.conf.cmd b/test/cfg2cmd/memory-hugepages-2m.conf.cmd index 5594e878..a0175160 100644 --- a/test/cfg2cmd/memory-hugepages-2m.conf.cmd +++ b/test/cfg2cmd/memory-hugepages-2m.conf.cmd @@ -27,4 +27,6 @@ -device 'VGA,id=vga,bus=pci.0,addr=0x2' \ -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' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/minimal-defaults.conf.cmd b/test/cfg2cmd/minimal-defaults.conf.cmd index 8da69fee..1def953a 100644 --- a/test/cfg2cmd/minimal-defaults.conf.cmd +++ b/test/cfg2cmd/minimal-defaults.conf.cmd @@ -22,4 +22,6 @@ -device 'VGA,id=vga,bus=pci.0,addr=0x2' \ -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' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/netdev-7.1-multiqueues.conf.cmd b/test/cfg2cmd/netdev-7.1-multiqueues.conf.cmd index 2c6c9054..9c08eb32 100644 --- a/test/cfg2cmd/netdev-7.1-multiqueues.conf.cmd +++ b/test/cfg2cmd/netdev-7.1-multiqueues.conf.cmd @@ -23,4 +23,6 @@ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on,queues=2' \ -device 'virtio-net-pci,mac=A2:C0:43:77:08:A0,netdev=net0,bus=pci.0,addr=0x12,id=net0,vectors=6,mq=on,packed=on,rx_queue_size=1024,tx_queue_size=256,bootindex=300,host_mtu=900' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/netdev-7.1.conf.cmd b/test/cfg2cmd/netdev-7.1.conf.cmd index 6ffa9717..d77dcb82 100644 --- a/test/cfg2cmd/netdev-7.1.conf.cmd +++ b/test/cfg2cmd/netdev-7.1.conf.cmd @@ -23,4 +23,6 @@ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=A2:C0:43:77:08:A0,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=256,bootindex=300,host_mtu=900' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/q35-ide.conf.cmd b/test/cfg2cmd/q35-ide.conf.cmd index dd4f1bbe..45134fc5 100644 --- a/test/cfg2cmd/q35-ide.conf.cmd +++ b/test/cfg2cmd/q35-ide.conf.cmd @@ -35,4 +35,6 @@ -device 'scsi-hd,bus=scsihw0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0,id=scsi0,bootindex=100' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=256,bootindex=300' \ - -machine 'type=q35+pve0' + -machine 'type=q35+pve1' \ + -global 'ICH9-LPC.disable_s3=1' \ + -global 'ICH9-LPC.disable_s4=1' diff --git a/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd b/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd index bc48c5ae..214689c6 100644 --- a/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd +++ b/test/cfg2cmd/q35-linux-hostpci-mapping.conf.cmd @@ -33,4 +33,6 @@ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=256,bootindex=300' \ - -machine 'type=q35+pve0' + -machine 'type=q35+pve1' \ + -global 'ICH9-LPC.disable_s3=1' \ + -global 'ICH9-LPC.disable_s4=1' diff --git a/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd b/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd index 0b1d85af..72c7c5fc 100644 --- a/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd +++ b/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd @@ -33,4 +33,6 @@ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=256,bootindex=300' \ - -machine 'type=q35+pve0' + -machine 'type=q35+pve1' \ + -global 'ICH9-LPC.disable_s3=1' \ + -global 'ICH9-LPC.disable_s4=1' diff --git a/test/cfg2cmd/q35-linux-hostpci-template.conf.cmd b/test/cfg2cmd/q35-linux-hostpci-template.conf.cmd index cda10630..db2d8676 100644 --- a/test/cfg2cmd/q35-linux-hostpci-template.conf.cmd +++ b/test/cfg2cmd/q35-linux-hostpci-template.conf.cmd @@ -26,5 +26,7 @@ -device 'virtio-scsi-pci,id=scsihw0,bus=pci.0,addr=0x5' \ -drive 'file=/var/lib/vz/images/100/base-100-disk-2.raw,if=none,id=drive-scsi0,format=raw,cache=none,aio=io_uring,detect-zeroes=on,readonly=on' \ -device 'scsi-hd,bus=scsihw0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0,id=scsi0' \ - -machine 'accel=tcg,type=pc+pve0' \ + -machine 'accel=tcg,type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' \ -snapshot diff --git a/test/cfg2cmd/q35-linux-hostpci-x-pci-overrides.conf.cmd b/test/cfg2cmd/q35-linux-hostpci-x-pci-overrides.conf.cmd index c7698d17..0f0753cf 100644 --- a/test/cfg2cmd/q35-linux-hostpci-x-pci-overrides.conf.cmd +++ b/test/cfg2cmd/q35-linux-hostpci-x-pci-overrides.conf.cmd @@ -32,4 +32,6 @@ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=256,bootindex=300' \ - -machine 'type=q35+pve0' + -machine 'type=q35+pve1' \ + -global 'ICH9-LPC.disable_s3=1' \ + -global 'ICH9-LPC.disable_s4=1' diff --git a/test/cfg2cmd/q35-linux-hostpci.conf.cmd b/test/cfg2cmd/q35-linux-hostpci.conf.cmd index 5289ec69..1c154a14 100644 --- a/test/cfg2cmd/q35-linux-hostpci.conf.cmd +++ b/test/cfg2cmd/q35-linux-hostpci.conf.cmd @@ -38,4 +38,6 @@ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=256,bootindex=300' \ - -machine 'type=q35+pve0' + -machine 'type=q35+pve1' \ + -global 'ICH9-LPC.disable_s3=1' \ + -global 'ICH9-LPC.disable_s4=1' diff --git a/test/cfg2cmd/q35-simple.conf.cmd b/test/cfg2cmd/q35-simple.conf.cmd index 98b22f46..5bdae382 100644 --- a/test/cfg2cmd/q35-simple.conf.cmd +++ b/test/cfg2cmd/q35-simple.conf.cmd @@ -26,4 +26,6 @@ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=256,bootindex=300' \ - -machine 'type=q35+pve0' + -machine 'type=q35+pve1' \ + -global 'ICH9-LPC.disable_s3=1' \ + -global 'ICH9-LPC.disable_s4=1' diff --git a/test/cfg2cmd/q35-viommu-intel.conf.cmd b/test/cfg2cmd/q35-viommu-intel.conf.cmd index 24e873d4..6d9a76e0 100644 --- a/test/cfg2cmd/q35-viommu-intel.conf.cmd +++ b/test/cfg2cmd/q35-viommu-intel.conf.cmd @@ -20,4 +20,6 @@ -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' \ - -machine 'type=q35+pve0,kernel-irqchip=split' + -machine 'type=q35+pve1,kernel-irqchip=split' \ + -global 'ICH9-LPC.disable_s3=1' \ + -global 'ICH9-LPC.disable_s4=1' diff --git a/test/cfg2cmd/q35-viommu-virtio.conf.cmd b/test/cfg2cmd/q35-viommu-virtio.conf.cmd index 294c353d..72397d27 100644 --- a/test/cfg2cmd/q35-viommu-virtio.conf.cmd +++ b/test/cfg2cmd/q35-viommu-virtio.conf.cmd @@ -20,4 +20,6 @@ -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 virtio-iommu-pci \ - -machine 'type=q35+pve0' + -machine 'type=q35+pve1' \ + -global 'ICH9-LPC.disable_s3=1' \ + -global 'ICH9-LPC.disable_s4=1' diff --git a/test/cfg2cmd/seabios_serial.conf.cmd b/test/cfg2cmd/seabios_serial.conf.cmd index 1c4e102c..e57551dc 100644 --- a/test/cfg2cmd/seabios_serial.conf.cmd +++ b/test/cfg2cmd/seabios_serial.conf.cmd @@ -30,4 +30,6 @@ -device 'scsi-hd,bus=scsihw0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0,id=scsi0,bootindex=100' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=A2:C0:43:77:08:A0,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=256,bootindex=300' \ - -machine 'smm=off,type=pc+pve0' + -machine 'smm=off,type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/simple-btrfs.conf.cmd b/test/cfg2cmd/simple-btrfs.conf.cmd index c2354887..fff2eec2 100644 --- a/test/cfg2cmd/simple-btrfs.conf.cmd +++ b/test/cfg2cmd/simple-btrfs.conf.cmd @@ -30,4 +30,6 @@ -device 'scsi-hd,bus=scsihw0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0,id=scsi0,bootindex=100' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=A2:C0:43:77:08:A0,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=256,bootindex=300' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/simple-virtio-blk.conf.cmd b/test/cfg2cmd/simple-virtio-blk.conf.cmd index d19aca6b..4a50531b 100644 --- a/test/cfg2cmd/simple-virtio-blk.conf.cmd +++ b/test/cfg2cmd/simple-virtio-blk.conf.cmd @@ -30,4 +30,6 @@ -device 'virtio-blk-pci,drive=drive-virtio0,id=virtio0,bus=pci.0,addr=0xa,iothread=iothread-virtio0,bootindex=100' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=A2:C0:43:77:08:A0,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=256,bootindex=300' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/simple1-template.conf.cmd b/test/cfg2cmd/simple1-template.conf.cmd index 35484600..9157cdd8 100644 --- a/test/cfg2cmd/simple1-template.conf.cmd +++ b/test/cfg2cmd/simple1-template.conf.cmd @@ -29,5 +29,7 @@ -device 'ahci,id=ahci0,multifunction=on,bus=pci.0,addr=0x7' \ -drive 'file=/var/lib/vz/images/8006/base-8006-disk-0.qcow2,if=none,id=drive-sata0,discard=on,format=qcow2,cache=none,aio=io_uring,detect-zeroes=unmap' \ -device 'ide-hd,bus=ahci0.0,drive=drive-sata0,id=sata0' \ - -machine 'accel=tcg,smm=off,type=pc+pve0' \ + -machine 'accel=tcg,smm=off,type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' \ -snapshot diff --git a/test/cfg2cmd/simple1.conf.cmd b/test/cfg2cmd/simple1.conf.cmd index ecd14bcc..b52f91e7 100644 --- a/test/cfg2cmd/simple1.conf.cmd +++ b/test/cfg2cmd/simple1.conf.cmd @@ -30,4 +30,6 @@ -device 'scsi-hd,bus=scsihw0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0,id=scsi0,bootindex=100' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=A2:C0:43:77:08:A0,netdev=net0,bus=pci.0,addr=0x12,id=net0,rx_queue_size=1024,tx_queue_size=256,bootindex=300' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/vnc-clipboard-spice.conf.cmd b/test/cfg2cmd/vnc-clipboard-spice.conf.cmd index f24cc7f0..23545dfa 100644 --- a/test/cfg2cmd/vnc-clipboard-spice.conf.cmd +++ b/test/cfg2cmd/vnc-clipboard-spice.conf.cmd @@ -24,4 +24,6 @@ -spice 'tls-port=61000,addr=127.0.0.1,tls-ciphers=HIGH,seamless-migration=on' \ -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' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=1' diff --git a/test/cfg2cmd/vnc-clipboard-std.conf.cmd b/test/cfg2cmd/vnc-clipboard-std.conf.cmd index c0c6cd28..8e2f4d89 100644 --- a/test/cfg2cmd/vnc-clipboard-std.conf.cmd +++ b/test/cfg2cmd/vnc-clipboard-std.conf.cmd @@ -24,4 +24,6 @@ -device 'virtserialport,chardev=vdagent,name=com.redhat.spice.0' \ -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' \ - -machine 'type=pc+pve0' + -machine 'type=pc+pve1' \ + -global 'PIIX4_PM.disable_s3=1' \ + -global 'PIIX4_PM.disable_s4=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] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 7/8] machine: bump pve machine version and reverse the s3/s4 defaults 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 7/8] machine: bump pve machine version and reverse the s3/s4 defaults Dominik Csapak @ 2025-03-06 15:04 ` Fiona Ebner 0 siblings, 0 replies; 35+ messages in thread From: Fiona Ebner @ 2025-03-06 15:04 UTC (permalink / raw) To: Proxmox VE development discussion, Dominik Csapak Am 06.03.25 um 11:44 schrieb Dominik Csapak: > so new guests (or guests with the 'latest' machine type) have > that setting automatically disabled. > > The previous default (enabling S3/S4), does not make too much sense in a > virtual environment, and sometimes makes problems, e.g. Windows defaults > to using 'hybrid shutdown' and 'fast startup' when S4 is enabled, which > leads to NVIDIA vGPU being broken on the boot after that. > > Since the tests don't pin the pve version themselves, we have to update > all the ones where the machine versions are derived from the installed > QEMU version. > > Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> Reviewed-by: Fiona Ebner <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] 35+ messages in thread
* [pve-devel] [PATCH qemu-server 8/8] tests: cfg2cmd: add test for windows machine pinning from meta info 2025-03-06 10:44 [pve-devel] [PATCH qemu-server 0/8] disable S3/S4 power states by default Dominik Csapak ` (6 preceding siblings ...) 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 7/8] machine: bump pve machine version and reverse the s3/s4 defaults Dominik Csapak @ 2025-03-06 10:44 ` Dominik Csapak 2025-03-06 15:10 ` Fiona Ebner 2025-03-07 14:46 ` [pve-devel] [PATCH qemu-server 0/8] disable S3/S4 power states by default Dominik Csapak 8 siblings, 1 reply; 35+ messages in thread From: Dominik Csapak @ 2025-03-06 10:44 UTC (permalink / raw) To: pve-devel once with included pve machine and once without Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- test/cfg2cmd/q35-windows-pinning-pvever.conf | 5 ++++ .../q35-windows-pinning-pvever.conf.cmd | 26 +++++++++++++++++++ test/cfg2cmd/q35-windows-pinning.conf | 5 ++++ test/cfg2cmd/q35-windows-pinning.conf.cmd | 24 +++++++++++++++++ 4 files changed, 60 insertions(+) create mode 100644 test/cfg2cmd/q35-windows-pinning-pvever.conf create mode 100644 test/cfg2cmd/q35-windows-pinning-pvever.conf.cmd create mode 100644 test/cfg2cmd/q35-windows-pinning.conf create mode 100644 test/cfg2cmd/q35-windows-pinning.conf.cmd diff --git a/test/cfg2cmd/q35-windows-pinning-pvever.conf b/test/cfg2cmd/q35-windows-pinning-pvever.conf new file mode 100644 index 00000000..45d87396 --- /dev/null +++ b/test/cfg2cmd/q35-windows-pinning-pvever.conf @@ -0,0 +1,5 @@ +# TEST: Config with q35, windows, meta (incl pvever) to test version pinning +# +machine: q35 +meta: creation-qemu=9.2.0,ctime=1741179133,creation-pve-machine=1 +ostype: win11 diff --git a/test/cfg2cmd/q35-windows-pinning-pvever.conf.cmd b/test/cfg2cmd/q35-windows-pinning-pvever.conf.cmd new file mode 100644 index 00000000..06ae3f27 --- /dev/null +++ b/test/cfg2cmd/q35-windows-pinning-pvever.conf.cmd @@ -0,0 +1,26 @@ +/usr/bin/kvm \ + -id 8006 \ + -name 'vm8006,debug-threads=on' \ + -no-shutdown \ + -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \ + -mon 'chardev=qmp,mode=control' \ + -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \ + -mon 'chardev=qmp-event,mode=control' \ + -pidfile /var/run/qemu-server/8006.pid \ + -daemonize \ + -smp '1,sockets=1,cores=1,maxcpus=1' \ + -nodefaults \ + -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \ + -vnc 'unix:/var/run/qemu-server/8006.vnc,password=on' \ + -cpu 'kvm64,enforce,hv_ipi,hv_relaxed,hv_reset,hv_runtime,hv_spinlocks=0x1fff,hv_stimer,hv_synic,hv_time,hv_vapic,hv_vpindex,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep' \ + -m 512 \ + -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \ + -device 'usb-tablet,id=tablet,bus=ehci.0,port=1' \ + -device 'VGA,id=vga,bus=pcie.0,addr=0x1,edid=off' \ + -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' \ + -rtc 'driftfix=slew,base=localtime' \ + -machine 'hpet=off,type=pc-q35-9.2+pve1' \ + -global 'kvm-pit.lost_tick_policy=discard' \ + -global 'ICH9-LPC.disable_s3=1' \ + -global 'ICH9-LPC.disable_s4=1' diff --git a/test/cfg2cmd/q35-windows-pinning.conf b/test/cfg2cmd/q35-windows-pinning.conf new file mode 100644 index 00000000..8bd47b0a --- /dev/null +++ b/test/cfg2cmd/q35-windows-pinning.conf @@ -0,0 +1,5 @@ +# TEST: Config with q35, win, meta to test version pinning +# +machine: q35 +meta: creation-qemu=9.2.0,ctime=1741179133 +ostype: win11 diff --git a/test/cfg2cmd/q35-windows-pinning.conf.cmd b/test/cfg2cmd/q35-windows-pinning.conf.cmd new file mode 100644 index 00000000..d8570b2c --- /dev/null +++ b/test/cfg2cmd/q35-windows-pinning.conf.cmd @@ -0,0 +1,24 @@ +/usr/bin/kvm \ + -id 8006 \ + -name 'vm8006,debug-threads=on' \ + -no-shutdown \ + -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \ + -mon 'chardev=qmp,mode=control' \ + -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \ + -mon 'chardev=qmp-event,mode=control' \ + -pidfile /var/run/qemu-server/8006.pid \ + -daemonize \ + -smp '1,sockets=1,cores=1,maxcpus=1' \ + -nodefaults \ + -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \ + -vnc 'unix:/var/run/qemu-server/8006.vnc,password=on' \ + -cpu 'kvm64,enforce,hv_ipi,hv_relaxed,hv_reset,hv_runtime,hv_spinlocks=0x1fff,hv_stimer,hv_synic,hv_time,hv_vapic,hv_vpindex,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep' \ + -m 512 \ + -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \ + -device 'usb-tablet,id=tablet,bus=ehci.0,port=1' \ + -device 'VGA,id=vga,bus=pcie.0,addr=0x1,edid=off' \ + -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' \ + -rtc 'driftfix=slew,base=localtime' \ + -machine 'hpet=off,type=pc-q35-9.2+pve0' \ + -global 'kvm-pit.lost_tick_policy=discard' -- 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] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 8/8] tests: cfg2cmd: add test for windows machine pinning from meta info 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 8/8] tests: cfg2cmd: add test for windows machine pinning from meta info Dominik Csapak @ 2025-03-06 15:10 ` Fiona Ebner 0 siblings, 0 replies; 35+ messages in thread From: Fiona Ebner @ 2025-03-06 15:10 UTC (permalink / raw) To: Proxmox VE development discussion, Dominik Csapak Am 06.03.25 um 11:44 schrieb Dominik Csapak: > once with included pve machine and once without > > Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> If we go for this fallback approach: Reviewed-by: Fiona Ebner <f.ebner@proxmox.com> In either case, it would be nice to also have a test with an explicitly pinned 9.2+pve0 too. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 0/8] disable S3/S4 power states by default 2025-03-06 10:44 [pve-devel] [PATCH qemu-server 0/8] disable S3/S4 power states by default Dominik Csapak ` (7 preceding siblings ...) 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 8/8] tests: cfg2cmd: add test for windows machine pinning from meta info Dominik Csapak @ 2025-03-07 14:46 ` Dominik Csapak 8 siblings, 0 replies; 35+ messages in thread From: Dominik Csapak @ 2025-03-07 14:46 UTC (permalink / raw) To: pve-devel superseded by v2: https://lore.proxmox.com/pve-devel/20250307144436.122621-1-d.csapak@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] 35+ messages in thread
end of thread, other threads:[~2025-03-07 14:47 UTC | newest] Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-03-06 10:44 [pve-devel] [PATCH qemu-server 0/8] disable S3/S4 power states by default Dominik Csapak 2025-03-06 10:44 ` [pve-devel] [RFC PATCH qemu-server 1/8] tests: cfg2cmd: pin QEMU version Dominik Csapak 2025-03-06 12:00 ` Fiona Ebner 2025-03-06 12:07 ` Dominik Csapak 2025-03-06 12:43 ` Fiona Ebner 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 2/8] config to command: add one '-global' option for each flag Dominik Csapak 2025-03-06 12:13 ` Fiona Ebner 2025-03-06 12:15 ` Dominik Csapak 2025-03-06 12:55 ` Fiona Ebner 2025-03-07 9:54 ` Dominik Csapak 2025-03-07 10:00 ` Fiona Ebner 2025-03-07 10:05 ` Dominik Csapak 2025-03-07 10:14 ` Fiona Ebner 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 3/8] meta info: also add current pve-machine version Dominik Csapak 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 4/8] machine: correctly select pve machine version for non pinned windows guests Dominik Csapak 2025-03-06 13:10 ` Fiona Ebner 2025-03-06 13:36 ` Dominik Csapak 2025-03-06 14:10 ` Fiona Ebner 2025-03-06 14:14 ` Fiona Ebner 2025-03-06 14:15 ` Dominik Csapak 2025-03-06 14:20 ` Fiona Ebner 2025-03-07 9:55 ` Dominik Csapak 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 5/8] machine: incorporate pve machine version when pinning " Dominik Csapak 2025-03-06 14:32 ` Fiona Ebner 2025-03-07 9:58 ` Dominik Csapak 2025-03-07 10:06 ` Fiona Ebner 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 6/8] machine: add S3/S4 power state properties Dominik Csapak 2025-03-06 14:52 ` Fiona Ebner 2025-03-07 10:02 ` Dominik Csapak 2025-03-07 10:10 ` Fiona Ebner 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 7/8] machine: bump pve machine version and reverse the s3/s4 defaults Dominik Csapak 2025-03-06 15:04 ` Fiona Ebner 2025-03-06 10:44 ` [pve-devel] [PATCH qemu-server 8/8] tests: cfg2cmd: add test for windows machine pinning from meta info Dominik Csapak 2025-03-06 15:10 ` Fiona Ebner 2025-03-07 14:46 ` [pve-devel] [PATCH qemu-server 0/8] disable S3/S4 power states by default Dominik Csapak
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inboxService provided by Proxmox Server Solutions GmbH | Privacy | Legal