Re: [PATCH] qemu: format serial, geometry and error policy on frontend disk device

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

 



On Mon, Jun 25, 2018 at 10:50:32 +0100, Daniel Berrange wrote:
> Currently we format the serial, geometry and error policy on the -drive
> backend argument.
> 
> QEMU added the ability to set serial and geometry on the frontend in
> the 1.2 release, and ability to set error policy in the 2.7 release.
> usb-storage is an oddity, however, as it lacks error policy support
> right now.
> 
> Setting any of these on -drive has been deprecated and support for this
> will be removed very soon.
> 
> Note that some disk buses (sd) still don't support -device. Although
> QEMU allowed these properties to be set on -drive for if=sd, they
> have been ignored so we don't loose anything by removing this.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
>  src/qemu/qemu_capabilities.c                  |  2 ++
>  src/qemu/qemu_capabilities.h                  |  1 +
>  src/qemu/qemu_command.c                       | 31 ++++++++++++++++---
>  .../caps_2.10.0.aarch64.xml                   |  1 +
>  .../caps_2.10.0.ppc64.xml                     |  1 +
>  .../caps_2.10.0.s390x.xml                     |  1 +
>  .../caps_2.10.0.x86_64.xml                    |  1 +
>  .../caps_2.11.0.s390x.xml                     |  1 +
>  .../caps_2.12.0.aarch64.xml                   |  1 +
>  .../caps_2.12.0.ppc64.xml                     |  1 +
>  .../caps_2.12.0.s390x.xml                     |  1 +
>  .../caps_2.12.0.x86_64.xml                    |  1 +
>  .../qemucapabilitiesdata/caps_2.7.0.s390x.xml |  1 +
>  .../caps_2.7.0.x86_64.xml                     |  1 +
>  .../qemucapabilitiesdata/caps_2.8.0.s390x.xml |  1 +
>  .../caps_2.8.0.x86_64.xml                     |  1 +
>  .../qemucapabilitiesdata/caps_2.9.0.ppc64.xml |  1 +
>  .../qemucapabilitiesdata/caps_2.9.0.s390x.xml |  1 +
>  .../caps_2.9.0.x86_64.xml                     |  1 +
>  .../disk-drive-network-tlsx509.args           | 12 +++----
>  .../disk-drive-network-vxhs.args              |  4 +--
>  tests/qemuxml2argvdata/disk-drive-shared.args |  5 +--
>  tests/qemuxml2argvdata/disk-geometry.args     |  6 ++--
>  tests/qemuxml2argvdata/disk-ide-wwn.args      |  5 ++-
>  .../qemuxml2argvdata/disk-scsi-disk-wwn.args  |  4 +--
>  tests/qemuxml2argvdata/disk-serial.args       | 10 +++---
>  tests/qemuxml2argvdata/disk-serial.xml        |  1 -
>  tests/qemuxml2argvtest.c                      |  1 +
>  tests/qemuxml2xmloutdata/disk-serial.xml      |  1 -
>  29 files changed, 69 insertions(+), 30 deletions(-)



> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 419208ad5c..b9a6c2d612 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -500,6 +500,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>  
>                /* 310 */
>                "sev-guest",
> +              "disk-error",
>      );
>  
>  
> @@ -1162,6 +1163,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioBlk[] = {
>      { "iommu_platform", QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM },
>      { "ats", QEMU_CAPS_VIRTIO_PCI_ATS },
>      { "write-cache", QEMU_CAPS_DISK_WRITE_CACHE },
> +    { "werror", QEMU_CAPS_DISK_ERROR },
>  };

Please split-off capability changes into a separate commit as it's
usual.

>  
>  static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioNet[] = {
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 3519a194e9..1245fb46cf 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -484,6 +484,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
>  
>      /* 310 */
>      QEMU_CAPS_SEV_GUEST, /* -object sev-guest,... */
> +    QEMU_CAPS_DISK_ERROR, /* -device $DISK,werror=,rerror=X */
>  
>      QEMU_CAPS_LAST /* this must always be the last item */
>  } virQEMUCapsFlags;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 446df3e1d8..e97158fe64 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1621,8 +1621,6 @@ qemuBuildDiskFrontendAttributes(virDomainDiskDefPtr disk,
>          virBufferAddLit(buf, ",serial=");
>          virBufferEscape(buf, '\\', " ", "%s", disk->serial);
>      }
> -
> -    qemuBuildDiskFrontendAttributeErrorPolicy(disk, buf);
>  }
>  
>  
> @@ -1662,11 +1660,29 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
>          virBufferAsprintf(&opt, "if=%s",
>                            virDomainDiskQEMUBusTypeToString(disk->bus));
>          virBufferAsprintf(&opt, ",index=%d", idx);
> +
> +        if (disk->serial) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Serial property not supported for drive bus '%s'"),
> +                           virDomainDiskBusTypeToString(disk->bus));
> +            goto error;
> +        }
> +        if (disk->geometry.cylinders > 0 &&
> +            disk->geometry.heads > 0 &&
> +            disk->geometry.sectors > 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Geometry not supported for drive bus '%s'"),
> +                           virDomainDiskBusTypeToString(disk->bus));
> +            goto error;
> +        }
>      }
>  
> -    /* Format attributes for the drive itself (not the storage backing it) which
> -     * we've formatted historically with -drive */
> -    qemuBuildDiskFrontendAttributes(disk, &opt);
> +    /* werror/rerror are really frontend attributes, but older
> +     * qemu requires them on -drive instead of -device */
> +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DISK_ERROR) ||
> +        disk->bus == VIR_DOMAIN_DISK_BUS_USB)
> +        qemuBuildDiskFrontendAttributeErrorPolicy(disk, &opt);
> +
>  
>      /* While this is a frontend attribute, it only makes sense to be used when
>       * legacy -drive is used. In modern qemu the 'ide-cd' or 'scsi-cd' are used.
> @@ -2125,6 +2141,11 @@ qemuBuildDriveDevStr(const virDomainDef *def,
>      if (qemuBuildDriveDevCacheStr(disk, &opt, qemuCaps) < 0)
>          goto error;
>  
> +    qemuBuildDiskFrontendAttributes(disk, &opt);
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DISK_ERROR) &&
> +        disk->bus != VIR_DOMAIN_DISK_BUS_USB)

Why is USB special here? Switching to -blockdev will require doing this
also for the USB disk, so I'd rather have a single case here.

> +        qemuBuildDiskFrontendAttributeErrorPolicy(disk, &opt);
> +
>      if (virBufferCheckError(&opt) < 0)
>          goto error;

The rest looks good.

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux