From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: [pve-devel] applied: [PATCH qemu-server] api: create/update vm: avoid printing empty machine string
Date: Mon, 20 Jan 2025 14:45:54 +0100 [thread overview]
Message-ID: <1737380735.2n0dxj49qy.astroid@yuna.none> (raw)
In-Reply-To: <20250120132720.95116-1-f.ebner@proxmox.com>
On January 20, 2025 2:27 pm, Fiona Ebner wrote:
> While no problem is known with having an empty machine string in the
> configuration and it was already possible setting an empty machine
> manually via 'qm set', the behavior changed because of commit 919e69d0
> ("machine: add check_and_pin_machine_string() helper") and there is
> potential for problematic edge cases. Restore the previous behavior.
>
> Pinning is also required when there is no given machine type, so the
> call to check_and_pin_machine_string() should stay unconditional.
>
> For update, pinning was recently added by commit 7a9570f3 ("api:
> update vm config: pin machine version when switching to windows os
> type"), so bring that in-line with the behavior for create too.
>
> Another idea would've been to just return the default machine in
> check_and_pin_machine_string(), but that would also be a change in
> behavior. In particular, the default depends on the arch, so an empty
> machine type will pick the default machine for the currently
> configured arch even when the arch is later changed.
>
> Reported-by: Daniel Herzig <d.herzig@proxmox.com>
> Fixes: 919e69d0 ("machine: add check_and_pin_machine_string() helper")
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> PVE/API2/Qemu.pm | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 8acd8d9f..45fe6ea6 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1243,8 +1243,9 @@ __PACKAGE__->register_method({
> }
>
> # always pin Windows' machine version on create, they get confused too easily
> - $conf->{machine} = PVE::QemuServer::Machine::check_and_pin_machine_string(
> + my $machine_string = PVE::QemuServer::Machine::check_and_pin_machine_string(
> $conf->{machine}, $conf->{ostype});
> + $conf->{machine} = $machine_string if $machine_string;
>
> $conf->{lock} = 'import' if $live_import_mapping;
>
> @@ -2120,9 +2121,10 @@ my $update_vm_api = sub {
> && !$modified->{machine} # detects deletion
> ) {
> eval {
> - $conf->{pending}->{machine} =
> + my $machine_string =
> PVE::QemuServer::Machine::check_and_pin_machine_string(
> $conf->{machine}, $param->{ostype});
> + $conf->{pending}->{machine} = $machine_string if $machine_string;
> };
> print "automatic pinning of machine version failed - $@" if $@;
> }
> --
> 2.39.5
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
prev parent reply other threads:[~2025-01-20 13:46 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-20 13:27 [pve-devel] " Fiona Ebner
2025-01-20 13:45 ` Fabian Grünbichler [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1737380735.2n0dxj49qy.astroid@yuna.none \
--to=f.gruenbichler@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox