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
