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.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
