On Mon, Jul 02, 2018 at 10:17:07 +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 deprecating use of -drive, with support being deleted > from -drive in 3.0. > > We keep formatting error policy on -drive for now, because we don't > ahve support for that with -device for usb-storage just yet. > > 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 now report an error in this case. > > Signed-off-by: Daniel P. Berrangé <[email protected]> > --- > > Changed in v3: > > - Report error for setting CHS on USB > - Report error for setting CHS translation mode for bus != IDE > - Use 'bios-chs-trans' property not 'trans' > > src/qemu/qemu_command.c | 46 ++++++++++++++++--- > .../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/qemuxml2xmloutdata/disk-serial.xml | 1 - > 10 files changed, 62 insertions(+), 32 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 4fc3176ad3..46726b055e 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -1599,7 +1599,7 @@ > qemuBuildDiskFrontendAttributeErrorPolicy(virDomainDiskDefPtr disk, > } > > > -static void > +static int > qemuBuildDiskFrontendAttributes(virDomainDiskDefPtr disk, > virBufferPtr buf) > { > @@ -1607,14 +1607,27 @@ qemuBuildDiskFrontendAttributes(virDomainDiskDefPtr > disk, > if (disk->geometry.cylinders > 0 && > disk->geometry.heads > 0 && > disk->geometry.sectors > 0) { > + if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("CHS geometry can not be set for 'usb' bus")); > + return -1; > + }
I'm not a fan of validation in the command line generator. I'd prefer if
you could move this to qemuDomainDeviceDefValidateDisk so that it gets
rejected at define time.
> +
> virBufferAsprintf(buf, ",cyls=%u,heads=%u,secs=%u",
> disk->geometry.cylinders,
> disk->geometry.heads,
> disk->geometry.sectors);
>
> - if (disk->geometry.trans != VIR_DOMAIN_DISK_TRANS_DEFAULT)
> - virBufferAsprintf(buf, ",trans=%s",
> + if (disk->geometry.trans != VIR_DOMAIN_DISK_TRANS_DEFAULT) {
> + if (disk->bus != VIR_DOMAIN_DISK_BUS_IDE) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("CHS translation mode can only be set for
> 'ide' bus not '%s'"),
> + virDomainDiskBusTypeToString(disk->bus));
> + return -1;
This would be a good candidate too.
> + }
> + virBufferAsprintf(buf, ",bios-chs-trans=%s",
>
> virDomainDiskGeometryTransTypeToString(disk->geometry.trans));
> + }
> }
>
> if (disk->serial) {
> @@ -1622,7 +1635,7 @@ qemuBuildDiskFrontendAttributes(virDomainDiskDefPtr
> disk,
> virBufferEscape(buf, '\\', " ", "%s", disk->serial);
> }
>
> - qemuBuildDiskFrontendAttributeErrorPolicy(disk, buf);
> + return 0;
> }
>
>
> @@ -1662,11 +1675,27 @@ 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;
> + }
I don't care that much about the -drive part though since that will
become almost unused.
> }
>
> - /* 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 */
> + 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 +2154,9 @@ qemuBuildDriveDevStr(const virDomainDef *def,
> if (qemuBuildDriveDevCacheStr(disk, &opt, qemuCaps) < 0)
> goto error;
>
> + if (qemuBuildDiskFrontendAttributes(disk, &opt) < 0)
> + goto error;
> +
> if (virBufferCheckError(&opt) < 0)
> goto error;
ACK
signature.asc
Description: PGP signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
