From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id B5FD51FF0BC for ; Thu, 29 Oct 2026 15:26:33 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id EE433213BA; Thu, 29 Oct 2026 15:26:30 +0100 (CET) Message-ID: Date: Thu, 29 Oct 2026 15:26:22 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH qemu-server 1/2] drive device: use configured serial as SCSI device ID To: =?UTF-8?Q?Michael_K=C3=B6ppl?= , pve-devel@lists.proxmox.com References: <20261028190713.2788477-1-m.koeppl@proxmox.com> <20261028190713.2788477-2-m.koeppl@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20261028190713.2788477-2-m.koeppl@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793283982626 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.324 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust 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: EQP6SEKCRWJTVUESXBROC2WTTLQXYNGN X-Message-ID-Hash: EQP6SEKCRWJTVUESXBROC2WTTLQXYNGN X-MailFrom: f.ebner@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: Am 28.10.26 um 8:07 PM schrieb Michael Köppl: > Since the switch to '-blockdev', the device ID of SCSI disks is > explicitly set to the drive name, even if serial is configured. So with > machine version 10.0 or newer, a SCSI disk with a serial reports the > drive name (e.g. 'drive-scsi1') and the /dev/disk/by-id paths that udev > derives from it no longer contain the serial. In a guest running Proxmox > VE, the disk list API endpoint (/nodes//disks/list) therefore does > not report the configured serial either. > > Set the device ID explicitly to the serial if one is configured, like > libvirt does since its switch to '-blockdev' [0]. For serials of up to > 20 bytes, this is the same device ID that QEMU used by default with > '-drive'. > > Relying on QEMU's default instead would not work for all serials. QEMU > accepts serials of up to 36 bytes [1], but only uses serials of up to > 20 bytes as the default device ID and refuses to start the device for > longer ones instead of truncating them [2][3]. Orthogonal to the series, but turns out, our schema allows serials longer than that, because it has maxLength => 20 * 3, # *3 since it's %xx url enoded but nothing enforces that each character is url-encoded, so regular alphanumeric characters do not contribute 3 each. On the other hand, it still has the old limit of 20 if all characters are url-encoded. We cannot just enforce it for existing values, as that would be a breaking change, but we could start checking the actual decoded length for new values (and using a limit of 36 there). But again, this is orthogonal. > > As this changes what the guest sees, only do so starting with machine > version 11.1+pve1. Check it with the version guard, so that only VMs > with a SCSI disk with a serial require the new revision. > > [0]: https://gitlab.com/libvirt/libvirt/-/commit/a1dce96236f6d35167924fa7e6a70f58f394b23c > [1]: https://lists.gnu.org/archive/html/qemu-stable/2016-09/msg00117.html > [2]: https://lists.gnu.org/archive/html/qemu-block/2024-06/msg00027.html > [3]: https://github.com/systemd/systemd/issues/39262 > > Fixes: 46d1225e ("drive device: fix regression with missing '/dev/disk/by-id' paths") > > Signed-off-by: Michael Köppl > --- > Added a machine version 11.1+pve1 specifically for this. If it makes > more sense to wait for 11.2, considering this is not an urgent issue, > I'll of course send a v2. Should be a bit simpler to implement anyway. Since AFAIK, nobody except you complained yet, we might want to wait for 11.2, yes. Maybe also print a line notifying users who have 'serial' configured and using 'latest' machine version about the upcoming change and tell them to pin the machine version if they don't want the change? I'm even somewhat inclined to delay it to the next major release, because it is a guest ABI change that can have quite some fallout unfortunately if the guest relies on the specific device path. Then we can scan in pve9to10 for disks with serial and using 'latest' machine type and warn accordingly. > > src/PVE/QemuServer.pm | 7 ++++--- > src/PVE/QemuServer/DriveDevice.pm | 22 +++++++++++++++++++--- > src/PVE/QemuServer/Machine.pm | 6 ++++++ > 3 files changed, 29 insertions(+), 6 deletions(-) > > diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm > index 1efeeb66..36471bdf 100644 > --- a/src/PVE/QemuServer.pm > +++ b/src/PVE/QemuServer.pm > @@ -3686,9 +3686,10 @@ sub config_to_command { > push @$devices, '-drive', $drive_cmd; > } > > - push @$devices, '-device', print_drivedevice_full( > - $storecfg, $conf, $vmid, $drive, $arch, $machine_type, > - ); > + push @$devices, '-device', > + print_drivedevice_full( > + $storecfg, $conf, $vmid, $drive, $arch, $machine_type, $version_guard, > + ); > }, > ); > > diff --git a/src/PVE/QemuServer/DriveDevice.pm b/src/PVE/QemuServer/DriveDevice.pm > index 621fcc2b..980a217a 100644 > --- a/src/PVE/QemuServer/DriveDevice.pm > +++ b/src/PVE/QemuServer/DriveDevice.pm > @@ -69,13 +69,23 @@ sub get_max_scsihw_index { > } > > sub print_drivedevice_full { > - my ($storecfg, $conf, $vmid, $drive, $arch, $machine_type) = @_; > + my ( > + $storecfg, > + $conf, > + $vmid, > + $drive, > + $arch, > + $machine_type, > + $version_guard, # checks the machine version and records the required +pveX revision > + ) = @_; > > my $device = ''; > my $maxdev = 0; > > my $machine_version = > PVE::QemuServer::Machine::extract_version($machine_type, kvm_user_version()); > + # when hotplugging, the machine is already running, so there is no required version to record > + $version_guard //= sub { return min_version($machine_version, @_); }; This should be at the call site for hotplug. In fact, it already is, just needs to moved out of the branch for 'netX' to the top-level of vm_deviceplug(). > my $has_write_cache = 1; # whether the device has a 'write-cache' option > > my $use_blockdev = min_version($machine_version, 10, 0); > @@ -112,9 +122,15 @@ sub print_drivedevice_full { > $device .= ",id=$drive_id"; > > # For the switch to -blockdev, the SCSI device ID needs to be explicitly specified. Note > - # that only ide-cd and ide-hd have a 'device_id' option. > + # that only ide-cd and ide-hd have a 'device_id' option. Since machine version 11.1+pve1, > + # a configured serial is used as the device ID, like QEMU's default with -drive. > if ($use_blockdev && ($device_type eq 'cd' || $device_type eq 'hd')) { > - $device .= ",device_id=drive-${drive_id}"; > + if ($drive->{serial} && $version_guard->(11, 1, 1)) { > + my $serial = _escape_qemu_arg_value(URI::Escape::uri_unescape($drive->{serial})); > + $device .= ",device_id=$serial"; > + } else { > + $device .= ",device_id=drive-${drive_id}"; > + } > } > > if ($drive->{ssd} && ($device_type eq 'block' || $device_type eq 'hd')) { > diff --git a/src/PVE/QemuServer/Machine.pm b/src/PVE/QemuServer/Machine.pm > index 35161d1f..68cc5daf 100644 > --- a/src/PVE/QemuServer/Machine.pm > +++ b/src/PVE/QemuServer/Machine.pm > @@ -50,6 +50,12 @@ our $PVE_MACHINE_VERSION = { > '+pve2' => 'Enable hv_emcs for Windows 11/2022/2025 to mitigate issues with VBS.', > }, > }, > + '11.1' => { > + highest => 1, > + revisions => { > + '+pve1' => 'Use the configured serial as device ID for SCSI disks.', > + }, > + }, > }; > > my $machine_fmt = { > -- > 2.47.3 > > > > >