Re: [PATCH qemu-server v2] fix #5578: smbios: set serial number

[Fiona Ebner <f.ebner@proxmox.com>]

Am 19.02.26 um 12:19 PM schrieb Manuel Federanko:
> If no smbios options are given on creation, default to generate a serial
> number. This is required for Windows Autopilot to identify a user
> device.
> Use the already generated smbios uuid as serial number, it should be
> unique enough for our purposes.

The bug report mentions VMWare using "VMware-42 xx yy zz .....". Maybe
we should also use a "Proxmox-" prefix or similar?

I do wonder if we should set this unconditionally or somehow guard it,
e.g. only do it for win11 OS type, or if guarding would be overly
cautious. It does only affect new VMs, so if this does cause some kind
of regression, we can still adapt it again.

> The base64 here is needed since all configuration options are stored as
> a base64 encoded string, which is ensured by the format. This ensures
> that the values are properly decoded in the gui, for example.
> 
> Since all fields are forced to be stored in base64 format it might make
> sense for a future patch to a) remove the flag or b) allow values
> different than base64 encoded data in the other fields.

Note that absence of the flag is needed for backwards compatibility with
pre-existing configuration files (also in backups) from before the flag
was introduced [0]. If the base64=1 is not set, the values are
interpreted directly and not decoded first.

> Tested by creating a new vm via the gui and command line.
> 
> Signed-off-by: Manuel Federanko <m.federanko@proxmox.com>
> ---
> Changes since v1:
> * switch serial to be the same uuid
> * use print_smbios1 over manually constructing strings
> * remove the dedicated generate_smbios1_uuid subroutine
> 
>  src/PVE/API2/Qemu.pm  |  9 ++++++++-
>  src/PVE/CLI/qm.pm     | 10 +++++++++-
>  src/PVE/QemuServer.pm |  4 ----
>  3 files changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
> index c2e185a6..bbf534e4 100644
> --- a/src/PVE/API2/Qemu.pm
> +++ b/src/PVE/API2/Qemu.pm

Missing use statement for MIME::Base64

> @@ -1471,7 +1471,14 @@ __PACKAGE__->register_method({
>  
>                      # auto generate uuid if user did not specify smbios1 option
>                      if (!$conf->{smbios1}) {
> -                        $conf->{smbios1} = PVE::QemuServer::generate_smbios1_uuid();
> +                        my $smbios1_uuid = PVE::QemuServer::generate_uuid();
> +                        my $smbios1_serial = MIME::Base64::encode_base64($smbios1_uuid, "");
> +                        my %smbios1 = (

Style nit: our code base mostly uses hash references rather than hashes
directly. Especially, since you need to pass it along as a reference,
you can already define it as such.

> +                            uuid => $smbios1_uuid,
> +                            serial => $smbios1_serial,
> +                            base64 => 1,
> +                        );
> +                        $conf->{smbios1} = PVE::QemuServer::print_smbios1(\%smbios1);
>                      }
>  
>                      if (
> diff --git a/src/PVE/CLI/qm.pm b/src/PVE/CLI/qm.pm
> index bdae9641..0daf402d 100755
> --- a/src/PVE/CLI/qm.pm
> +++ b/src/PVE/CLI/qm.pm

Missing use statement for MIME::Base64

> @@ -912,7 +912,15 @@ __PACKAGE__->register_method({
>          eval {
>              # order matters, as do_import() will load_config() internally
>              $conf->{vmgenid} = PVE::QemuServer::generate_uuid();
> -            $conf->{smbios1} = PVE::QemuServer::generate_smbios1_uuid();
> +            my $smbios1_uuid = PVE::QemuServer::generate_uuid();
> +            my $smbios1_serial = MIME::Base64::encode_base64($smbios1_uuid, "");
> +            my %smbios1 = (
> +                uuid => $smbios1_uuid,
> +                serial => $smbios1_serial,
> +                base64 => 1,
> +            );
> +            $conf->{smbios1} = PVE::QemuServer::print_smbios1(\%smbios1);
> +
>              PVE::QemuConfig->write_config($vmid, $conf);
>  
>              foreach my $disk (@{ $parsed->{disks} }) {
> diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
> index 545758dc..b371d00b 100644
> --- a/src/PVE/QemuServer.pm
> +++ b/src/PVE/QemuServer.pm
> @@ -8082,10 +8082,6 @@ sub generate_uuid {
>      return $uuid_str;
>  }
>  
> -sub generate_smbios1_uuid {
> -    return "uuid=" . generate_uuid();

Rather than removing the helper here, it could be extended. This would
avoid duplicating the behavior on the (previous) call sites that need it.

> -}
> -
>  sub create_reboot_request {
>      my ($vmid) = @_;
>      open(my $fh, '>', "/run/qemu-server/$vmid.reboot")

[0]:
https://git.proxmox.com/?p=qemu-server.git;a=commitdiff;h=1f30ac3a9f18c215d3e3c657f9e81d9d3125f46c