[libvirt PATCH v2 3/3] qemu: add vdpa support
Laine Stump
laine at redhat.com
Thu Sep 3 04:55:56 UTC 2020
On 9/2/20 3:25 PM, Jonathon Jongsma wrote:
> Enable <interface type='vdpa'> for qemu domains. This provides basic
> support and does not support hotplug or migration.
Is there something specifically preventing hotplug, or you just haven't
implemented it yet?
(How does that work with FD passing, anyway? I haven't really looked at
the details of FD passing...)
If hotplug is possible, then I really think that needs to be implemented
in the initial patch set.
Same question for migration - is it not possible with these devices, or
we're just not certain or are missing some pieces so we're disabling it
out of caution? For migration it's even more important to implement it
with the initial patches if it works at all. Otherwise, once we do
implement it, we'll have to have a way of detecting whether or not the
destination of a migration supports migrating vdpa devices. (I can see
where it simply may not work, since I guess the hardware's memory is
used for packet buffers, right?)
>
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---
> src/qemu/qemu_command.c | 28 ++++++++++++--
> src/qemu/qemu_command.h | 3 +-
> src/qemu/qemu_domain.c | 6 +++
> src/qemu/qemu_hotplug.c | 12 +++---
> src/qemu/qemu_interface.c | 23 ++++++++++++
> src/qemu/qemu_interface.h | 2 +
> src/qemu/qemu_migration.c | 10 ++++-
> .../net-vdpa.x86_64-latest.args | 37 +++++++++++++++++++
> tests/qemuxml2argvmock.c | 11 +++++-
> tests/qemuxml2argvtest.c | 1 +
> 10 files changed, 122 insertions(+), 11 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/net-vdpa.x86_64-latest.args
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 7b7176eb72..b9830292ee 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3553,7 +3553,8 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
> size_t tapfdSize,
> char **vhostfd,
> size_t vhostfdSize,
> - const char *slirpfd)
> + const char *slirpfd,
> + const char *vdpadev)
> {
> bool is_tap = false;
> virDomainNetType netType = virDomainNetGetActualType(net);
> @@ -3692,6 +3693,12 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
> break;
>
> case VIR_DOMAIN_NET_TYPE_VDPA:
> + /* Caller will pass the fd to qemu with add-fd */
> + if (virJSONValueObjectCreate(&netprops, "s:type", "vhost-vdpa", NULL) < 0 ||
> + virJSONValueObjectAppendString(netprops, "vhostdev", vdpadev) < 0)
> + return NULL;
> + break;
> +
> case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> /* Should have been handled earlier via PCI/USB hotplug code. */
> case VIR_DOMAIN_NET_TYPE_LAST:
> @@ -8017,6 +8024,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
> char **tapfdName = NULL;
> char **vhostfdName = NULL;
> g_autofree char *slirpfdName = NULL;
> + g_autofree char *vdpafdName = NULL;
> + int vdpafd = -1;
> virDomainNetType actualType = virDomainNetGetActualType(net);
> const virNetDevBandwidth *actualBandwidth;
> bool requireNicdev = false;
> @@ -8102,13 +8111,17 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
>
> break;
>
> + case VIR_DOMAIN_NET_TYPE_VDPA:
> + if ((vdpafd = qemuInterfaceVDPAConnect(net)) < 0)
> + goto cleanup;
> + break;
> +
> 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;
> @@ -8225,13 +8238,22 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
> vhostfd[i] = -1;
> }
>
> + if (vdpafd > 0) {
> + virCommandPassFD(cmd, vdpafd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> + g_autofree char *fdset = qemuVirCommandGetFDSet(cmd, vdpafd);
> + if (!fdset)
> + goto cleanup;
> + virCommandAddArgList(cmd, "-add-fd", fdset, NULL);
> + vdpafdName = qemuVirCommandGetDevSet(cmd, vdpafd);
> + }
> +
> if (chardev)
> virCommandAddArgList(cmd, "-chardev", chardev, NULL);
>
> if (!(hostnetprops = qemuBuildHostNetStr(net,
> tapfdName, tapfdSize,
> vhostfdName, vhostfdSize,
> - slirpfdName)))
> + slirpfdName, vdpafdName)))
> goto cleanup;
>
> if (!(host = virQEMUBuildNetdevCommandlineFromJSON(hostnetprops,
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 89d99b111f..8db51f93b1 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -99,7 +99,8 @@ virJSONValuePtr qemuBuildHostNetStr(virDomainNetDefPtr net,
> size_t tapfdSize,
> char **vhostfd,
> size_t vhostfdSize,
> - const char *slirpfd);
> + const char *slirpfd,
> + const char *vdpadev);
>
> /* Current, best practice */
> char *qemuBuildNicDevStr(virDomainDefPtr def,
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 680e7d5bf8..b94a181043 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -4488,6 +4488,12 @@ qemuDomainValidateActualNetDef(const virDomainNetDef *net,
> macstr, virDomainNetTypeToString(actualType));
> return -1;
> }
> + if (actualType == VIR_DOMAIN_NET_TYPE_VDPA &&
> + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV_VHOST_VDPA)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("vDPA device is not supported with this QEMU binary"));
> + return -1;
> + }
Note that this function isn't called until the domain is being started.
Since a type='network' interface can't have an ActualType of VDPA, we
can do this validation at domain definition time, in
qemuValidateDomainDeviceDefNetwork() (writing this, and looking at the
other functions called, points out to me that those validation functions
*really* need more consistent names, and also that
qemuDomainValidateActualNetDef() should probably be moved from
qemu_domain.c to qemu_validate.c. But neither of those is your problem;
just move this check to the other function so it happens at domain
define time rather than domain start time.
> return 0;
> }
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 78dd5e9f19..45c07a7f4e 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1389,7 +1389,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
> if (!(netprops = qemuBuildHostNetStr(net,
> tapfdName, tapfdSize,
> vhostfdName, vhostfdSize,
> - slirpfdName)))
> + slirpfdName, NULL)))
> goto cleanup;
>
> qemuDomainObjEnterMonitor(driver, vm);
> @@ -3484,8 +3484,9 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
> olddev = *devslot;
>
> oldType = virDomainNetGetActualType(olddev);
> - if (oldType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
> - /* no changes are possible to a type='hostdev' interface */
> + if (oldType == VIR_DOMAIN_NET_TYPE_HOSTDEV ||
> + oldType == VIR_DOMAIN_NET_TYPE_VDPA) {
> + /* no changes are possible to a type='hostdev' or type='vdpa' interface */
> virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> _("cannot change config of '%s' network type"),
> virDomainNetTypeToString(oldType));
> @@ -3672,8 +3673,9 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
>
> newType = virDomainNetGetActualType(newdev);
>
> - if (newType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
> - /* can't turn it into a type='hostdev' interface */
> + if (newType == VIR_DOMAIN_NET_TYPE_HOSTDEV ||
> + newType == VIR_DOMAIN_NET_TYPE_VDPA) {
> + /* can't turn it into a type='hostdev' or type='vdpa' interface */
> virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> _("cannot change network interface type to '%s'"),
> virDomainNetTypeToString(newType));
> diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
> index 33157dbbed..a91563dd72 100644
> --- a/src/qemu/qemu_interface.c
> +++ b/src/qemu/qemu_interface.c
> @@ -641,6 +641,29 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def,
> }
>
>
> +/* qemuInterfaceVDPAConnect:
> + * @net: pointer to the VM's interface description
> + *
> + * returns: file descriptor of the vdpa device
> + *
> + * Called *only* called if actualType is VIR_DOMAIN_NET_TYPE_VDPA
> + */
> +int
> +qemuInterfaceVDPAConnect(virDomainNetDefPtr net)
> +{
> + int fd;
> +
> + if ((fd = open(net->data.vdpa.devicepath, O_RDWR)) < 0) {
> + virReportSystemError(errno,
> + _("Unable to open '%s' for vdpa device"),
> + net->data.vdpa.devicepath);
> + return -1;
> + }
> +
> + return fd;
> +}
> +
> +
> qemuSlirpPtr
> qemuInterfacePrepareSlirp(virQEMUDriverPtr driver,
> virDomainNetDefPtr net)
> diff --git a/src/qemu/qemu_interface.h b/src/qemu/qemu_interface.h
> index 3dcefc6a12..1ba24f0a6f 100644
> --- a/src/qemu/qemu_interface.h
> +++ b/src/qemu/qemu_interface.h
> @@ -58,3 +58,5 @@ int qemuInterfaceOpenVhostNet(virDomainDefPtr def,
>
> qemuSlirpPtr qemuInterfacePrepareSlirp(virQEMUDriverPtr driver,
> virDomainNetDefPtr net);
> +
> +int qemuInterfaceVDPAConnect(virDomainNetDefPtr net) G_GNUC_NO_INLINE;
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 142faa2cf9..238abadd6a 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1281,7 +1281,15 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver,
>
> for (i = 0; i < vm->def->nnets; i++) {
> virDomainNetDefPtr net = vm->def->nets[i];
> - qemuSlirpPtr slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp;
> + qemuSlirpPtr slirp;
> +
> + if (net->type == VIR_DOMAIN_NET_TYPE_VDPA) {
> + virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("a vDPA device cannot be migrated"));
> + return false;
> + }
> +
> + slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp;
Interesting that the original code didn't check the device type before
looking at the slirp privateData. I guess it isn't really necessary,
since that will be NULL unless type==slirp anyway, but...
Maybe this should be done with a switch(virDomainNetGetActualType(net))
so that new interface types are reminded they may need to do something
for this.
>
> if (slirp && !qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE)) {
> virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> diff --git a/tests/qemuxml2argvdata/net-vdpa.x86_64-latest.args b/tests/qemuxml2argvdata/net-vdpa.x86_64-latest.args
> new file mode 100644
> index 0000000000..8e76ac7794
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/net-vdpa.x86_64-latest.args
> @@ -0,0 +1,37 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/tmp/lib/domain--1-QEMUGuest1 \
> +USER=test \
> +LOGNAME=test \
> +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
> +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
> +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu-system-i386 \
> +-name guest=QEMUGuest1,debug-threads=on \
> +-S \
> +-object secret,id=masterKey0,format=raw,\
> +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
> +-machine pc,accel=tcg,usb=off,dump-guest-core=off \
> +-cpu qemu64 \
> +-m 214 \
> +-overcommit mem-lock=off \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-display none \
> +-no-user-config \
> +-nodefaults \
> +-chardev socket,id=charmonitor,fd=1729,server,nowait \
> +-mon chardev=charmonitor,id=monitor,mode=control \
> +-rtc base=utc \
> +-no-shutdown \
> +-no-acpi \
> +-boot strict=on \
> +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
> +-add-fd set=0,fd=1732 \
> +-netdev vhost-vdpa,vhostdev=/dev/fdset/0,id=hostnet0 \
> +-device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:95:db:c0,bus=pci.0,\
> +addr=0x2 \
> +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
> +resourcecontrol=deny \
> +-msg timestamp=on
> diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
> index e5841bc8e3..516776697f 100644
> --- a/tests/qemuxml2argvmock.c
> +++ b/tests/qemuxml2argvmock.c
> @@ -205,7 +205,7 @@ virHostGetDRMRenderNode(void)
>
> static void (*real_virCommandPassFD)(virCommandPtr cmd, int fd, unsigned int flags);
>
> -static const int testCommandPassSafeFDs[] = { 1730, 1731 };
> +static const int testCommandPassSafeFDs[] = { 1730, 1731, 1732 };
>
> void
> virCommandPassFD(virCommandPtr cmd,
> @@ -283,3 +283,12 @@ qemuBuildTPMOpenBackendFDs(const char *tpmdev G_GNUC_UNUSED,
> *cancelfd = 1731;
> return 0;
> }
> +
> +
> +int
> +qemuInterfaceVDPAConnect(virDomainNetDefPtr net G_GNUC_UNUSED)
> +{
> + if (fcntl(1732, F_GETFD) != -1)
> + abort();
> + return 1732;
> +}
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index e93948e3fc..000d6919f2 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -1446,6 +1446,7 @@ mymain(void)
> QEMU_CAPS_DEVICE_VFIO_PCI);
> DO_TEST_FAILURE("net-hostdev-fail",
> QEMU_CAPS_DEVICE_VFIO_PCI);
> + DO_TEST_CAPS_LATEST("net-vdpa");
>
> DO_TEST("hostdev-pci-multifunction",
> QEMU_CAPS_KVM,
Any luck finding hardware to test it? I'm curious if all the SELinux t's
are crossed and i's dotted.
More information about the libvir-list
mailing list