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