Re: [libvirt] [PATCH 05/18] qemu: Support interface model=virtio-{non-}transitional

On Thu, 2019-01-17 at 12:52 -0500, Cole Robinson wrote: > Add new <interface> model handling for virtio transitional devices. Ex: > > <interface> > <model type='virtio-transitional'/> > </interface> > > * "virtio-transitional" maps to qemu "virtio-net-pci-transitional" > * "virtio-non-transitional" maps to qemu "virtio-net-pci-non-transitional" > > Signed-off-by: Cole Robinson <crobinso(a)redhat.com&gt; > (cherry picked from commit b6698b81846e2010e0cc030bcd6c2c549cf04e97) Guess this slipped in somehow :) [...] > @@ -1112,6 +1116,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { > { "memory-backend-memfd", QEMU_CAPS_OBJECT_MEMORY_MEMFD }, > {"virtio-blk-pci-transitional", QEMU_CAPS_DEVICE_VIRTIO_BLK_TRANSITIONAL}, > {"virtio-blk-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_BLK_NON_TRANSITIONAL}, > + {"virtio-net-pci-transitional", QEMU_CAPS_DEVICE_VIRTIO_NET_TRANSITIONAL}, > + {"virtio-net-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_NET_NON_TRANSITIONAL}, Same comments about spacing, naming the capabilities and introducing them in their separate commit as for virtio-blk apply. [...] > @@ -449,6 +449,7 @@ qemuBuildVirtioTransitional(virBufferPtr buf, > virQEMUCapsPtr qemuCaps, > virDomainDeviceAddressType type, > int model, > + const char *modelstr, Ewww. We really, really need to make the handling of model names for network interface more sane. In the meantime, can you add some sort of (ideally compile-time) sanity check to ensure that the function is either called with model == -1 && modelStr != NULL or model >= 0 && modelStr == NULL? Or at the very least some documentation on how you're supposed to be calling it.