[pve-devel] [PATCH qemu] api: qemu: create: default cpu to x86-64-v2-AES

[pve-devel] [PATCH qemu] api: qemu: create: default cpu to x86-64-v2-AES

[Maximiliano Sandoval]

This makes it so newly created VMs, e.g. with `qm create` will have the
same default value as VMs created via the web UI.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
I am not entirely sure if there is side-effect that I am not taking into account.

 PVE/API2/Qemu.pm | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index d25a79fe..383218fd 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1152,6 +1152,10 @@ __PACKAGE__->register_method({
 			$conf->{vmgenid} = PVE::QemuServer::generate_uuid();
 		    }
 
+		    if (!defined($param->{cpu})) {
+			$conf->{cpu} = 'x86-64-v2-AES';
+		    }
+
 		    my $machine_conf = PVE::QemuServer::Machine::parse_machine($conf->{machine});
 		    my $machine = $machine_conf->{type};
 		    if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu] api: qemu: create: default cpu to x86-64-v2-AES

[Shannon Sterz]

note that the gui's default is a bit "fake". it pre-sets the field to
`x86-64-v2-AES`, but if you click the little "x" next to it, the field
will be cleared and if you submit the form like that the default ends up
being `kvm64` again. this happens because the final config file does not
specify a cpu type in this case and, hence, the old default is used for
compatability reasons.

so, this effectively removes the ability to set *no* cpu type via the
cli if i'm not mistaken. might be worth considering.

On Tue Oct 1, 2024 at 4:08 PM CEST, Maximiliano Sandoval wrote:
> This makes it so newly created VMs, e.g. with `qm create` will have the
> same default value as VMs created via the web UI.
>
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
> I am not entirely sure if there is side-effect that I am not taking into account.
>
>  PVE/API2/Qemu.pm | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index d25a79fe..383218fd 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1152,6 +1152,10 @@ __PACKAGE__->register_method({
>  			$conf->{vmgenid} = PVE::QemuServer::generate_uuid();
>  		    }
>
> +		    if (!defined($param->{cpu})) {
> +			$conf->{cpu} = 'x86-64-v2-AES';
> +		    }
> +
>  		    my $machine_conf = PVE::QemuServer::Machine::parse_machine($conf->{machine});
>  		    my $machine = $machine_conf->{type};
>  		    if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu] api: qemu: create: default cpu to x86-64-v2-AES

[Fiona Ebner]

Am 01.10.24 um 16:08 schrieb Maximiliano Sandoval:
> This makes it so newly created VMs, e.g. with `qm create` will have the
> same default value as VMs created via the web UI.
> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
> I am not entirely sure if there is side-effect that I am not taking into account.
> 

This is a breaking change, because existing API callers now suddenly get
a new default. Even if it were just CLI and not API, it would be
breaking for the same reason (there are scripts using the CLI tools out
there).

If we do this, then in a major release and prominently communicate it to
all users in the release notes. And it should also be documented it in
the API schema, that creation uses another default than the schema default.

>  PVE/API2/Qemu.pm | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index d25a79fe..383218fd 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1152,6 +1152,10 @@ __PACKAGE__->register_method({
>  			$conf->{vmgenid} = PVE::QemuServer::generate_uuid();
>  		    }
>  
> +		    if (!defined($param->{cpu})) {
> +			$conf->{cpu} = 'x86-64-v2-AES';
> +		    }
> +
>  		    my $machine_conf = PVE::QemuServer::Machine::parse_machine($conf->{machine});
>  		    my $machine = $machine_conf->{type};
>  		    if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu] api: qemu: create: default cpu to x86-64-v2-AES

[Thomas Lamprecht]

Am 22/10/2024 um 13:23 schrieb Fiona Ebner:
> Am 01.10.24 um 16:08 schrieb Maximiliano Sandoval:
>> This makes it so newly created VMs, e.g. with `qm create` will have the
>> same default value as VMs created via the web UI.
>>
>> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
>> ---
>> I am not entirely sure if there is side-effect that I am not taking into account.
>>
> This is a breaking change, because existing API callers now suddenly get
> a new default. Even if it were just CLI and not API, it would be
> breaking for the same reason (there are scripts using the CLI tools out
> there).
> 
> If we do this, then in a major release and prominently communicate it to
> all users in the release notes. And it should also be documented it in
> the API schema, that creation uses another default than the schema default.

And even then, would this still break restoring backups made before that
change?
If, this would fall under the changes that need versioning the guest
configs. Which naturally is possible but is IMO also not something I'd
do lightly, as that's something that might have wide-reaching consequences.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu] api: qemu: create: default cpu to x86-64-v2-AES

[Maximiliano Sandoval]

Thomas Lamprecht <t.lamprecht@proxmox.com> writes:

> And even then, would this still break restoring backups made before that
> change?
> If, this would fall under the changes that need versioning the guest
> configs. Which naturally is possible but is IMO also not something I'd
> do lightly, as that's something that might have wide-reaching consequences.

Would it make more sense to do this only for the CLI tool, and only when creating new VMs?

-- 
Maximiliano


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu] api: qemu: create: default cpu to x86-64-v2-AES

[Thomas Lamprecht]

Am 22/10/2024 um 14:15 schrieb Maximiliano Sandoval:
> Thomas Lamprecht <t.lamprecht@proxmox.com> writes:
> 
>> And even then, would this still break restoring backups made before that
>> change?
>> If, this would fall under the changes that need versioning the guest
>> configs. Which naturally is possible but is IMO also not something I'd
>> do lightly, as that's something that might have wide-reaching consequences.
> 
> Would it make more sense to do this only for the CLI tool, and only when creating new VMs?
> 

Hmm, it would feel a bit odd to me to single that specific "mismatch" between
UI default and CLI/schema out but ignore the others, like e.g., the memory
default of 2048, or some dynamic defaults like the OS-type dependent ones.

And while it would have reduced impact, it would be still a breaking change
that might affect peoples automation/scripts.
But if more than just the CPU type would be looked at, and adapted such that
UI and CLI behave more the same, it might be indeed a feasible way to improve
UX slightly on the next major release next year.
Conveying the reason for different defaults and how one can look into which
one will be applied would warrant a short paragraph in the docs then too.
And taking the chance and looking at the CT story on this would be great too,
e.g., like is 512 MB really still a good default for UI/CLI, or unprivileged
vs. privileged.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu] api: qemu: create: default cpu to x86-64-v2-AES

[Fiona Ebner]

Am 22.10.24 um 15:35 schrieb Thomas Lamprecht:
> Am 22/10/2024 um 14:15 schrieb Maximiliano Sandoval:
>> Thomas Lamprecht <t.lamprecht@proxmox.com> writes:
>>
>>> And even then, would this still break restoring backups made before that
>>> change?
>>> If, this would fall under the changes that need versioning the guest
>>> configs. Which naturally is possible but is IMO also not something I'd
>>> do lightly, as that's something that might have wide-reaching consequences.
>>
>> Would it make more sense to do this only for the CLI tool, and only when creating new VMs?
>>
> 
> Hmm, it would feel a bit odd to me to single that specific "mismatch" between
> UI default and CLI/schema out but ignore the others, like e.g., the memory
> default of 2048, or some dynamic defaults like the OS-type dependent ones.
> 
> And while it would have reduced impact, it would be still a breaking change
> that might affect peoples automation/scripts.
> But if more than just the CPU type would be looked at, and adapted such that
> UI and CLI behave more the same, it might be indeed a feasible way to improve
> UX slightly on the next major release next year.
> Conveying the reason for different defaults and how one can look into which
> one will be applied would warrant a short paragraph in the docs then too.
> And taking the chance and looking at the CT story on this would be great too,
> e.g., like is 512 MB really still a good default for UI/CLI, or unprivileged
> vs. privileged.

Maybe we could have (datacenter-wide?) default profiles (one for
containers, one for VMs) whose values are used for new guest creation?
Users could modify them if desired and they could also be used to
pre-fill values in the UI. Could then be shipped for PVE 9 with sensible
values.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu] api: qemu: create: default cpu to x86-64-v2-AES

[Thomas Lamprecht]

Am 23/10/2024 um 11:56 schrieb Fiona Ebner:
> Maybe we could have (datacenter-wide?) default profiles (one for
> containers, one for VMs) whose values are used for new guest creation?
> Users could modify them if desired and they could also be used to
> pre-fill values in the UI. Could then be shipped for PVE 9 with sensible
> values.

This sounds (at least partially) like the config profile work I wished for
and Dominik worked on a while ago:

https://lore.proxmox.com/pve-devel/20231117114548.3208470-1-d.csapak@proxmox.com/

And yes, with that one could also ship a few built-in profiles that could
be passed on VM/CT creation.

I did not think it through completely, but default profiles could be also
seen as variation of this.

And FWIW, currently the above linked profile approach resolves values and
solves them explicitly, and is not just a reference where changing the
profile will alter the effective config of guests that got created before
that profile change happened. Which might be something that can be desired,
but IMO such a behavior would need to be opt-in.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel