From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id A7F031FF142 for ; Tue, 07 Apr 2026 15:27:25 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 673F81BB3C; Tue, 7 Apr 2026 15:28:00 +0200 (CEST) Message-ID: <4bd31261-0287-49ab-9af5-e05334454075@proxmox.com> Date: Tue, 7 Apr 2026 15:27:56 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH qemu-server v3] fix #5578: smbios: set serial number To: Manuel Federanko , pve-devel@lists.proxmox.com References: <20260303104919.33634-1-m.federanko@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: <20260303104919.33634-1-m.federanko@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1775568410565 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.050 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: NQKW7CG47ATKVVVJ2PCON4PW5YQCXXS4 X-Message-ID-Hash: NQKW7CG47ATKVVVJ2PCON4PW5YQCXXS4 X-MailFrom: d.csapak@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: not sure if it was discussed off-list, but you didn't really address fionas comment about just adding this conditionally, e.g. via ostype or machine version this will increase the config size by a bit and i guess most guest operating systems don't gain much from this? i'm guilty of adding such flags unconditionally myself in the past (see vmgenid) but i think we should avoid that when possible e.g. an empty config (qm create ID) looks like this currently: ``` boot: meta: creation-qemu=10.2.1,ctime=1775568119 smbios1: uuid=a0f6c957-1c8b-439f-b25a-1e45dc151263 vmgenid: 0ed5ca0d-0e72-4c1a-b62f-ad2f7aaa8819 ``` with your patch it looks like this: ``` boot: meta: creation-qemu=10.2.1,ctime=1775568283 smbios1: base64=1,serial=UFZFLWM5OTY3ZDQwLTVlZTUtNDQ1My1hZDI0LTljZWUzODJmZTg1ZA==,uuid=c9967d40-5ee5-4453-ad24-9cee382fe85d vmgenid: 1c7fe857-3520-4a20-8e09-611a7fb1be3b ``` which is quite a bit of noise. If you think it's worthwhile to have a serial number for every guest, we could e.g. still give it to qemus commandline if it's missing in the config (especially if it's the same as the normal uuid, but prefixed with PVE-) IMHO it makes no sense having the same uuid twice in the config. if someone sets a serial manually, we should use that of course. sorry if any of these were discussed already, i checked the m-l but didn't find any discussion regarding this. On 3/3/26 11:49 AM, Manuel Federanko wrote: > 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 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. > > Tested by creating a new vm via the gui and command line. > > Signed-off-by: Manuel Federanko > --- > Changes since v2: > * add "PVE-" prefix to serial > * move logic back into a helper subroutine > 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 | 2 +- > src/PVE/CLI/qm.pm | 2 +- > src/PVE/QemuServer.pm | 10 ++++++++-- > 3 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm > index c2e185a6..a68b1906 100644 > --- a/src/PVE/API2/Qemu.pm > +++ b/src/PVE/API2/Qemu.pm > @@ -1471,7 +1471,7 @@ __PACKAGE__->register_method({ > > # auto generate uuid if user did not specify smbios1 option > if (!$conf->{smbios1}) { > - $conf->{smbios1} = PVE::QemuServer::generate_smbios1_uuid(); > + $conf->{smbios1} = PVE::QemuServer::generate_smbios1(); > } > > if ( > diff --git a/src/PVE/CLI/qm.pm b/src/PVE/CLI/qm.pm > index bdae9641..975ec3f5 100755 > --- a/src/PVE/CLI/qm.pm > +++ b/src/PVE/CLI/qm.pm > @@ -912,7 +912,7 @@ __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(); > + $conf->{smbios1} = PVE::QemuServer::generate_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..77dd7cca 100644 > --- a/src/PVE/QemuServer.pm > +++ b/src/PVE/QemuServer.pm > @@ -8082,8 +8082,14 @@ sub generate_uuid { > return $uuid_str; > } > > -sub generate_smbios1_uuid { > - return "uuid=" . generate_uuid(); > +sub generate_smbios1 { > + my $smbios1_uuid = PVE::QemuServer::generate_uuid(); > + my $smbios1 = { > + uuid => $smbios1_uuid, > + serial => encode_base64("PVE-" . $smbios1_uuid, ""), > + base64 => 1, > + }; > + return PVE::QemuServer::print_smbios1($smbios1); > } > > sub create_reboot_request {