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

[Andrea Bolognani]

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 at 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.

[...]
> @@ -3741,8 +3747,11 @@ qemuBuildNicDevStr(virDomainDefPtr def,
>      bool usingVirtio = false;
>      char macaddr[VIR_MAC_STRING_BUFLEN];
>  
> -    if (STREQ(net->model, "virtio")) {
> -        if (qemuBuildVirtioDevStr(&buf, "virtio-net", net->info.type) < 0)
> +    if (virDomainNetHasVirtioModel(net)) {
> +        if (qemuBuildVirtioTransitional(&buf, "virtio-net", qemuCaps,
> +                                        net->info.type,
> +                                        0, net->model,
> +                                        VIR_DOMAIN_DEVICE_NET) < 0)

The change from STREQ() to virDomainNetHasVirtioModel() should be
squashed into the previous patch.

-- 
Andrea Bolognani / Red Hat / Virtualization