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@redhat.com>
---
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(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0a62317a33..2036ae663c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1873,6 +1873,7 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
const char *contAlias;
char *backendAlias = NULL;
+ char *scsiVPDDeviceId = NULL;
int controllerModel;
if (qemuCheckDiskConfig(disk, qemuCaps) < 0)
@@ -1962,6 +1963,21 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
virBufferAddLit(&opt, "scsi-cd");
else
virBufferAddLit(&opt, "scsi-hd");
+
+ /* qemu historically used the name of -drive as one of the device
+ * ids in the Vital Product Data Device Identification page if
+ * disk serial was not set and the disk serial otherwise.
+ * To avoid a guest-visible regression we need to provide it
+ * ourselves especially for cases when -blockdev will be used */
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_DEVICE_ID)) {
+ if (disk->serial) {
+ if (VIR_STRDUP(scsiVPDDeviceId, disk->serial) < 0)
+ goto error;
+ } else {
+ if (!(scsiVPDDeviceId = qemuAliasDiskDriveFromDisk(disk)))
+ goto error;
+ }
+ }
}
if (!(contAlias = virDomainControllerAliasFind(def, VIR_DOMAIN_CONTROLLER_TYPE_SCSI,
@@ -2004,6 +2020,12 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
disk->info.addr.drive.target,
disk->info.addr.drive.unit);
}
+
+ if (scsiVPDDeviceId) {
+ virBufferAddLit(&opt, ",device_id=");
+ virBufferEscape(&opt, '\\', " ", "%s", scsiVPDDeviceId);
+ }
+
break;
case VIR_DOMAIN_DISK_BUS_SATA:
diff --git a/tests/qemuxml2argvdata/controller-virtio-scsi.x86_64-latest.args b/tests/qemuxml2argvdata/controller-virtio-scsi.x86_64-latest.args
index cf8a497eb7..095b4724d3 100644
--- a/tests/qemuxml2argvdata/controller-virtio-scsi.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/controller-virtio-scsi.x86_64-latest.args
@@ -30,20 +30,20 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
-device virtio-scsi-pci,id=scsi3,max_sectors=512,bus=pci.0,addr=0x5 \
-device virtio-scsi-pci,id=scsi4,ioeventfd=on,bus=pci.0,addr=0x6 \
-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-scsi0-0-0-0 \
--device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,\
-id=scsi0-0-0-0,bootindex=1 \
+-device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\
+device_id=drive-scsi0-0-0-0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 \
-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-scsi1-0-0-0 \
--device scsi-hd,bus=scsi1.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi1-0-0-0,\
-id=scsi1-0-0-0 \
+-device scsi-hd,bus=scsi1.0,channel=0,scsi-id=0,lun=0,\
+device_id=drive-scsi1-0-0-0,drive=drive-scsi1-0-0-0,id=scsi1-0-0-0 \
-drive file=/dev/HostVG/QEMUGuest3,format=raw,if=none,id=drive-scsi2-0-0-0 \
--device scsi-hd,bus=scsi2.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi2-0-0-0,\
-id=scsi2-0-0-0 \
+-device scsi-hd,bus=scsi2.0,channel=0,scsi-id=0,lun=0,\
+device_id=drive-scsi2-0-0-0,drive=drive-scsi2-0-0-0,id=scsi2-0-0-0 \
-drive file=/dev/HostVG/QEMUGuest4,format=raw,if=none,id=drive-scsi3-0-0-0 \
--device scsi-hd,bus=scsi3.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi3-0-0-0,\
-id=scsi3-0-0-0 \
+-device scsi-hd,bus=scsi3.0,channel=0,scsi-id=0,lun=0,\
+device_id=drive-scsi3-0-0-0,drive=drive-scsi3-0-0-0,id=scsi3-0-0-0 \
-drive file=/dev/HostVG/QEMUGuest5,format=raw,if=none,id=drive-scsi4-0-0-0 \
--device scsi-hd,bus=scsi4.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi4-0-0-0,\
-id=scsi4-0-0-0 \
+-device scsi-hd,bus=scsi4.0,channel=0,scsi-id=0,lun=0,\
+device_id=drive-scsi4-0-0-0,drive=drive-scsi4-0-0-0,id=scsi4-0-0-0 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x7 \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
resourcecontrol=deny \
diff --git a/tests/qemuxml2argvdata/disk-cache.x86_64-latest.args b/tests/qemuxml2argvdata/disk-cache.x86_64-latest.args
index 9220e6bee9..7f703d0861 100644
--- a/tests/qemuxml2argvdata/disk-cache.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-cache.x86_64-latest.args
@@ -35,8 +35,8 @@ cache=unsafe \
write-cache=on \
-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-scsi0-0-0,\
cache=none \
--device scsi-hd,bus=scsi0.0,scsi-id=0,drive=drive-scsi0-0-0,id=scsi0-0-0,\
-write-cache=on \
+-device scsi-hd,bus=scsi0.0,scsi-id=0,device_id=drive-scsi0-0-0,\
+drive=drive-scsi0-0-0,id=scsi0-0-0,write-cache=on \
-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-virtio-disk0,\
cache=writethrough \
-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,\
diff --git a/tests/qemuxml2argvdata/disk-scsi-device-auto.x86_64-latest.args b/tests/qemuxml2argvdata/disk-scsi-device-auto.x86_64-latest.args
index 549a162808..508daa0365 100644
--- a/tests/qemuxml2argvdata/disk-scsi-device-auto.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-scsi-device-auto.x86_64-latest.args
@@ -28,7 +28,8 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
-drive file=/tmp/scsidisk.img,format=raw,if=none,id=drive-scsi0-0-0 \
--device scsi-hd,bus=scsi0.0,scsi-id=0,drive=drive-scsi0-0-0,id=scsi0-0-0 \
+-device scsi-hd,bus=scsi0.0,scsi-id=0,device_id=drive-scsi0-0-0,\
+drive=drive-scsi0-0-0,id=scsi0-0-0 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
resourcecontrol=deny \
diff --git a/tests/qemuxml2argvdata/disk-scsi.x86_64-latest.args b/tests/qemuxml2argvdata/disk-scsi.x86_64-latest.args
index 68a8b2921d..e5b930a28a 100644
--- a/tests/qemuxml2argvdata/disk-scsi.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-scsi.x86_64-latest.args
@@ -31,16 +31,18 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
-drive file=/tmp/scsidisk.img,format=raw,if=none,id=drive-scsi0-0-0 \
--device scsi-hd,bus=scsi0.0,scsi-id=0,drive=drive-scsi0-0-0,id=scsi0-0-0 \
+-device scsi-hd,bus=scsi0.0,scsi-id=0,device_id=drive-scsi0-0-0,\
+drive=drive-scsi0-0-0,id=scsi0-0-0 \
-drive file=/tmp/scsidisk2.img,format=raw,if=none,id=drive-scsi1-0-0-0 \
--device scsi-hd,bus=scsi1.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi1-0-0-0,\
-id=scsi1-0-0-0,serial=abcdefghijklmn \
+-device scsi-hd,bus=scsi1.0,channel=0,scsi-id=0,lun=0,device_id=abcdefghijklmn,\
+drive=drive-scsi1-0-0-0,id=scsi1-0-0-0,serial=abcdefghijklmn \
-drive file=/tmp/scsidisk3.img,format=raw,if=none,id=drive-scsi2-0-0-0 \
--device scsi-hd,bus=scsi2.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi2-0-0-0,\
-id=scsi2-0-0-0,wwn=0x5000c50015ea71ac \
+-device scsi-hd,bus=scsi2.0,channel=0,scsi-id=0,lun=0,\
+device_id=drive-scsi2-0-0-0,drive=drive-scsi2-0-0-0,id=scsi2-0-0-0,\
+wwn=0x5000c50015ea71ac \
-drive file=/tmp/scsidisk4.img,format=raw,if=none,id=drive-scsi3-0-0-0 \
--device scsi-hd,bus=scsi3.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi3-0-0-0,\
-id=scsi3-0-0-0 \
+-device scsi-hd,bus=scsi3.0,channel=0,scsi-id=0,lun=0,\
+device_id=drive-scsi3-0-0-0,drive=drive-scsi3-0-0-0,id=scsi3-0-0-0 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
resourcecontrol=deny \
diff --git a/tests/qemuxml2argvdata/disk-shared.x86_64-latest.args b/tests/qemuxml2argvdata/disk-shared.x86_64-latest.args
index 4913d1ed20..141cf735a5 100644
--- a/tests/qemuxml2argvdata/disk-shared.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-shared.x86_64-latest.args
@@ -33,8 +33,9 @@ bootindex=1,write-cache=on,serial=XYZXYZXYZYXXYZYZYXYZY \
readonly=on \
-device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \
-drive file=/dev/scsi,format=raw,if=none,id=drive-scsi0-0-0-0,cache=none \
--device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,share-rw=on,\
-drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,write-cache=on \
+-device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\
+device_id=drive-scsi0-0-0-0,share-rw=on,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,\
+write-cache=on \
-drive file=/dev/virtio,format=raw,if=none,id=drive-virtio-disk0,cache=none \
-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x3,share-rw=on,\
drive=drive-virtio-disk0,id=virtio-disk0,write-cache=on \
diff --git a/tests/qemuxml2argvdata/iothreads-virtio-scsi-pci.x86_64-latest.args b/tests/qemuxml2argvdata/iothreads-virtio-scsi-pci.x86_64-latest.args
index 4513edcdd0..cde42c7dd1 100644
--- a/tests/qemuxml2argvdata/iothreads-virtio-scsi-pci.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/iothreads-virtio-scsi-pci.x86_64-latest.args
@@ -35,8 +35,8 @@ id=drive-virtio-disk1 \
drive=drive-virtio-disk1,id=virtio-disk1 \
-drive file=/var/lib/libvirt/images/iothrtest2.img,format=raw,if=none,\
id=drive-scsi0-0-0-3 \
--device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=3,drive=drive-scsi0-0-0-3,\
-id=scsi0-0-0-3 \
+-device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=3,\
+device_id=drive-scsi0-0-0-3,drive=drive-scsi0-0-0-3,id=scsi0-0-0-3 \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
resourcecontrol=deny \
-msg timestamp=on
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com>
> ---
> 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(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 0a62317a33..2036ae663c 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1873,6 +1873,7 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
> const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
> const char *contAlias;
> char *backendAlias = NULL;
> + char *scsiVPDDeviceId = NULL;
> int controllerModel;
>
> if (qemuCheckDiskConfig(disk, qemuCaps) < 0)
> @@ -1962,6 +1963,21 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
> virBufferAddLit(&opt, "scsi-cd");
> else
> virBufferAddLit(&opt, "scsi-hd");
> +
> + /* qemu historically used the name of -drive as one of the device
> + * ids in the Vital Product Data Device Identification page if
> + * disk serial was not set and the disk serial otherwise.
> + * To avoid a guest-visible regression we need to provide it
> + * ourselves especially for cases when -blockdev will be used */
> + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_DEVICE_ID)) {
> + if (disk->serial) {
> + if (VIR_STRDUP(scsiVPDDeviceId, disk->serial) < 0)
> + goto error;
> + } else {
> + if (!(scsiVPDDeviceId = qemuAliasDiskDriveFromDisk(disk)))
> + goto error;
> + }
> + }
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.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com> > > --- > > 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. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com> > > > --- > > > 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. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com> > > > > --- > > > > 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. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jan 31, 2019 at 01:56:57PM +0100, Peter Krempa wrote: > 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@redhat.com> > > > > > --- > > > > > 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. The only thing I can imagine working is if QEMU extends the 'query-machines' QMP data to add a flag "use-blockdev" to indicate whether it is ok to use blockdev or not. This feels like too much of a special case hack to warrant exposing in QMP though. So I think we'll just have to live with the gross property forever Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.