[libvirt] [PATCH v3] qemu: format serial and geometry on frontend disk device

[Peter Krempa]

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 at redhat.com>
> > > ---
> > > 
> > > 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180702/dfc85359/attachment-0001.sig>