[libvirt] [PATCH 2/9] qemuDomain{Attach, Remove}NetDevice: Prefer qemuDomainSupportsNetdev

[Laine Stump]

On 08/16/2016 11:41 AM, Michal Privoznik wrote:
> Depending on domain OS type, and interface address type we might
> not want to use -netdev even though qemu has the capability. We
> should use more advanced check implemented in
> qemuDomainSupportsNetdev() function.

So the effect of using qemuDomainSupportsNetdev rather than a direct 
check of the capability is that aarch64 network devices that are using 
neither PCI nor virtio-mmio connections would get a different 
commandline. Is there a particular situation where this makes a 
difference, or did it just "look like the right thing to do"? I ask this 
because the more I look at qemuDomainSupportsNetdev() and 
qemuDomainSupportsNicdev(), the more I wonder if they're still 
accomplishing what was originally intended (or if what was originally 
intended is still necessary).

qemuDomainSupportsNicdev() (called by qemuDomainSupportsNetdev()) says 
that its raison d'etre is

     "non-virtio ARM nics require legacy -net nic"

But that's not really what's checked for; instead, it's checking if the 
network device has a virtio-mmio *or PCI* address - the result of this 
test for both virtio-net-pci and for emulated NICs (do they even work on 
aarch64? (e1000 for example - the qemu-system-aarch64 binary says it's 
compiled in, but has it actually been tested, or was it just "not taken 
out"?)) is the same, so virtio and emulated NICs will both get the 
netdev-type commandline.

For that matter, by the time the post-parse is done, any interface 
device that hasn't had a manually-specified <address> element is going 
to have either type='pci' or type='virtio-mmio' (is an address type of 
virtio-mmio a valid thing for a non-virtio emulated device? I wouldn't 
think so, but I'm just going on common sense, not on any actual 
knowledge). And as far as I can see, the only network device that can 
even use any address type other than virtio-mmio or PCI is "usb-net", 
which requires <address type='usb' .../>.

(I looked back through the git history a bit, and that function was 
added prior to support for PCI on ARM (when support for virtio-mmio 
addressing was added - see commits 54a77c and 4fa172), and it looks like 
it was intended to force the use of -net rather than -netdev when 
virtio-mmio addressing isn't used. But before virtio-mmio and support 
for PCI addresses, how were these devices connected in the guest? And 
now is it even a useful thing at all, or is it obsolete? Cole?




>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>   src/qemu/qemu_hotplug.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index d1acdd9..feb1f44 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1108,7 +1108,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>   
>       releaseaddr = true;
>   
> -    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
> +    if (qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) {
>           vlan = -1;
>       } else {
>           vlan = qemuDomainNetVLAN(net);
> @@ -1134,7 +1134,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>               goto cleanup;
>       }
>   
> -    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
> +    if (qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) {
>           if (!(netstr = qemuBuildHostNetStr(net, driver,
>                                              ',', -1,
>                                              tapfdName, tapfdSize,
> @@ -1149,7 +1149,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>       }
>   
>       qemuDomainObjEnterMonitor(driver, vm);
> -    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
> +    if (qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) {
>           if (qemuMonitorAddNetdev(priv->mon, netstr,
>                                    tapfd, tapfdName, tapfdSize,
>                                    vhostfd, vhostfdName, vhostfdSize) < 0) {
> @@ -1195,7 +1195,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>           } else {
>               qemuDomainObjEnterMonitor(driver, vm);
>   
> -            if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
> +            if (qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) {
>                   if (qemuMonitorSetLink(priv->mon, net->info.alias, VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) < 0) {
>                       ignore_value(qemuDomainObjExitMonitor(driver, vm));
>                       virDomainAuditNet(vm, NULL, net, "attach", false);
> @@ -1278,7 +1278,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>           goto cleanup;
>   
>       if (vlan < 0) {
> -        if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
> +        if (qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) {
>               char *netdev_name;
>               if (virAsprintf(&netdev_name, "host%s", net->info.alias) < 0)
>                   goto cleanup;
> @@ -3326,7 +3326,7 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
>           goto cleanup;
>   
>       qemuDomainObjEnterMonitor(driver, vm);
> -    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
> +    if (qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) {
>           if (qemuMonitorRemoveNetdev(priv->mon, hostnet_name) < 0) {
>               if (qemuDomainObjExitMonitor(driver, vm) < 0)
>                   goto cleanup;