[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]