[libvirt PATCH v5 3/6] qemu: add vdpa support

[John Ferlan]

On 10/14/20 1:08 PM, Jonathon Jongsma wrote:
> Enable <interface type='vdpa'> for qemu domains. This provides basic
> support and does not support hotplug or migration.
> 
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---
>  src/qemu/qemu_command.c                       | 35 +++++++++++++++--
>  src/qemu/qemu_command.h                       |  3 +-
>  src/qemu/qemu_domain.c                        |  6 ++-
>  src/qemu/qemu_hotplug.c                       | 14 ++++---
>  src/qemu/qemu_interface.c                     | 23 +++++++++++
>  src/qemu/qemu_interface.h                     |  2 +
>  src/qemu/qemu_migration.c                     | 10 ++++-
>  src/qemu/qemu_validate.c                      | 14 +++++++
>  .../net-vdpa.x86_64-latest.args               | 38 +++++++++++++++++++
>  tests/qemuxml2argvdata/net-vdpa.xml           | 28 ++++++++++++++
>  tests/qemuxml2argvmock.c                      | 11 +++++-
>  tests/qemuxml2argvtest.c                      |  1 +
>  tests/qemuxml2xmloutdata/net-vdpa.xml         | 34 +++++++++++++++++
>  tests/qemuxml2xmltest.c                       |  1 +
>  14 files changed, 206 insertions(+), 14 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/net-vdpa.x86_64-latest.args
>  create mode 100644 tests/qemuxml2argvdata/net-vdpa.xml
>  create mode 100644 tests/qemuxml2xmloutdata/net-vdpa.xml

Coverity indicated a possible RESOURCE_LEAK

[...]

> @@ -8203,13 +8212,17 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
>  
>          break;
>  
> +    case VIR_DOMAIN_NET_TYPE_VDPA:
> +        if ((vdpafd = qemuInterfaceVDPAConnect(net)) < 0)
> +            goto cleanup;
> +        break;
> +

Between here and where it gets used/consumed, it's possible to jump to
cleanup. Whether it's technically possible based on various tests made,
I'm not 100% sure. The cleanup code would need to account for
VIR_CLOSE_FORCE(vdpafd) if (vdpafd >= 0)...

>      case VIR_DOMAIN_NET_TYPE_USER:
>      case VIR_DOMAIN_NET_TYPE_SERVER:
>      case VIR_DOMAIN_NET_TYPE_CLIENT:
>      case VIR_DOMAIN_NET_TYPE_MCAST:
>      case VIR_DOMAIN_NET_TYPE_INTERNAL:
>      case VIR_DOMAIN_NET_TYPE_UDP:
> -    case VIR_DOMAIN_NET_TYPE_VDPA:
>      case VIR_DOMAIN_NET_TYPE_LAST:
>          /* nada */
>          break;
> @@ -8327,13 +8340,29 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
>          vhostfd[i] = -1;
>      }
>  
> +    if (vdpafd > 0) {
> +        g_autofree char *fdset = NULL;
> +        g_autofree char *addfdarg = NULL;
> +
> +        virCommandPassFD(cmd, vdpafd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> +        fdset = qemuVirCommandGetFDSet(cmd, vdpafd);
> +        if (!fdset)
> +            goto cleanup;
> +        vdpafdName = qemuVirCommandGetDevSet(cmd, vdpafd);
> +        /* set opaque to the devicepath so that we can look up the fdset later
> +         * if necessary */
> +        addfdarg = g_strdup_printf("%s,opaque=%s", fdset,
> +                                   net->data.vdpa.devicepath);
> +        virCommandAddArgList(cmd, "-add-fd", addfdarg, NULL);
> +    }
> +

As long as the above code consumes vdpafd, then just set it to -1 right
after consumption to avoid double cleanup when it is really closed.


John


>      if (chardev)
>          virCommandAddArgList(cmd, "-chardev", chardev, NULL);
>  
>      if (!(hostnetprops = qemuBuildHostNetStr(net,
>                                               tapfdName, tapfdSize,
>                                               vhostfdName, vhostfdSize,
> -                                             slirpfdName)))
> +                                             slirpfdName, vdpafdName)))
>          goto cleanup;
>  
>      if (!(host = virQEMUBuildNetdevCommandlineFromJSON(hostnetprops,

[...]