We haven't checked for memalloc failure in many years, and that was the only reason this function would have ever failed. Signed-off-by: Laine Stump <laine(a)redhat.com&gt; --- src/conf/domain_conf.c | 15 +++++---------- src/conf/domain_conf.h | 2 +- src/libxl/libxl_domain.c | 5 +---- src/libxl/libxl_driver.c | 3 +-- src/lxc/lxc_driver.c | 3 +-- src/qemu/qemu_driver.c | 3 +-- src/qemu/qemu_process.c | 3 +-- 7 files changed, 11 insertions(+), 23 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 87f87bbe56..50dc4a33a6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14465,12 +14465,10 @@ virDomainChrTargetTypeToString(int deviceType, return type; } -int +void virDomainHostdevInsert(virDomainDef *def, virDomainHostdevDef *hostdev) { VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev); - - return 0; } virDomainHostdevDef * @@ -14886,9 +14884,8 @@ virDomainDiskRemoveByName(virDomainDef *def, const char *name) int virDomainNetInsert(virDomainDef *def, virDomainNetDef *net) { /* hostdev net devices must also exist in the hostdevs array */ - if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV && - virDomainHostdevInsert(def, &net->data.hostdev.def) < 0) - return -1; + if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) + virDomainHostdevInsert(def, &net->data.hostdev.def); VIR_APPEND_ELEMENT(def->nets, def->nnets, net); return 0; @@ -19281,10 +19278,8 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, * where the actual network type is already known to be * hostdev) must also be in the hostdevs array. */ - if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV && - virDomainHostdevInsert(def, virDomainNetGetActualHostdev(net)) < 0) { - return NULL; - } + if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) + virDomainHostdevInsert(def, virDomainNetGetActualHostdev(net)); } VIR_FREE(nodes); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e51c74b6d1..9da6586e66 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3994,7 +3994,7 @@ virDomainNetDef *virDomainNetRemove(virDomainDef *def, size_t i); virDomainNetDef *virDomainNetRemoveByObj(virDomainDef *def, virDomainNetDef *net); void virDomainNetRemoveHostdev(virDomainDef *def, virDomainNetDef *net); -int virDomainHostdevInsert(virDomainDef *def, virDomainHostdevDef *hostdev); +void virDomainHostdevInsert(virDomainDef *def, virDomainHostdevDef *hostdev); virDomainHostdevDef * virDomainHostdevRemove(virDomainDef *def, size_t i); int virDomainHostdevFind(virDomainDef *def, virDomainHostdevDef *match, diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index a049cdb30f..6805160923 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -1014,10 +1014,7 @@ libxlNetworkPrepareDevices(virDomainDef *def) /* Each type='hostdev' network device must also have a * corresponding entry in the hostdevs array. */ - virDomainHostdevDef *hostdev = virDomainNetGetActualHostdev(net); - - if (virDomainHostdevInsert(def, hostdev) < 0) - return -1; + virDomainHostdevInsert(def, virDomainNetGetActualHostdev(net)); } } diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index bd858d8127..edf7b37581 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3574,8 +3574,7 @@ libxlDomainAttachDeviceConfig(virDomainDef *vmdef, virDomainDeviceDef *dev) return -1; } - if (virDomainHostdevInsert(vmdef, hostdev) < 0) - return -1; + virDomainHostdevInsert(vmdef, hostdev); dev->data.hostdev = NULL; break; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index e63732dbea..22266c1ab6 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2993,8 +2993,7 @@ lxcDomainAttachDeviceConfig(virDomainDef *vmdef, _("device is already in the domain configuration")); return -1; } - if (virDomainHostdevInsert(vmdef, hostdev) < 0) - return -1; + virDomainHostdevInsert(vmdef, hostdev); dev->data.hostdev = NULL; ret = 0; break; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 772cb405d6..1d0da1028f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6732,8 +6732,7 @@ qemuDomainAttachDeviceConfig(virDomainDef *vmdef, _("device is already in the domain configuration")); return -1; } - if (virDomainHostdevInsert(vmdef, hostdev)) - return -1; + virDomainHostdevInsert(vmdef, hostdev); dev->data.hostdev = NULL; break; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d015285b0d..910229a616 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5928,8 +5928,7 @@ qemuProcessPrepareDomainNetwork(virDomainObj *vm) if (qemuDomainPrepareHostdev(hostdev, priv) < 0) return -1; - if (virDomainHostdevInsert(def, hostdev) < 0) - return -1; + virDomainHostdevInsert(def, hostdev); } } return 0; -- 2.47.1

Both vhostuser and vdpa interface types must use the virtio model in the guest (because part of the functionality is implemented in the guest virtio driver). Due to ["because that's the way it happened"] this has been validated for vhostuser in the hypervisor-agnostic validate function, but for vdpa it has been done in the QEMU-specific validate. Since these interface models are only supported by QEMU anyway, validate for both of them in the QEMU validation function. Take advantage of this change to switch to using virDomainNetIsVirtioModel(net) instead of "net->model == VIR_DOMAIN_NET_MODEL_VIRTIO" (the former also matches ...VIRTIO_TRANSITIONAL and ...VIRTIO_NON_TRANSITIONAL, so is more correct). Signed-off-by: Laine Stump <laine(a)redhat.com&gt; --- src/conf/domain_validate.c | 6 ------ src/qemu/qemu_validate.c | 11 ++++++----- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index eb5e764c02..d0e2bcaccf 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -2186,12 +2186,6 @@ virDomainNetDefValidate(const virDomainNetDef *net) switch (net->type) { case VIR_DOMAIN_NET_TYPE_VHOSTUSER: - if (!virDomainNetIsVirtioModel(net)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Wrong or no <model> 'type' attribute specified with <interface type='vhostuser'/>. vhostuser requires the virtio-net* frontend")); - return -1; - } - if (net->data.vhostuser->data.nix.listen && net->data.vhostuser->data.nix.reconnect.enabled == VIR_TRISTATE_BOOL_YES) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 06093bc42b..243c499a33 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1823,17 +1823,18 @@ qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net, _("vDPA devices are not supported with this QEMU binary")); return -1; } + } - if (net->model != VIR_DOMAIN_NET_MODEL_VIRTIO) { + if (!virDomainNetIsVirtioModel(net)) { + if (net->type == VIR_DOMAIN_NET_TYPE_VDPA || + net->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("invalid model for interface of type '%1$s': '%2$s'"), + _("invalid model for interface of type '%1$s': '%2$s' - must be 'virtio'"), virDomainNetTypeToString(net->type), virDomainNetModelTypeToString(net->model)); return -1; } - } - - if (virDomainNetIsVirtioModel(net)) { + } else { if (net->driver.virtio.rx_queue_size) { if (!VIR_IS_POW2(net->driver.virtio.rx_queue_size)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -- 2.47.1

Since vhostuser is only used/supported by the QEMU driver, and all the rest of the vhostuser-specific validation is done in QEMU's validation, lets move the final check (to see if they've tried to enable auto-reconnect when this interface is on the server side of the vhostuser socket) to the QEMU validate. Signed-off-by: Laine Stump <laine(a)redhat.com&gt; --- src/conf/domain_validate.c | 10 +--------- src/qemu/qemu_validate.c | 8 ++++++++ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index d0e2bcaccf..577dbab0af 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -2185,15 +2185,6 @@ virDomainNetDefValidate(const virDomainNetDef *net) } switch (net->type) { - case VIR_DOMAIN_NET_TYPE_VHOSTUSER: - if (net->data.vhostuser->data.nix.listen && - net->data.vhostuser->data.nix.reconnect.enabled == VIR_TRISTATE_BOOL_YES) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("'reconnect' attribute unsupported 'server' mode for <interface type='vhostuser'>")); - return -1; - } - break; - case VIR_DOMAIN_NET_TYPE_USER: if (net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) { size_t p; @@ -2217,6 +2208,7 @@ virDomainNetDefValidate(const virDomainNetDef *net) } break; + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_VDPA: case VIR_DOMAIN_NET_TYPE_BRIDGE: diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 243c499a33..351fe38830 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1825,6 +1825,14 @@ qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net, } } + if (net->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER && + net->data.vhostuser->data.nix.listen && + net->data.vhostuser->data.nix.reconnect.enabled == VIR_TRISTATE_BOOL_YES) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'reconnect' attribute is not supported when source mode='server' for <interface type='vhostuser'>")); + return -1; + } + if (!virDomainNetIsVirtioModel(net)) { if (net->type == VIR_DOMAIN_NET_TYPE_VDPA || net->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER) { -- 2.47.1

qemuProcessPrepareDomain()'s comments say that it should be the only place to change the "live XML" of a domain (i.e. the public parts of the virDomainDef object that is shown in the domain's status XML), and that seems like a reasonable idea (although there aren't many users of it to date). qemuProcessPrepareDomainNetwork() is called by the aforementioned qemuProcessPrepareDomain() - this patch changes the "if (type == HOSTDEV)" in that function to a "switch(type)" so it's simpler to add DomainDef modifications for various other types of virDomainNetDef, and also so that anyone who adds a new interface type is forced to look at the code and decide if anything needs to be done here for the new type. Signed-off-by: Laine Stump <laine(a)redhat.com&gt; --- src/qemu/qemu_process.c | 75 ++++++++++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 28 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 910229a616..963d090963 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5887,7 +5887,6 @@ qemuProcessPrepareDomainNetwork(virDomainObj *vm) for (i = 0; i < def->nnets; i++) { virDomainNetDef *net = def->nets[i]; - virDomainNetType actualType; /* If appropriate, grab a physical device from the configured * network's pool of devices, or resolve bridge device name @@ -5900,36 +5899,56 @@ qemuProcessPrepareDomainNetwork(virDomainObj *vm) return -1; } - actualType = virDomainNetGetActualType(net); - if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV && - net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { - /* Each type='hostdev' network device must also have a - * corresponding entry in the hostdevs array. For netdevs - * that are hardcoded as type='hostdev', this is already - * done by the parser, but for those allocated from a - * network / determined at runtime, we need to do it - * separately. - */ - virDomainHostdevDef *hostdev = virDomainNetGetActualHostdev(net); - virDomainHostdevSubsysPCI *pcisrc = &hostdev->source.subsys.u.pci; - - if (virDomainHostdevFind(def, hostdev, NULL) >= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("PCI device %1$04x:%2$02x:%3$02x.%4$x allocated from network %5$s is already in use by domain %6$s"), - pcisrc->addr.domain, pcisrc->addr.bus, - pcisrc->addr.slot, pcisrc->addr.function, - net->data.network.name, def->name); - return -1; - } + switch (virDomainNetGetActualType(net)) { + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + /* Each type='hostdev' network device must also have a + * corresponding entry in the hostdevs array. For netdevs + * that are hardcoded as type='hostdev', this is already + * done by the parser, but for those allocated from a + * network / determined at runtime, we need to do it + * separately. + */ + virDomainHostdevDef *hostdev = virDomainNetGetActualHostdev(net); + virDomainHostdevSubsysPCI *pcisrc = &hostdev->source.subsys.u.pci; + + if (virDomainHostdevFind(def, hostdev, NULL) >= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("PCI device %1$04x:%2$02x:%3$02x.%4$x allocated from network %5$s is already in use by domain %6$s"), + pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function, + net->data.network.name, def->name); + return -1; + } - /* For hostdev present in qemuProcessPrepareDomain() phase this was - * done already, but this code runs after that, so we have to call - * it ourselves. */ - if (qemuDomainPrepareHostdev(hostdev, priv) < 0) - return -1; + /* For hostdev present in qemuProcessPrepareDomain() phase this was + * done already, but this code runs after that, so we have to call + * it ourselves. */ + if (qemuDomainPrepareHostdev(hostdev, priv) < 0) + return -1; - virDomainHostdevInsert(def, hostdev); + virDomainHostdevInsert(def, hostdev); + } + break; + + case VIR_DOMAIN_NET_TYPE_DIRECT: + case VIR_DOMAIN_NET_TYPE_BRIDGE: + case VIR_DOMAIN_NET_TYPE_NETWORK: + case VIR_DOMAIN_NET_TYPE_ETHERNET: + case VIR_DOMAIN_NET_TYPE_USER: + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + 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_NULL: + case VIR_DOMAIN_NET_TYPE_VDS: + case VIR_DOMAIN_NET_TYPE_LAST: + break; } + } return 0; } -- 2.47.1

<interface type='vhostuser'><backend type='passt'/> needs to run the passt command just as is done for interface type='user', but then add vhostuser bits to the qemu commandline/monitor command. There are some changes to the parsing/validation along with changes to the vhostuser codepath do do the extra stuff for passt. I tried keeping them separated into different patches, but then the unit test failed in a strange way deep down in the bowels of the commandline generation, so this patch both 1) makes the final changes to parsing/formatting and 2) adds passt stuff at appropriate places for vhostuser (as well as making a couple of things *not* happen when the passt backend is chosen). The result is that you can now have: <interface type='vhostuser'> <backend type='passt'/> ... </interface> Then as long as you also have the following as a subelement of <domain>: <memoryBacking> <access mode='shared'/> </memoryBacking> your passt interfaces will benefit from the greatly improved efficiency of a vhost-user data path, and all without requiring special privileges or capabilities *anywhere* (i.e. it works for unprivileged libvirt (qemu:///session) as well as privileged libvirt). Signed-off-by: Laine Stump <laine(a)redhat.com&gt; --- src/conf/domain_conf.c | 36 ++++++--- src/conf/domain_validate.c | 75 +++++++------------ src/conf/schemas/domaincommon.rng | 32 +++++++- src/qemu/qemu_command.c | 7 +- src/qemu/qemu_extdevice.c | 6 +- src/qemu/qemu_hotplug.c | 21 +++++- src/qemu/qemu_passt.c | 3 + src/qemu/qemu_process.c | 14 +++- src/qemu/qemu_validate.c | 7 +- ...t-user-slirp-portforward.x86_64-latest.err | 2 +- .../net-vhostuser-passt.x86_64-latest.args | 42 +++++++++++ .../net-vhostuser-passt.x86_64-latest.xml | 72 ++++++++++++++++++ tests/qemuxmlconfdata/net-vhostuser-passt.xml | 70 +++++++++++++++++ tests/qemuxmlconftest.c | 1 + 14 files changed, 315 insertions(+), 73 deletions(-) create mode 100644 tests/qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/net-vhostuser-passt.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6b382eb63f..49555efc56 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9457,9 +9457,25 @@ virDomainNetBackendParseXML(xmlNodePtr node, g_autofree char *tap = virXMLPropString(node, "tap"); g_autofree char *vhost = virXMLPropString(node, "vhost"); - /* The VIR_DOMAIN_NET_BACKEND_DEFAULT really means 'use hypervisor's - * builtin SLIRP'. It's reported in domain caps and thus we need to accept - * it. Hence VIR_XML_PROP_NONE instead of VIR_XML_PROP_NONZERO. */ + /* In the case of NET_TYPE_USER, backend type can be unspecified + * (i.e. VIR_DOMAIN_NET_BACKEND_DEFAULT) and that means 'use + * hypervisor's builtin SLIRP (or if that isn't available, use + * passt)'. Similarly, it can also be left unspecified in the case + * of NET_TYPE_VHOSTUSER, and then it means "use the traditional + * vhost-user backend (which auto-detects between connecting to a + * socket created by OVS, or connecting to a standalone socket + * used (mostly in testing) to connect the vhost-user interface of + * one guest directly to the vhost-user interface of another + * guest. + * + * If backend type is set to 'passt', then in both cases a passt + * process will be started, and libvirt will connect that to the + * guest interface (either communicating everything over the + * socket created by passt using a specific-to-passt protocol + * (interface type='user'>), or by using the socket for control + * plane messages and shared memory for data using the vhost-user + * protocol (<interface type='vhostuser'>)). + */ if (virXMLPropEnum(node, "type", virDomainNetBackendTypeFromString, VIR_XML_PROP_NONE, &def->backend.type) < 0) { return -1; @@ -24616,7 +24632,11 @@ virDomainNetDefFormat(virBuffer *buf, break; case VIR_DOMAIN_NET_TYPE_VHOSTUSER: - if (def->data.vhostuser->type == VIR_DOMAIN_CHR_TYPE_UNIX) { + if (def->data.vhostuser->type == VIR_DOMAIN_CHR_TYPE_UNIX && + def->backend.type != VIR_DOMAIN_NET_BACKEND_PASST) { + /* in the case of BACKEND_PASST, the values of all of these are either + * fixed (type, mode, reconnect), or derived from elsewhere (path) + */ virBufferAddLit(&sourceAttrBuf, " type='unix'"); virBufferEscapeString(&sourceAttrBuf, " path='%s'", def->data.vhostuser->data.nix.path); @@ -24627,7 +24647,6 @@ virDomainNetDefFormat(virBuffer *buf, virDomainChrSourceReconnectDefFormat(&sourceChildBuf, &def->data.vhostuser->data.nix.reconnect); } - } break; @@ -24689,15 +24708,14 @@ virDomainNetDefFormat(virBuffer *buf, } case VIR_DOMAIN_NET_TYPE_USER: - if (def->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) - virBufferEscapeString(&sourceAttrBuf, " dev='%s'", def->sourceDev); - break; - case VIR_DOMAIN_NET_TYPE_NULL: case VIR_DOMAIN_NET_TYPE_LAST: break; } + if (def->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) + virBufferEscapeString(&sourceAttrBuf, " dev='%s'", def->sourceDev); + if (def->hostIP.nips || def->hostIP.nroutes) { if (virDomainNetIPInfoFormat(&sourceChildBuf, &def->hostIP) < 0) return -1; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 577dbab0af..d38a77aa22 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -2163,7 +2163,8 @@ virDomainNetDefValidate(const virDomainNetDef *net) return -1; } - if (net->type != VIR_DOMAIN_NET_TYPE_USER) { + if (net->type != VIR_DOMAIN_NET_TYPE_USER && + net->type != VIR_DOMAIN_NET_TYPE_VHOSTUSER) { if (net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("The 'passt' backend can only be used with interface type='user'")); @@ -2171,59 +2172,37 @@ virDomainNetDefValidate(const virDomainNetDef *net) } } - if (net->nPortForwards > 0 && - (net->type != VIR_DOMAIN_NET_TYPE_USER || - (net->type == VIR_DOMAIN_NET_TYPE_USER && - net->backend.type != VIR_DOMAIN_NET_BACKEND_PASST))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("The <portForward> element can only be used with <interface type='user'> and its 'passt' backend")); - return -1; - } + if (net->nPortForwards > 0) { + size_t p; - if (!virNetDevBandwidthValidate(net->bandwidth)) { - return -1; - } + if ((net->type != VIR_DOMAIN_NET_TYPE_USER && + net->type != VIR_DOMAIN_NET_TYPE_VHOSTUSER) || + net->backend.type != VIR_DOMAIN_NET_BACKEND_PASST) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("The <portForward> element can only be used with the 'passt' backend of interface type='user' or type='vhostuser'")); + return -1; + } - switch (net->type) { - case VIR_DOMAIN_NET_TYPE_USER: - if (net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) { - size_t p; - - for (p = 0; p < net->nPortForwards; p++) { - size_t r; - virDomainNetPortForward *pf = net->portForwards[p]; - - for (r = 0; r < pf->nRanges; r++) { - virDomainNetPortForwardRange *range = pf->ranges[r]; - - if (!range->start - && (range->end || range->to - || range->exclude != VIR_TRISTATE_BOOL_ABSENT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("The 'range' of a 'portForward' requires 'start' attribute if 'end', 'to', or 'exclude' is specified")); - return -1; - } + for (p = 0; p < net->nPortForwards; p++) { + size_t r; + virDomainNetPortForward *pf = net->portForwards[p]; + + for (r = 0; r < pf->nRanges; r++) { + virDomainNetPortForwardRange *range = pf->ranges[r]; + + if (!range->start + && (range->end || range->to + || range->exclude != VIR_TRISTATE_BOOL_ABSENT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("The 'range' of a 'portForward' requires 'start' attribute if 'end', 'to', or 'exclude' is specified")); + return -1; } } } - break; + } - case VIR_DOMAIN_NET_TYPE_VHOSTUSER: - case VIR_DOMAIN_NET_TYPE_NETWORK: - case VIR_DOMAIN_NET_TYPE_VDPA: - case VIR_DOMAIN_NET_TYPE_BRIDGE: - case VIR_DOMAIN_NET_TYPE_CLIENT: - case VIR_DOMAIN_NET_TYPE_SERVER: - case VIR_DOMAIN_NET_TYPE_MCAST: - case VIR_DOMAIN_NET_TYPE_UDP: - case VIR_DOMAIN_NET_TYPE_INTERNAL: - case VIR_DOMAIN_NET_TYPE_DIRECT: - case VIR_DOMAIN_NET_TYPE_HOSTDEV: - case VIR_DOMAIN_NET_TYPE_VDS: - case VIR_DOMAIN_NET_TYPE_ETHERNET: - case VIR_DOMAIN_NET_TYPE_NULL: - case VIR_DOMAIN_NET_TYPE_LAST: - break; + if (!virNetDevBandwidthValidate(net->bandwidth)) { + return -1; } return 0; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index e5da550e45..3328a63205 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -3486,8 +3486,36 @@ </attribute> <interleave> <optional> - <ref name="unixSocketSource"/> - </optional> + <element name="source"> + <optional> + <attribute name="type"> + <value>unix</value> + </attribute> + </optional> + <optional> + <attribute name="path"> + <ref name="absFilePath"/> + </attribute> + </optional> + <optional> + <attribute name="mode"> + <choice> + <value>server</value> + <value>client</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name="dev"> + <ref name="deviceName"/> + </attribute> + </optional> + <optional> + <ref name="reconnect"/> + </optional> + <empty/> + </element> + </optional> <ref name="interface-options"/> </interleave> </group> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7370711918..54130ac4f0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8649,11 +8649,12 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, if (qemuInterfaceVhostuserConnect(cmd, net, qemuCaps) < 0) goto cleanup; - if (virNetDevOpenvswitchGetVhostuserIfname(net->data.vhostuser->data.nix.path, + if (net->backend.type != VIR_DOMAIN_NET_BACKEND_PASST && + virNetDevOpenvswitchGetVhostuserIfname(net->data.vhostuser->data.nix.path, net->data.vhostuser->data.nix.listen, - &net->ifname) < 0) + &net->ifname) < 0) { goto cleanup; - + } break; case VIR_DOMAIN_NET_TYPE_VDPA: diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 954cb323a4..2384bab7a6 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -212,13 +212,15 @@ qemuExtDevicesStart(virQEMUDriver *driver, for (i = 0; i < def->nnets; i++) { virDomainNetDef *net = def->nets[i]; - if (net->type != VIR_DOMAIN_NET_TYPE_USER) + if (net->type != VIR_DOMAIN_NET_TYPE_USER && + net->type != VIR_DOMAIN_NET_TYPE_VHOSTUSER) { continue; + } if (net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) { if (qemuPasstStart(vm, net) < 0) return -1; - } else { + } else if (net->type == VIR_DOMAIN_NET_TYPE_USER) { if (qemuSlirpStart(vm, net, incomingMigration) < 0) return -1; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6c224c9793..28ca321c5c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1262,10 +1262,23 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, if (!(charDevAlias = qemuAliasChardevFromDevAlias(net->info.alias))) goto cleanup; - if (virNetDevOpenvswitchGetVhostuserIfname(net->data.vhostuser->data.nix.path, - net->data.vhostuser->data.nix.listen, - &net->ifname) < 0) - goto cleanup; + if (net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) { + + /* vhostuser needs socket path in this location, and when + * backend is passt, the path is derived from other info, + * not taken from config. + */ + g_free(net->data.vhostuser->data.nix.path); + net->data.vhostuser->data.nix.path = qemuPasstCreateSocketPath(vm, net); + + if (qemuPasstStart(vm, net) < 0) + goto cleanup; + } else { + if (virNetDevOpenvswitchGetVhostuserIfname(net->data.vhostuser->data.nix.path, + net->data.vhostuser->data.nix.listen, + &net->ifname) < 0) + goto cleanup; + } if (qemuSecuritySetNetdevLabel(driver, vm, net) < 0) goto cleanup; diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c index 8a3ac4e988..b9616d1c63 100644 --- a/src/qemu/qemu_passt.c +++ b/src/qemu/qemu_passt.c @@ -180,6 +180,9 @@ qemuPasstStart(virDomainObj *vm, virCommandClearCaps(cmd); + if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_VHOSTUSER) + virCommandAddArg(cmd, "--vhost-user"); + virCommandAddArgList(cmd, "--one-off", "--socket", passtSocketName, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 963d090963..be8e32fbc3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -64,6 +64,7 @@ #include "qemu_backup.h" #include "qemu_dbus.h" #include "qemu_snapshot.h" +#include "qemu_passt.h" #include "cpu/cpu.h" #include "cpu/cpu_x86.h" @@ -5931,12 +5932,22 @@ qemuProcessPrepareDomainNetwork(virDomainObj *vm) } break; + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + if (net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) { + /* when using the passt backend, the path of the + * unix socket is always derived from other info + * *not* manually given in the config, but all the + * vhostuser code looks for it there. + */ + net->data.vhostuser->data.nix.path = qemuPasstCreateSocketPath(vm, net); + } + break; + case VIR_DOMAIN_NET_TYPE_DIRECT: case VIR_DOMAIN_NET_TYPE_BRIDGE: case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_ETHERNET: case VIR_DOMAIN_NET_TYPE_USER: - case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: @@ -5948,7 +5959,6 @@ qemuProcessPrepareDomainNetwork(virDomainObj *vm) case VIR_DOMAIN_NET_TYPE_LAST: break; } - } return 0; } diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index b0cf5e866c..92e745cea1 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1751,7 +1751,9 @@ qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net, return -1; } - if (net->type == VIR_DOMAIN_NET_TYPE_USER) { + if (net->type == VIR_DOMAIN_NET_TYPE_USER || + (net->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER && + net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST)) { virDomainCapsDeviceNet netCaps = { }; virQEMUCapsFillDomainDeviceNetCaps(qemuCaps, &netCaps); @@ -1826,7 +1828,8 @@ qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net, } if (net->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER) { - if (!net->data.vhostuser->data.nix.path) { + if (!net->data.vhostuser->data.nix.path && + net->backend.type != VIR_DOMAIN_NET_BACKEND_PASST) { virReportError(VIR_ERR_XML_ERROR, _("Missing required attribute '%1$s' in element '%2$s'"), "path", "source"); diff --git a/tests/qemuxmlconfdata/net-user-slirp-portforward.x86_64-latest.err b/tests/qemuxmlconfdata/net-user-slirp-portforward.x86_64-latest.err index eaa934742e..e231677e57 100644 --- a/tests/qemuxmlconfdata/net-user-slirp-portforward.x86_64-latest.err +++ b/tests/qemuxmlconfdata/net-user-slirp-portforward.x86_64-latest.err @@ -1 +1 @@ -unsupported configuration: The <portForward> element can only be used with <interface type='user'> and its 'passt' backend +unsupported configuration: The <portForward> element can only be used with the 'passt' backend of interface type='user' or type='vhostuser' diff --git a/tests/qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.args b/tests/qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.args new file mode 100644 index 0000000000..21d78d6072 --- /dev/null +++ b/tests/qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.args @@ -0,0 +1,42 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \ +-accel tcg \ +-cpu qemu64 \ +-m size=219136k \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-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=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","read-only":false}' \ +-device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-storage","id":"ide0-0-0","bootindex":1}' \ +-chardev socket,id=charnet0,path=/var/run/libvirt/qemu/passt/-1-QEMUGuest1-net0.socket \ +-netdev '{"type":"vhost-user","chardev":"charnet0","id":"hostnet0"}' \ +-device '{"driver":"virtio-net-pci","netdev":"hostnet0","id":"net0","mac":"00:11:22:33:44:55","bus":"pci.0","addr":"0x2"}' \ +-chardev socket,id=charnet1,path=/var/run/libvirt/qemu/passt/-1-QEMUGuest1-net1.socket \ +-netdev '{"type":"vhost-user","chardev":"charnet1","id":"hostnet1"}' \ +-device '{"driver":"virtio-net-pci","netdev":"hostnet1","id":"net1","mac":"00:11:22:33:44:11","bus":"pci.0","addr":"0x3"}' \ +-chardev socket,id=charnet2,path=/var/run/libvirt/qemu/passt/-1-QEMUGuest1-net2.socket \ +-netdev '{"type":"vhost-user","chardev":"charnet2","id":"hostnet2"}' \ +-device '{"driver":"virtio-net-pci","netdev":"hostnet2","id":"net2","mac":"00:11:22:33:44:11","bus":"pci.0","addr":"0x4"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.xml b/tests/qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.xml new file mode 100644 index 0000000000..26aa4c8d05 --- /dev/null +++ b/tests/qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.xml @@ -0,0 +1,72 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0' model='none'/> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <interface type='vhostuser'> + <mac address='00:11:22:33:44:55'/> + <ip address='172.17.2.0' family='ipv4' prefix='24'/> + <ip address='2001:db8:ac10:fd01::feed' family='ipv6'/> + <portForward proto='tcp' address='2001:db8:ac10:fd01::1:10'> + <range start='22' to='2022'/> + <range start='1000' end='1050'/> + <range start='1020' exclude='yes'/> + <range start='1030' end='1040' exclude='yes'/> + </portForward> + <portForward proto='udp' address='1.2.3.4' dev='eth0'> + <range start='5000' end='5020' to='6000'/> + <range start='5010' end='5015' exclude='yes'/> + </portForward> + <portForward proto='tcp'> + <range start='80'/> + </portForward> + <portForward proto='tcp'> + <range start='443' to='344'/> + </portForward> + <model type='virtio'/> + <backend type='passt' logFile='/var/log/loglaw.blog'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </interface> + <interface type='vhostuser'> + <mac address='00:11:22:33:44:11'/> + <model type='virtio'/> + <backend type='passt'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + <interface type='vhostuser'> + <mac address='00:11:22:33:44:11'/> + <model type='virtio'/> + <backend type='passt'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconfdata/net-vhostuser-passt.xml b/tests/qemuxmlconfdata/net-vhostuser-passt.xml new file mode 100644 index 0000000000..e44c91e541 --- /dev/null +++ b/tests/qemuxmlconfdata/net-vhostuser-passt.xml @@ -0,0 +1,70 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0' model='none'/> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <interface type='vhostuser'> + <mac address='00:11:22:33:44:55'/> + <ip address='172.17.2.0' family='ipv4' prefix='24'/> + <ip address='2001:db8:ac10:fd01::feed' family='ipv6'/> + <source dev='eth42'/> + <portForward proto='tcp' address='2001:db8:ac10:fd01::1:10'> + <range start='22' to='2022'/> + <range start='1000' end='1050'/> + <range start='1020' exclude='yes'/> + <range start='1030' end='1040' exclude='yes'/> + </portForward> + <portForward proto='udp' address='1.2.3.4' dev='eth0'> + <range start='5000' end='5020' to='6000'/> + <range start='5010' end='5015' exclude='yes'/> + </portForward> + <portForward proto='tcp'> + <range start='80'/> + </portForward> + <portForward proto='tcp'> + <range start='443' to='344'/> + </portForward> + <model type='virtio'/> + <backend type='passt' logFile='/var/log/loglaw.blog'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </interface> + <interface type='vhostuser'> + <mac address='00:11:22:33:44:11'/> + <backend type='passt'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + <interface type='vhostuser'> + <mac address='00:11:22:33:44:11'/> + <source dev='eth43'/> + <model type='virtio'/> + <backend type='passt'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 1f0068864a..34674551a4 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -1793,6 +1793,7 @@ mymain(void) DO_TEST_CAPS_LATEST("net-user-passt"); DO_TEST_CAPS_VER("net-user-passt", "7.2.0"); DO_TEST_CAPS_LATEST_PARSE_ERROR("net-user-slirp-portforward"); + DO_TEST_CAPS_LATEST("net-vhostuser-passt"); DO_TEST_CAPS_LATEST("net-virtio"); DO_TEST_CAPS_LATEST("net-virtio-device"); DO_TEST_CAPS_LATEST("net-virtio-disable-offloads"); -- 2.47.1

On 2/14/25 6:03 AM, Andrea Bolognani wrote: Keeping in mind that this is a pre-existing requirement for a pre-existing functionality that's been there since 2014... My assumption has been that whatever type='vhostuser' already does is "good enough", and at some point during testing of uncompleted code, one of the unit tests produced an error saying something about "you've selected a feature that requires shared memory but don't have shared memory turned on; your configuration may not work correctly" or something like that. Because I saw that during a failed unit test (which was later fixed) I figured that must be reported when a vhost-user config doesn't have shared memory enabled (and didn't think to actually try it), but just now I tried making a vhost-user config without shared memory enabled, and it didn't produce any error in libvirt at virsh edit time, nor did either QEMU or passt log any error (that I can see, althouygh maybe I'm not looking in all the right places), but it did produce a non-working interface for the guest! I agree with you that it doesn't "make for a good user experience" and something should be done about it. I had considered having shared memory turned on automatically for type='vhostuser', but anything about "shared memory" sets security red flags ringing (or some other mixed metaphor) so I've assumed that's why they didn't already do this for traditional vhost-user interfaces. (I did ask about this online, but sbrivio was the only one who answered (saying a variation of the above)) (admittedly it was late in the day for me, so anybody in a further east timezone was probably already offline (except sbrivio, who seems to not sleep until after it is sunrise in Australia for some strange reason ;-)). As a followup, I'd be happy to make a patch that causes a config with a vhost-user interface but no shared mem enabled to fail validation. That shouldn't hold up what's here though - as I said above, it's a preexisting condition (I don't know if you were actually suggesting that, but just want to "head it off at the pass" if you were :-)) (Now *there's* a nice American colloquialism (straight out of old western movies) - do people outside the U.S. every use it?) me too - double :-) Oops, missed that one. I'll fix it before I push anything. I was thinking that myself as I was writing this code! We really should be grabbing *all* external commands and putting them somewhere to compare (I guess they could all just go into the same file, and then the unit test would be able to verify the commandline was correct, and that the commands had all been executed in the proper order).