On 01/18/2019 09:20 AM, Andrea Bolognani wrote:
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@redhat.com> (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.
I'll respond to the rest of the review in time but I just wanted to mention here: I'm working on a series to try and untangle the net model mess and it undoes this hack. It converts nearly all usage of net->model internally to use an enum, but preserves the string passthrough in xen+qemu to be dealt with separately since that will take some discussion and probably need to be phased out after a grace period. I plan to finish + post the series today unless I hit unexpected hurdles. Thanks for the reviews! - Cole