[libvirt] [PATCH v2 12/25] qemu: Support disk model=virtio-{non-}transitional

[Andrea Bolognani]

On Wed, 2019-02-06 at 12:17 -0500, Cole Robinson wrote:
> On 1/29/19 5:25 AM, Andrea Bolognani wrote:
> > On Wed, 2019-01-23 at 16:32 -0500, Cole Robinson wrote:
> > [...]
> > > +    switch (devtype) {
> > > +        case VIR_DOMAIN_DEVICE_DISK:
> > > +            has_tmodel = device.data.disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO_TRANSITIONAL;
> > > +            has_ntmodel = device.data.disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO_NON_TRANSITIONAL;
> > > +            tmodel_cap = QEMU_CAPS_DEVICE_VIRTIO_BLK_PCI_TRANSITIONAL;
> > > +            ntmodel_cap = QEMU_CAPS_DEVICE_VIRTIO_BLK_PCI_NON_TRANSITIONAL;
> > > +            break;
> > 
> > I wonder if this would look slightly nicer as
> > 
> >    case VIR_DOMAIN_DEVICE_DISK: {
> >      virDomainDiskDefPtr disk = (virDomainDiskDefPtr) devdata;
> > 
> >      has_tmodel = disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO_TRANSITIONAL;
> >      has_ntmodel = disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO_NON_TRANSITIONAL;
> >      tmodel_cap = QEMU_CAPS_DEVICE_VIRTIO_BLK_PCI_TRANSITIONAL;
> >      ntmodel_cap = QEMU_CAPS_DEVICE_VIRTIO_BLK_PCI_NON_TRANSITIONAL;
> > 
> >      break;
> >    }
> > 
> > but up to you, really.
> 
> Makes for shorter lines which is nice but kind of offsets the benefit of 
> converting to virDomainDeviceSetData in the first place...

A bit, yes: if you did this, then the only use you'd get out of
the virDomainDeviceDef you just reconstructed would be getting a
virDomainDeviceInfo out of it... On the other hand, that also means
you wouldn't be basically open-coding virDomainDeviceGetInfo() in
yet another place.

As I said, pick whichever you like the most :)

-- 
Andrea Bolognani / Red Hat / Virtualization