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