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@xxxxxxxxxx>
> ---
> 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,
[...]
