Re: [libvirt] [PATCH 31/32] qemu: Use the 'device_id' property of SCSI disks to avoid regressing

On Thu, Jan 31, 2019 at 12:31:58 +0000, Daniel Berrange wrote: > On Thu, Jan 31, 2019 at 01:26:24PM +0100, Peter Krempa wrote: > > On Tue, Jan 29, 2019 at 16:06:35 +0000, Daniel Berrange wrote: > > > On Mon, Jan 28, 2019 at 05:19:00PM +0100, Peter Krempa wrote: > > > > QEMU accidentally exposed the id of -drive (or same value as disk > > > > serial, if provided) in one of the identifiers visible from the guest. > > > > > > > > To avoid regression in case when -blockdev will be used we need to > > > > always specify it ourselves. > > > > > > > > Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; > > > > --- > > > > src/qemu/qemu_command.c | 22 +++++++++++++++++++ > > > > .../controller-virtio-scsi.x86_64-latest.args | 20 ++++++++--------- > > > > .../disk-cache.x86_64-latest.args | 4 ++-- > > > > .../disk-scsi-device-auto.x86_64-latest.args | 3 ++- > > > > .../disk-scsi.x86_64-latest.args | 16 ++++++++------ > > > > .../disk-shared.x86_64-latest.args | 5 +++-- > > > > ...threads-virtio-scsi-pci.x86_64-latest.args | 4 ++-- > > > > 7 files changed, 50 insertions(+), 24 deletions(-) > > > > [...] > > > > > I rather wish there was a way we could avoid exposing the alias ID to every > > > guest forever more. > > > > > > QEMU could achieve that with machine type versioning, so that it only exposes > > > the drive ID to guests with legacy machine types for sake of backport. We need > > > to explicitly set this though, as with -blockdev QEMU can't do the right thing > > > for legacy machine types as it lacks tie drive ID entirely. Once we set it, > > > we enable it for non-legacy machine types too :-( Annoyingly I don't see a > > > way out of this mess such that libvirt only enables the back compat for > > > existing guests. > > > > Do you mean that for any new machine type we should not put the -drive > > ID as the 'device_id' property? AFAIK qemu should now handle that > > correctly by not creating any ID. > > > > In that case we could tie -blockdev to the new machine type only and > > thus will not need to add any code to format the backward compatibility > > stuff. > > The tricky thing is how to tie the -blockdev to the new machine type. > Libvirt generally aims to treat the version part of the machine type > as an opaque string, because it can be arbitrarily changed by distros, > so there's no reliable way to determine "newest" version for a machine > type. At best we know that one of them is the default and so the latest, > but we don't know anything about ordering of others. Well, if we don't want to look at the machine type I don't see much other options than to just always add the property forever.