On Mon, Jul 02, 2018 at 02:38:52PM +0200, Peter Krempa wrote:
> On Mon, Jul 02, 2018 at 13:35:49 +0100, Daniel Berrange wrote:
> > On Mon, Jul 02, 2018 at 02:31:31PM +0200, Peter Krempa wrote:
> > > 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é <berrange@xxxxxxxxxx>
> > > > ---
> > > >
> > > > 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.
> >
> > Won't that cause us to fail to load the VM definition when upgrading existing
> > libvirt.
>
> No. The validation callbacks are not called for existing configs on
> disk. They are re-called on vm startup though so that any existing
> config gets surely validated.
Ok, that's great, I'll make such a change.
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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
