[libvirt PATCH 0/9] Support libvirt-managed QEMU domain <interface> backed by a passt process
passt (https://passt.top) provides a method for connecting a guest to the larger network without requiring any elevated privileges. This set of patches allows libvirt/QEMU users to easily configure a QEMU domain to use passt for the backend of any emulated network interface. More details are in the individual patches, but the short explanation is that you will use: <interface type='user'> <backend type='passt'> ... to select the passt backend. (We decided to do it this way since the concept is so similar to slirp, which was the original "type='user'") The following caveats currently apply: 1) passt support requires "-netdev stream" in QEMU, which is only available starting with qemu-7.2.0. So if you want to test these patches out, you need the latest upstream release of QEMU. 2) SELinux must be set to "permissive". This is of course temporary. As I understand it, the remedy to this is a new SELinux profile for the passt binary, which is outside the control of libvirt and so not something that can be addressed in this patchset (or any other patch to libvirt). 3) Although there is a a new option for QEMU's -netdev that will tell QEMU to attempt to reconnect to a new incarnation of the same socket if passt unexpectedly exits, and a new QEMU event that will be put into QEMU to inform libvirt that the passt process has exited (so that it can start a new, identical passt process), I think this hasn't been pushed upstream yet (??), and I haven't implemented any support for it here. So, if the passt process unexpectedly exits, the guest will be without networking. However, Stefano (passt author) is emphatic that passt will never unexpectedly exit :-) passt has *many* other options that libvirt could support, but the small subset here are the things that seem most useful (and/or were specifically requested by prospective users of passt). It is always easier to add more stuff in the future than to remove "mistakes", so I tried to no go overboard. Laine Stump (9): conf: rename virDomainNetBackend* to virDomainNetDriver* conf: move anonymous backend struct from virDomainNetDef into its own struct conf: put interface <backend> parsing/formatting separate functions conf: add passt XML additions to schema conf: parse/format passt-related XML additions qemu: new capability QEMU_CAPS_NETDEV_STREAM qemu: add passtStateDir to qemu driver config qemu: hook up passt config to qemu domains specfile: require passt for the build if fedora >= 36 or rhel >= 9 docs/formatdomain.rst | 95 +++++- libvirt.spec.in | 7 + meson.build | 1 + po/POTFILES | 1 + src/conf/domain_conf.c | 303 ++++++++++++++++-- src/conf/domain_conf.h | 64 +++- src/conf/domain_validate.c | 32 +- src/conf/schemas/domaincommon.rng | 65 ++++ src/conf/virconftypes.h | 6 + src/libvirt_private.syms | 1 + src/qemu/meson.build | 2 + src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 11 +- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_conf.c | 2 + src/qemu/qemu_conf.h | 1 + src/qemu/qemu_domain.c | 5 +- src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_driver.c | 12 + src/qemu/qemu_extdevice.c | 25 +- src/qemu/qemu_hotplug.c | 26 +- src/qemu/qemu_interface.c | 8 +- src/qemu/qemu_passt.c | 284 ++++++++++++++++ src/qemu/qemu_passt.h | 38 +++ src/qemu/qemu_process.c | 1 + src/qemu/qemu_validate.c | 9 +- src/security/virt-aa-helper.c | 2 +- .../caps_7.2.0.x86_64.xml | 1 + tests/qemuxml2argvdata/net-user-passt.args | 34 ++ .../net-user-passt.x86_64-latest.args | 37 +++ tests/qemuxml2argvdata/net-user-passt.xml | 57 ++++ tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmloutdata/net-user-passt.xml | 1 + tests/qemuxml2xmltest.c | 1 + 35 files changed, 1087 insertions(+), 60 deletions(-) create mode 100644 src/qemu/qemu_passt.c create mode 100644 src/qemu/qemu_passt.h create mode 100644 tests/qemuxml2argvdata/net-user-passt.args create mode 100644 tests/qemuxml2argvdata/net-user-passt.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/net-user-passt.xml create mode 120000 tests/qemuxml2xmloutdata/net-user-passt.xml -- 2.38.1
This fits better with the element containing the value (<driver>), and allows us to use virDomainNetBackend* for things in the <backend> element. Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/domain_conf.c | 8 ++++---- src/conf/domain_conf.h | 14 +++++++------- src/qemu/qemu_interface.c | 8 ++++---- src/security/virt-aa-helper.c | 2 +- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a180398b14..30b0cef131 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -604,8 +604,8 @@ VIR_ENUM_IMPL(virDomainNetModel, "82543GC", ); -VIR_ENUM_IMPL(virDomainNetBackend, - VIR_DOMAIN_NET_BACKEND_TYPE_LAST, +VIR_ENUM_IMPL(virDomainNetDriver, + VIR_DOMAIN_NET_DRIVER_TYPE_LAST, "default", "qemu", "vhost", @@ -8862,7 +8862,7 @@ virDomainNetDefParseXMLDriver(virDomainNetDef *def, xmlNodePtr guestNode; if (virXMLPropEnum(driver_node, "name", - virDomainNetBackendTypeFromString, + virDomainNetDriverTypeFromString, VIR_XML_PROP_NONZERO, &def->driver.virtio.name) < 0) return -1; @@ -23190,7 +23190,7 @@ virDomainVirtioNetDriverFormat(virBuffer *buf, { if (def->driver.virtio.name) { virBufferAsprintf(buf, " name='%s'", - virDomainNetBackendTypeToString(def->driver.virtio.name)); + virDomainNetDriverTypeToString(def->driver.virtio.name)); } if (def->driver.virtio.txmode) { virBufferAsprintf(buf, " txmode='%s'", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c19dfc5470..61d2ee819b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -975,12 +975,12 @@ typedef enum { /* the backend driver used for virtio interfaces */ typedef enum { - VIR_DOMAIN_NET_BACKEND_TYPE_DEFAULT = 0, /* prefer kernel, fall back to user */ - VIR_DOMAIN_NET_BACKEND_TYPE_QEMU, /* userland */ - VIR_DOMAIN_NET_BACKEND_TYPE_VHOST, /* kernel */ + VIR_DOMAIN_NET_DRIVER_TYPE_DEFAULT = 0, /* prefer kernel, fall back to user */ + VIR_DOMAIN_NET_DRIVER_TYPE_QEMU, /* userland */ + VIR_DOMAIN_NET_DRIVER_TYPE_VHOST, /* kernel */ - VIR_DOMAIN_NET_BACKEND_TYPE_LAST -} virDomainNetBackendType; + VIR_DOMAIN_NET_DRIVER_TYPE_LAST +} virDomainNetDriverType; /* the TX algorithm used for virtio interfaces */ typedef enum { @@ -1062,7 +1062,7 @@ struct _virDomainNetDef { char *modelstr; union { struct { - virDomainNetBackendType name; /* which driver backend to use */ + virDomainNetDriverType name; /* which driver backend to use */ virDomainNetVirtioTxModeType txmode; virTristateSwitch ioeventfd; virTristateSwitch event_idx; @@ -4020,7 +4020,7 @@ VIR_ENUM_DECL(virDomainFSModel); VIR_ENUM_DECL(virDomainFSCacheMode); VIR_ENUM_DECL(virDomainFSSandboxMode); VIR_ENUM_DECL(virDomainNet); -VIR_ENUM_DECL(virDomainNetBackend); +VIR_ENUM_DECL(virDomainNetDriver); VIR_ENUM_DECL(virDomainNetVirtioTxMode); VIR_ENUM_DECL(virDomainNetMacType); VIR_ENUM_DECL(virDomainNetTeaming); diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c index 264d5e060c..b6895cedde 100644 --- a/src/qemu/qemu_interface.c +++ b/src/qemu/qemu_interface.c @@ -719,14 +719,14 @@ qemuInterfaceOpenVhostNet(virDomainObj *vm, /* If running a plain QEMU guest, or * if the config says explicitly to not use vhost, return now */ if (vm->def->virtType != VIR_DOMAIN_VIRT_KVM || - net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) + net->driver.virtio.name == VIR_DOMAIN_NET_DRIVER_TYPE_QEMU) return 0; /* If qemu doesn't support vhost-net mode (including the -netdev and * -device command options), don't try to open the device. */ if (!qemuDomainSupportsNicdev(vm->def, net)) { - if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) { + if (net->driver.virtio.name == VIR_DOMAIN_NET_DRIVER_TYPE_VHOST) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("vhost-net is not supported with this QEMU binary")); return -1; @@ -736,7 +736,7 @@ qemuInterfaceOpenVhostNet(virDomainObj *vm, /* If the nic model isn't virtio, don't try to open. */ if (!virDomainNetIsVirtioModel(net)) { - if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) { + if (net->driver.virtio.name == VIR_DOMAIN_NET_DRIVER_TYPE_VHOST) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("vhost-net is only supported for virtio network interfaces")); return -1; @@ -753,7 +753,7 @@ qemuInterfaceOpenVhostNet(virDomainObj *vm, */ if (fd < 0) { virDomainAuditNetDevice(vm->def, net, vhostnet_path, false); - if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) { + if (net->driver.virtio.name == VIR_DOMAIN_NET_DRIVER_TYPE_VHOST) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("vhost-net was requested for an interface, but is unavailable")); return -1; diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 53a1cd1048..62a20ba0e8 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1282,7 +1282,7 @@ get_files(vahControl * ctl) for (i = 0; i < ctl->def->nnets; i++) { virDomainNetDef *net = ctl->def->nets[i]; if (net && virDomainNetGetModelString(net)) { - if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) + if (net->driver.virtio.name == VIR_DOMAIN_NET_DRIVER_TYPE_QEMU) continue; if (!virDomainNetIsVirtioModel(net)) continue; -- 2.38.1
On a Sunday in 2023, Laine Stump wrote:
This fits better with the element containing the value (<driver>), and allows us to use virDomainNetBackend* for things in the <backend> element.
Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/domain_conf.c | 8 ++++---- src/conf/domain_conf.h | 14 +++++++------- src/qemu/qemu_interface.c | 8 ++++---- src/security/virt-aa-helper.c | 2 +- 4 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c19dfc5470..61d2ee819b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -975,12 +975,12 @@ typedef enum {
/* the backend driver used for virtio interfaces */ typedef enum { - VIR_DOMAIN_NET_BACKEND_TYPE_DEFAULT = 0, /* prefer kernel, fall back to user */ - VIR_DOMAIN_NET_BACKEND_TYPE_QEMU, /* userland */ - VIR_DOMAIN_NET_BACKEND_TYPE_VHOST, /* kernel */ + VIR_DOMAIN_NET_DRIVER_TYPE_DEFAULT = 0, /* prefer kernel, fall back to user */ + VIR_DOMAIN_NET_DRIVER_TYPE_QEMU, /* userland */ + VIR_DOMAIN_NET_DRIVER_TYPE_VHOST, /* kernel */
- VIR_DOMAIN_NET_BACKEND_TYPE_LAST -} virDomainNetBackendType; + VIR_DOMAIN_NET_DRIVER_TYPE_LAST +} virDomainNetDriverType;
/* the TX algorithm used for virtio interfaces */ typedef enum { @@ -1062,7 +1062,7 @@ struct _virDomainNetDef { char *modelstr; union { struct { - virDomainNetBackendType name; /* which driver backend to use */ + virDomainNetDriverType name; /* which driver backend to use */
I recommend geting rid of the mention of 'backend' by dropping the useless comment.
virDomainNetVirtioTxModeType txmode; virTristateSwitch ioeventfd; virTristateSwitch event_idx;
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
This will allow us to call parser/formatter functions with a pointer to just the backend part. Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/domain_conf.h | 10 ++++++---- src/conf/virconftypes.h | 2 ++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 61d2ee819b..e57e70866a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1051,6 +1051,11 @@ struct _virDomainActualNetDef { unsigned int class_id; /* class ID for bandwidth 'floor' */ }; +struct _virDomainNetBackend { + char *tap; + char *vhost; +}; + /* Stores the virtual network interface configuration */ struct _virDomainNetDef { virDomainNetType type; @@ -1089,10 +1094,7 @@ struct _virDomainNetDef { virTristateSwitch rss_hash_report; } virtio; } driver; - struct { - char *tap; - char *vhost; - } backend; + virDomainNetBackend backend; virDomainNetTeamingInfo *teaming; union { virDomainChrSourceDef *vhostuser; diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index 154805091a..7bd9aa8e0a 100644 --- a/src/conf/virconftypes.h +++ b/src/conf/virconftypes.h @@ -172,6 +172,8 @@ typedef struct _virDomainMomentObjList virDomainMomentObjList; typedef struct _virDomainNVRAMDef virDomainNVRAMDef; +typedef struct _virDomainNetBackend virDomainNetBackend; + typedef struct _virDomainNetDef virDomainNetDef; typedef struct _virDomainNetTeamingInfo virDomainNetTeamingInfo; -- 2.38.1
On a Sunday in 2023, Laine Stump wrote:
This will allow us to call parser/formatter functions with a pointer to just the backend part.
Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/domain_conf.h | 10 ++++++---- src/conf/virconftypes.h | 2 ++ 2 files changed, 8 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
In preparation for adding more stuff to <backend>. Signed-off-by: Laine Stump <laine@redhat.com> --- I wanted virDomainNetBackendParseXML to simply take a virDomainNetBackend*, but there is a test case specifically checking to be sure that backend/vhost isn't parsed if the interface isn't virtio. Silently Ignoring+stripping this during parse is arguably the wrong thing to do - either we should log an error on validation, or we should just leave it in (it's only ever used if the interface is virtio), but that's a problem for another day. (Opinions on the proper thing to do are welcome - since it's currently always stripped out on parse, I *think* I could begin checking for it during validation - there is no way that old code could leave the backend/vhost for a non-virtio interface in any domain xml written to disk. Alternately would could just allow it to be parsed and faithfully format it even when the interface isn't virtio, and not log any error.) src/conf/domain_conf.c | 57 +++++++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 30b0cef131..9502f2ebab 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8970,6 +8970,26 @@ virDomainNetDefParseXMLDriver(virDomainNetDef *def, } +static int +virDomainNetBackendParseXML(xmlNodePtr node, + virDomainNetDef *def) +{ + g_autofree char *tap = virXMLPropString(node, "tap"); + g_autofree char *vhost = virXMLPropString(node, "vhost"); + + if (tap) + def->backend.tap = virFileSanitizePath(tap); + + if (vhost && + def->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && + virDomainNetIsVirtioModel(def)) { + def->backend.vhost = virFileSanitizePath(vhost); + } + + return 0; +} + + static int virDomainNetDefParseXMLRequireSource(virDomainNetDef *def, xmlNodePtr source_node) @@ -9016,6 +9036,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr mac_node = NULL; xmlNodePtr target_node = NULL; xmlNodePtr coalesce_node = NULL; + xmlNodePtr backend_node = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) int rv; g_autofree char *macaddr = NULL; @@ -9319,9 +9340,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, (virNetDevVlanParse(vlan_node, ctxt, &def->vlan) < 0)) return NULL; - if ((tap = virXPathString("string(./backend/@tap)", ctxt))) - def->backend.tap = virFileSanitizePath(tap); - if ((mac_node = virXPathNode("./mac", ctxt))) { if ((macaddr = virXMLPropString(mac_node, "address"))) { if (virMacAddrParse((const char *)macaddr, &def->mac) < 0) { @@ -9376,12 +9394,9 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, if (virDomainNetDefParseXMLDriver(def, ctxt) < 0) return NULL; - if (def->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && - virDomainNetIsVirtioModel(def)) { - g_autofree char *vhost = virXPathString("string(./backend/@vhost)", ctxt); - - if (vhost) - def->backend.vhost = virFileSanitizePath(vhost); + if ((backend_node = virXPathNode("./backend", ctxt)) && + virDomainNetBackendParseXML(backend_node, def) < 0) { + return NULL; } def->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT; @@ -23255,6 +23270,21 @@ virDomainNetTeamingInfoFormat(virDomainNetTeamingInfo *teaming, } +static void +virDomainNetBackendFormat(virBuffer *buf, + virDomainNetBackend *backend) +{ + + if (!(backend->tap || backend->vhost)) + return; + + virBufferAddLit(buf, "<backend"); + virBufferEscapeString(buf, " tap='%s'", backend->tap); + virBufferEscapeString(buf, " vhost='%s'", backend->vhost); + virBufferAddLit(buf, "/>\n"); +} + + int virDomainNetDefFormat(virBuffer *buf, virDomainNetDef *def, @@ -23555,12 +23585,9 @@ virDomainNetDefFormat(virBuffer *buf, virXMLFormatElement(buf, "driver", &driverAttrBuf, &driverChildBuf); } } - if (def->backend.tap || def->backend.vhost) { - virBufferAddLit(buf, "<backend"); - virBufferEscapeString(buf, " tap='%s'", def->backend.tap); - virBufferEscapeString(buf, " vhost='%s'", def->backend.vhost); - virBufferAddLit(buf, "/>\n"); - } + + virDomainNetBackendFormat(buf, &def->backend); + if (def->filter) { if (virNWFilterFormatParamAttributes(buf, def->filterparams, def->filter) < 0) -- 2.38.1
On a Sunday in 2023, Laine Stump wrote:
In preparation for adding more stuff to <backend>.
Signed-off-by: Laine Stump <laine@redhat.com> ---
I wanted virDomainNetBackendParseXML to simply take a virDomainNetBackend*, but there is a test case specifically checking to be sure that backend/vhost isn't parsed if the interface isn't virtio. Silently Ignoring+stripping this during parse is arguably the wrong thing to do - either we should log an error on validation, or we should just leave it in (it's only ever used if the interface is virtio), but that's a problem for another day.
(Opinions on the proper thing to do are welcome - since it's currently always stripped out on parse, I *think* I could begin checking for it during validation - there is no way that old code could leave the backend/vhost for a non-virtio interface in any domain xml written to disk.
This seems like the right thing to do.
Alternately would could just allow it to be parsed and faithfully format it even when the interface isn't virtio, and not log any error.)
src/conf/domain_conf.c | 57 +++++++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 30b0cef131..9502f2ebab 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8970,6 +8970,26 @@ virDomainNetDefParseXMLDriver(virDomainNetDef *def, }
+static int +virDomainNetBackendParseXML(xmlNodePtr node, + virDomainNetDef *def) +{ + g_autofree char *tap = virXMLPropString(node, "tap"); + g_autofree char *vhost = virXMLPropString(node, "vhost"); + + if (tap) + def->backend.tap = virFileSanitizePath(tap); + + if (vhost && + def->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && + virDomainNetIsVirtioModel(def)) { + def->backend.vhost = virFileSanitizePath(vhost); + } + + return 0; +} + + static int virDomainNetDefParseXMLRequireSource(virDomainNetDef *def, xmlNodePtr source_node) @@ -9016,6 +9036,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr mac_node = NULL; xmlNodePtr target_node = NULL; xmlNodePtr coalesce_node = NULL; + xmlNodePtr backend_node = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) int rv; g_autofree char *macaddr = NULL;
src/conf/domain_conf.c:9220:22: error: unused variable 'tap' [-Werror,-Wunused-variable] g_autofree char *tap = NULL; ^ 1 error generated.
@@ -9319,9 +9340,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, (virNetDevVlanParse(vlan_node, ctxt, &def->vlan) < 0)) return NULL;
- if ((tap = virXPathString("string(./backend/@tap)", ctxt))) - def->backend.tap = virFileSanitizePath(tap); - if ((mac_node = virXPathNode("./mac", ctxt))) { if ((macaddr = virXMLPropString(mac_node, "address"))) { if (virMacAddrParse((const char *)macaddr, &def->mac) < 0) {
With the unused variable removed: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On 1/9/23 1:47 AM, Ján Tomko wrote:
On a Sunday in 2023, Laine Stump wrote:
In preparation for adding more stuff to <backend>.
Signed-off-by: Laine Stump <laine@redhat.com> ---
I wanted virDomainNetBackendParseXML to simply take a virDomainNetBackend*, but there is a test case specifically checking to be sure that backend/vhost isn't parsed if the interface isn't virtio. Silently Ignoring+stripping this during parse is arguably the wrong thing to do - either we should log an error on validation, or we should just leave it in (it's only ever used if the interface is virtio), but that's a problem for another day.
(Opinions on the proper thing to do are welcome - since it's currently always stripped out on parse, I *think* I could begin checking for it during validation - there is no way that old code could leave the backend/vhost for a non-virtio interface in any domain xml written to disk.
This seems like the right thing to do.
Alternately would could just allow it to be parsed and faithfully format it even when the interface isn't virtio, and not log any error.)
src/conf/domain_conf.c | 57 +++++++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 30b0cef131..9502f2ebab 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8970,6 +8970,26 @@ virDomainNetDefParseXMLDriver(virDomainNetDef *def, }
+static int +virDomainNetBackendParseXML(xmlNodePtr node, + virDomainNetDef *def) +{ + g_autofree char *tap = virXMLPropString(node, "tap"); + g_autofree char *vhost = virXMLPropString(node, "vhost"); + + if (tap) + def->backend.tap = virFileSanitizePath(tap); + + if (vhost && + def->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && + virDomainNetIsVirtioModel(def)) { + def->backend.vhost = virFileSanitizePath(vhost); + } + + return 0; +} + + static int virDomainNetDefParseXMLRequireSource(virDomainNetDef *def, xmlNodePtr source_node) @@ -9016,6 +9036,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr mac_node = NULL; xmlNodePtr target_node = NULL; xmlNodePtr coalesce_node = NULL; + xmlNodePtr backend_node = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) int rv; g_autofree char *macaddr = NULL;
src/conf/domain_conf.c:9220:22: error: unused variable 'tap' [-Werror,-Wunused-variable] g_autofree char *tap = NULL; ^ 1 error generated.
Yeah, I just found that when I looked at the results of gitlab CI (it isn't found by gcc in Fedora 37 :-/). I fixed that as well as eliminating a leak in the new function virDomainNetPortForwardFree() (I had forgotten to free the pf->range after freeing all the ranges that it pointed to).
@@ -9319,9 +9340,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, (virNetDevVlanParse(vlan_node, ctxt, &def->vlan) < 0)) return NULL;
- if ((tap = virXPathString("string(./backend/@tap)", ctxt))) - def->backend.tap = virFileSanitizePath(tap); - if ((mac_node = virXPathNode("./mac", ctxt))) { if ((macaddr = virXMLPropString(mac_node, "address"))) { if (virMacAddrParse((const char *)macaddr, &def->mac) < 0) {
With the unused variable removed:
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
Initial support for network devices using passt (https://passt.top) for the backend connection will require: * new attributes of the <backend> subelement: * "type" that can have the value "passt" (to differentiate from slirp, because both slirp and passt will use <interface type='user'>) * "logFile" (a path to a file that passt should use for its logging) * "upstream" (a netdev name, e.g. "eth0"). * a new subelement <portForward> (described in more detail later) Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/schemas/domaincommon.rng | 65 +++++++++++++++++++++++ tests/qemuxml2argvdata/net-user-passt.xml | 57 ++++++++++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 tests/qemuxml2argvdata/net-user-passt.xml diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 8bc627d114..0e66b84576 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -3581,6 +3581,7 @@ </element> </optional> <ref name="interface-ip-info"/> + <ref name="interface-port-forwards"/> <optional> <element name="script"> <attribute name="path"> @@ -3617,6 +3618,13 @@ </optional> <optional> <element name="backend"> + <optional> + <attribute name="type"> + <choice> + <value>passt</value> + </choice> + </attribute> + </optional> <optional> <attribute name="tap"> <ref name="absFilePath"/> @@ -3627,6 +3635,16 @@ <ref name="absFilePath"/> </attribute> </optional> + <optional> + <attribute name="logFile"> + <ref name="absFilePath"/> + </attribute> + </optional> + <optional> + <attribute name="upstream"> + <ref name="deviceName"/> + </attribute> + </optional> </element> </optional> <optional> @@ -3843,6 +3861,53 @@ </interleave> </define> + <define name="interface-port-forwards"> + <zeroOrMore> + <element name="portForward"> + <attribute name="proto"> + <choice> + <value>tcp</value> + <value>udp</value> + </choice> + </attribute> + <optional> + <attribute name="address"> + <ref name="ipAddr"/> + </attribute> + </optional> + <optional> + <attribute name="dev"> + <ref name="deviceName"/> + </attribute> + </optional> + <interleave> + <zeroOrMore> + <element name="range"> + <attribute name="start"> + <ref name="PortNumber"/> + </attribute> + <optional> + <attribute name="end"> + <ref name="PortNumber"/> + </attribute> + </optional> + <optional> + <attribute name="to"> + <ref name="PortNumber"/> + </attribute> + </optional> + <optional> + <attribute name="exclude"> + <ref name="virYesNo"/> + </attribute> + </optional> + </element> + </zeroOrMore> + </interleave> + </element> + </zeroOrMore> + </define> + <define name="teaming"> <element name="teaming"> <choice> diff --git a/tests/qemuxml2argvdata/net-user-passt.xml b/tests/qemuxml2argvdata/net-user-passt.xml new file mode 100644 index 0000000000..b82eebd089 --- /dev/null +++ b/tests/qemuxml2argvdata/net-user-passt.xml @@ -0,0 +1,57 @@ +<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='user'> + <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='rtl8139'/> + <backend type='passt' logFile='/var/log/loglaw.blog' upstream='eth42'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='none'/> + </devices> +</domain> -- 2.38.1
On a Sunday in 2023, Laine Stump wrote:
Initial support for network devices using passt (https://passt.top) for the backend connection will require:
* new attributes of the <backend> subelement: * "type" that can have the value "passt" (to differentiate from slirp, because both slirp and passt will use <interface type='user'>) * "logFile" (a path to a file that passt should use for its logging) * "upstream" (a netdev name, e.g. "eth0").
* a new subelement <portForward> (described in more detail later)
Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/schemas/domaincommon.rng | 65 +++++++++++++++++++++++ tests/qemuxml2argvdata/net-user-passt.xml | 57 ++++++++++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 tests/qemuxml2argvdata/net-user-passt.xml
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On Sun, Jan 08, 2023 at 11:11:07PM -0500, Laine Stump wrote:
Initial support for network devices using passt (https://passt.top) for the backend connection will require:
* new attributes of the <backend> subelement: * "type" that can have the value "passt" (to differentiate from slirp, because both slirp and passt will use <interface type='user'>) * "logFile" (a path to a file that passt should use for its logging) * "upstream" (a netdev name, e.g. "eth0").
IMHO this attribute is inappropriate for <backend>....
* a new subelement <portForward> (described in more detail later)
Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/schemas/domaincommon.rng | 65 +++++++++++++++++++++++ tests/qemuxml2argvdata/net-user-passt.xml | 57 ++++++++++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 tests/qemuxml2argvdata/net-user-passt.xml
diff --git a/tests/qemuxml2argvdata/net-user-passt.xml b/tests/qemuxml2argvdata/net-user-passt.xml new file mode 100644 index 0000000000..b82eebd089 --- /dev/null +++ b/tests/qemuxml2argvdata/net-user-passt.xml @@ -0,0 +1,57 @@ +<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='user'> + <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='rtl8139'/> + <backend type='passt' logFile='/var/log/loglaw.blog' upstream='eth42'/>
I don't think that 'upstream' is really describing a property of the backend. This is expressing a traffic routing restriction for the 'user' networking type. IMHO it should probably be using the existing <source dev="xxxx"/> element, that is currently used by the 'direct' networking type. Can we see about fixing this before release.
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='none'/> + </devices> +</domain>
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 1/11/23 1:33 PM, Daniel P. Berrangé wrote:
On Sun, Jan 08, 2023 at 11:11:07PM -0500, Laine Stump wrote:
Initial support for network devices using passt (https://passt.top) for the backend connection will require:
* new attributes of the <backend> subelement: * "type" that can have the value "passt" (to differentiate from slirp, because both slirp and passt will use <interface type='user'>) * "logFile" (a path to a file that passt should use for its logging) * "upstream" (a netdev name, e.g. "eth0").
IMHO this attribute is inappropriate for <backend>....
[...]
+ <interface type='user'> + <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='rtl8139'/> + <backend type='passt' logFile='/var/log/loglaw.blog' upstream='eth42'/>
I don't think that 'upstream' is really describing a property of the backend.
This is expressing a traffic routing restriction for the 'user' networking type. IMHO it should probably be using the existing <source dev="xxxx"/> element, that is currently used by the 'direct' networking type.
I'm still not sure *exactly* what it does; it apparently grabs the routes that are fed to the guest from the given host interface; I should probably ask Stefano to explain it to me again (he described it once, but that was along with explanations of several other things). So it's not *exactly* the same as <source dev='xxx'/> for type='direct' (which determines the link-level connection rather than IP routing), but definitely very similar.
Can we see about fixing this before release.
Yes, that makes sense. I'm not sure why I didn't think of it (usually I try *too* hard to re-use existing XML). I'll make a patch and send it later today.
On Thu, 12 Jan 2023 09:45:39 -0500 Laine Stump <laine@redhat.com> wrote:
On 1/11/23 1:33 PM, Daniel P. Berrangé wrote:
On Sun, Jan 08, 2023 at 11:11:07PM -0500, Laine Stump wrote:
Initial support for network devices using passt (https://passt.top) for the backend connection will require:
* new attributes of the <backend> subelement: * "type" that can have the value "passt" (to differentiate from slirp, because both slirp and passt will use <interface type='user'>) * "logFile" (a path to a file that passt should use for its logging) * "upstream" (a netdev name, e.g. "eth0").
IMHO this attribute is inappropriate for <backend>....
[...]
+ <interface type='user'> + <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='rtl8139'/> + <backend type='passt' logFile='/var/log/loglaw.blog' upstream='eth42'/>
I don't think that 'upstream' is really describing a property of the backend.
This is expressing a traffic routing restriction for the 'user' networking type. IMHO it should probably be using the existing <source dev="xxxx"/> element, that is currently used by the 'direct' networking type.
I'm still not sure *exactly* what it does; it apparently grabs the routes that are fed to the guest from the given host interface; I should probably ask Stefano to explain it to me again (he described it once, but that was along with explanations of several other things).
Yes, it's pretty much that... recycling from the man page: -i, --interface name Use host interface name to derive addresses and routes. Default is to use the interfaces with the first default routes for each IP version. It's not actually a routing restriction -- passt can't do that. The only interface binding that passt implements (with Linux kernel versions >= 5.7) is an optional bound interface specification for port forwarding.
So it's not *exactly* the same as <source dev='xxx'/> for type='direct' (which determines the link-level connection rather than IP routing), but definitely very similar.
Right, I think so too, and "source" is probably a good name for that in any case. -- Stefano
On Thu, Jan 12, 2023 at 09:45:39 -0500, Laine Stump wrote:
On 1/11/23 1:33 PM, Daniel P. Berrangé wrote:
On Sun, Jan 08, 2023 at 11:11:07PM -0500, Laine Stump wrote:
+ <backend type='passt' logFile='/var/log/loglaw.blog' upstream='eth42'/>
I don't think that 'upstream' is really describing a property of the backend.
This is expressing a traffic routing restriction for the 'user' networking type. IMHO it should probably be using the existing <source dev="xxxx"/> element, that is currently used by the 'direct' networking type.
I'm still not sure *exactly* what it does; it apparently grabs the routes that are fed to the guest from the given host interface; I should probably ask Stefano to explain it to me again (he described it once, but that was along with explanations of several other things).
So it's not *exactly* the same as <source dev='xxx'/> for type='direct' (which determines the link-level connection rather than IP routing), but definitely very similar.
Can we see about fixing this before release.
Yes, that makes sense. I'm not sure why I didn't think of it (usually I try *too* hard to re-use existing XML).
I'll make a patch and send it later today.
Great, I'm waiting with tagging rc2 until this is done. Jirka
This implements XML config to represent a subset of the features supported by 'passt' (https://passt.top), which is an alternative backend for emulated network devices that requires no elevated privileges (similar to slirp, but "better"). Along with setting the backend to use passt (via <backend type='passt'/> when the interface type='user'), we also support passt's --log-file and --interface options (via the <backend> subelement logFile and upstream attributes) and its --tcp-ports and --udp-ports options (which selectively forward incoming connections to the host on to the guest) via the new <portForward> subelement of <interface>. Here is an example of the config for a network interface that uses passt to connect: <interface type='user'> <mac address='52:54:00:a8:33:fc'/> <ip address='192.168.221.122' family='ipv4'/> <model type='virtio'/> <backend type='passt' logFile='/tmp/xyzzy.log' upstream='eth0'/> <portForward address='10.0.0.1' proto='tcp' dev='eth0'> <range start='2022' to='22'/> <range start='5000' end='5099' to='1000'/> <range start='5010' end='5029' exclude='yes'/> </portForward> <portForward proto='udp'> <range start='10101'/> </portForward> </interface> In this case: * the guest will be offered address 192.168.221.122 for its interface via DHCP * the passt process will write all log messages to /tmp/xyzzy.log * routes to the outside for the guest will be derived from the addresses and routes associated with the host interface "eth0". * incoming tcp port 2022 to the host will be forwarded to port 22 on the guest. * incoming tcp ports 5000-5099 (with the exception of ports 5010-5029) to the host will be forwarded to port 1000-1099 on the guest. * incoming udp packets on port 10101 will be forwarded (unchanged) to the guest. Signed-off-by: Laine Stump <laine@redhat.com> --- docs/formatdomain.rst | 95 +++++++- src/conf/domain_conf.c | 242 +++++++++++++++++++- src/conf/domain_conf.h | 40 ++++ src/conf/domain_validate.c | 32 ++- src/conf/virconftypes.h | 4 + src/libvirt_private.syms | 1 + tests/qemuxml2xmloutdata/net-user-passt.xml | 1 + tests/qemuxml2xmltest.c | 1 + 8 files changed, 401 insertions(+), 15 deletions(-) create mode 120000 tests/qemuxml2xmloutdata/net-user-passt.xml diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index d7fffc6e0b..cd8fd4483b 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -4767,19 +4767,25 @@ to the interface. </devices> ... -Userspace SLIRP stack -^^^^^^^^^^^^^^^^^^^^^ +Userspace (SLIRP or passt) connection +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The ``user`` type connects the guest interface to the outside via a +transparent userspace proxy that doesn't require any special system +privileges, making it usable in cases when libvirt itself is running +with no privileges (e.g. libvirt's "session mode" daemon, or when +libvirt is run inside an unprivileged container). -Provides a virtual LAN with NAT to the outside world. The virtual network has -DHCP & DNS services and will give the guest VM addresses starting from -``10.0.2.15``. The default router will be ``10.0.2.2`` and the DNS server will -be ``10.0.2.3``. This networking is the only option for unprivileged users who -need their VMs to have outgoing access. :since:`Since 3.8.0` it is possible to -override the default network address by including an ``ip`` element specifying -an IPv4 address in its one mandatory attribute, ``address``. Optionally, a -second ``ip`` element with a ``family`` attribute set to "ipv6" can be specified -to add an IPv6 address to the interface. ``address``. Optionally, address -``prefix`` can be specified. +By default, this user proxy is done with QEMU's internal SLIRP driver +which has DHCP & DNS services that give the guest IP addresses +starting from ``10.0.2.15``, a default route of ``10.0.2.2`` and DNS +server of ``10.0.2.3``. :since:`Since 3.8.0` it is possible to override +the default network address by including an ``ip`` element specifying +an IPv4 address in its one mandatory attribute, +``address``. Optionally, a second ``ip`` element with a ``family`` +attribute set to "ipv6" can be specified to add an IPv6 address to the +interface. ``address``. Optionally, address ``prefix`` can be +specified. :: @@ -4795,6 +4801,71 @@ to add an IPv6 address to the interface. ``address``. Optionally, address </devices> ... +:since:`Since 9.0.0` an alternate backend implementation of the +``user`` interface type can be selected by setting the interface's +``<backend>`` subelement ``type`` attribute to ``passt``. In this +case, the passt transport (https://passt.top) is used. Similar to +SLIRP, passt has an internal DHCP server that provides a requesting +guest with one ipv4 and one ipv6 address; it then uses userspace +proxies and a separate network namespace to provide outgoing +UDP/TCP/ICMP sessions, and optionally redirect incoming traffic +destined for the host toward the guest instead. + +When the passt backend is used, the ``<backend>`` attribute +``logFile`` can be used to tell the passt process for this interface +where to write its message log, and the ``<backend>`` attribute +``upstream`` can tell it to restrict upstream traffic to a particular +host interface. + +Additionally, when passt is used, multiple ``<portForward>`` elements +can be added to forward incoming network traffic for the host to this +guest interface. Each ``<portForward>`` must have a ``proto`` +attribute (set to ``tcp`` or ``udp``) and optional original +``address`` (if not specified, then all incoming sessions to any host +IP for the given proto/port(s) will be forwarded to the guest). + +The decision of which ports to forward is described with zero or more +``<range>`` subelements of ``<portForward>`` (if there is no +``<range>`` then **all** ports for the given proto/address will be +forwarded). Each ``<range>`` has a ``start`` and optional ``end`` +attribute. If ``end`` is omitted then a single port will be forwarded, +otherwise all ports between ``start`` and ``end`` (inclusive) will be +forwarded. If the port number(s) should remain unmodified as the +session is forwarded, no further options are needed, but if the guest +is expecting the sessions on a different port, then this should be +specified with the ``to`` attribute of ``<range>`` - the port number +of each forwarded session in the range will be offeset by "``to`` - +``start``". A ``<range>`` element can also be used to specify a range +of ports that should **not** be forwarded. This is done by setting the +range's ``exclude`` attribute to ``yes``. This may not seem very +useful, but can be when it is desirable to forward a long range of +ports **with the exception of some subset**. + +:: + + ... + <devices> + ... + <interface type='user'> + <backend type='passt' logFile='/var/log/passt.log' upstream='eth0'/> + <mac address="00:11:22:33:44:55"/> + <ip family='ipv4' address='172.17.2.0' prefix='24'/> + <ip family='ipv6' address='2001:db8:ac10:fd01::' prefix='64'/> + <portForward proto='tcp' address='2001:db8:ac10:fd01::1:10' start='2022'> + <port start='22'/> + </portForward> + <portForward proto='udp' address='1.2.3.4' start='5000' end='5020'> + <port start='6000' end='6020'/> + </portForward> + <portForward exclude='yes' proto='tcp' address='1.2.3.4' start='5010' end='5015'/> + <portForward proto='tcp' start='80'/> + <portForward proto='tcp' start='443'> + <port start='344'/> + </portForward> + </interface> + </devices> + ... + Generic ethernet connection ^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9502f2ebab..19252b28c5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -632,6 +632,19 @@ VIR_ENUM_IMPL(virDomainNetInterfaceLinkState, "down", ); +VIR_ENUM_IMPL(virDomainNetBackend, + VIR_DOMAIN_NET_BACKEND_LAST, + "default", + "passt", +); + +VIR_ENUM_IMPL(virDomainNetProto, + VIR_DOMAIN_NET_PROTO_LAST, + "none", + "tcp", + "udp", +); + VIR_ENUM_IMPL(virDomainChrDeviceState, VIR_DOMAIN_CHR_DEVICE_STATE_LAST, "default", @@ -2602,10 +2615,25 @@ virDomainNetTeamingInfoFree(virDomainNetTeamingInfo *teaming) g_free(teaming); } +void +virDomainNetPortForwardFree(virDomainNetPortForward *pf) +{ + size_t i; + + if (pf) + g_free(pf->dev); + + for (i = 0; i < pf->nRanges; i++) + g_free(pf->ranges[i]); + + g_free(pf); +} void virDomainNetDefFree(virDomainNetDef *def) { + size_t i; + if (!def) return; @@ -2663,6 +2691,8 @@ virDomainNetDefFree(virDomainNetDef *def) g_free(def->backend.tap); g_free(def->backend.vhost); + g_free(def->backend.logFile); + g_free(def->backend.upstream); virDomainNetTeamingInfoFree(def->teaming); g_free(def->virtPortProfile); g_free(def->script); @@ -2684,6 +2714,10 @@ virDomainNetDefFree(virDomainNetDef *def) virNetDevBandwidthFree(def->bandwidth); virNetDevVlanClear(&def->vlan); + for (i = 0; i < def->nPortForwards; i++) + virDomainNetPortForwardFree(def->portForwards[i]); + g_free(def->portForwards); + virObjectUnref(def->privateData); g_free(def); } @@ -8977,6 +9011,14 @@ virDomainNetBackendParseXML(xmlNodePtr node, g_autofree char *tap = virXMLPropString(node, "tap"); g_autofree char *vhost = virXMLPropString(node, "vhost"); + if (virXMLPropEnum(node, "type", virDomainNetBackendTypeFromString, + VIR_XML_PROP_NONZERO, &def->backend.type) < 0) { + return -1; + } + + def->backend.logFile = virXMLPropString(node, "logFile"); + def->backend.upstream = virXMLPropString(node, "upstream"); + if (tap) def->backend.tap = virFileSanitizePath(tap); @@ -8990,6 +9032,122 @@ virDomainNetBackendParseXML(xmlNodePtr node, } +static virDomainNetPortForwardRange * +virDomainNetPortForwardRangeParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt) +{ + VIR_XPATH_NODE_AUTORESTORE(ctxt) + g_autofree virDomainNetPortForwardRange *def = g_new0(virDomainNetPortForwardRange, 1); + + ctxt->node = node; + + if (virXMLPropUInt(node, "start", 10, + VIR_XML_PROP_NONZERO, &def->start) < 0) { + return NULL; + } + if (virXMLPropUInt(node, "end", 10, + VIR_XML_PROP_NONZERO, &def->end) < 0) { + return NULL; + } + if (virXMLPropUInt(node, "to", 10, + VIR_XML_PROP_NONZERO, &def->to) < 0) { + return NULL; + } + if (virXMLPropTristateBool(node, "exclude", VIR_XML_PROP_NONE, + &def->exclude) < 0) { + return NULL; + } + + return g_steal_pointer(&def); +} + + +static int +virDomainNetPortForwardRangesParseXML(virDomainNetPortForward *def, + xmlXPathContextPtr ctxt) +{ + int nRanges; + g_autofree xmlNodePtr *ranges = NULL; + size_t i; + + if ((nRanges = virXPathNodeSet("./range", + ctxt, &ranges)) <= 0) { + return nRanges; + } + + def->ranges = g_new0(virDomainNetPortForwardRange *, nRanges); + + for (i = 0; i < nRanges; i++) { + g_autofree virDomainNetPortForwardRange *range = NULL; + + if (!(range = virDomainNetPortForwardRangeParseXML(ranges[i], ctxt))) { + return -1; + } + def->ranges[def->nRanges++] = g_steal_pointer(&range); + } + return 0; +} + + +static virDomainNetPortForward * +virDomainNetPortForwardDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt) +{ + VIR_XPATH_NODE_AUTORESTORE(ctxt) + g_autofree char *address = NULL; + g_autoptr(virDomainNetPortForward) def = g_new0(virDomainNetPortForward, 1); + + ctxt->node = node; + + if (virXMLPropEnum(node, "proto", virDomainNetProtoTypeFromString, + VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO, + &def->proto) < 0) { + return NULL; + } + + address = virXMLPropString(node, "address"); + if (address && virSocketAddrParse(&def->address, address, AF_UNSPEC) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid address '%s' in <portForward>"), address); + return NULL; + } + + def->dev = virXMLPropString(node, "dev"); + + if (virDomainNetPortForwardRangesParseXML(def, ctxt) < 0) + return NULL; + + return g_steal_pointer(&def); +} + + +static int +virDomainNetPortForwardsParseXML(virDomainNetDef *def, + xmlXPathContextPtr ctxt) +{ + int nPortForwards; + g_autofree xmlNodePtr *portForwards = NULL; + size_t i; + + if ((nPortForwards = virXPathNodeSet("./portForward", + ctxt, &portForwards)) <= 0) { + return nPortForwards; + } + + def->portForwards = g_new0(virDomainNetPortForward *, nPortForwards); + + for (i = 0; i < nPortForwards; i++) { + g_autoptr(virDomainNetPortForward) pf = NULL; + + if (!(pf = virDomainNetPortForwardDefParseXML(portForwards[i], ctxt))) { + return -1; + } + def->portForwards[def->nPortForwards++] = g_steal_pointer(&pf); + } + return 0; +} + + static int virDomainNetDefParseXMLRequireSource(virDomainNetDef *def, xmlNodePtr source_node) @@ -9381,6 +9539,9 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, ctxt, &def->guestIP) < 0) return NULL; + if (virDomainNetPortForwardsParseXML(def, ctxt) < 0) + return NULL; + if (def->managed_tap != VIR_TRISTATE_BOOL_NO && def->ifname && (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && (STRPREFIX(def->ifname, VIR_NET_GENERATED_VNET_PREFIX) || @@ -23274,17 +23435,91 @@ static void virDomainNetBackendFormat(virBuffer *buf, virDomainNetBackend *backend) { - - if (!(backend->tap || backend->vhost)) + if (!(backend->type || backend->tap || backend->vhost + || backend->logFile || backend->upstream)) { return; + } virBufferAddLit(buf, "<backend"); + if (backend->type) + virBufferAsprintf(buf, " type='%s'", virDomainNetBackendTypeToString(backend->type)); virBufferEscapeString(buf, " tap='%s'", backend->tap); virBufferEscapeString(buf, " vhost='%s'", backend->vhost); + virBufferEscapeString(buf, " logFile='%s'", backend->logFile); + virBufferEscapeString(buf, " upstream='%s'", backend->upstream); virBufferAddLit(buf, "/>\n"); } +static void +virDomainNetPortForwardRangesFormat(virBuffer *buf, + virDomainNetPortForward *def) +{ + size_t i; + + for (i = 0; i < def->nRanges; i++) { + virDomainNetPortForwardRange *range = def->ranges[i]; + + virBufferAddLit(buf, "<range"); + + if (range->start) { + virBufferAsprintf(buf, " start='%u'", range->start); + if (range->end) + virBufferAsprintf(buf, " end='%u'", range->end); + if (range->to) + virBufferAsprintf(buf, " to='%u'", range->to); + } + + if (range->exclude) { + virBufferAsprintf(buf, " exclude='%s'", + virTristateBoolTypeToString(range->exclude)); + } + + virBufferAddLit(buf, "/>\n"); + } +} + + +static int +virDomainNetPortForwardsFormat(virBuffer *buf, + virDomainNetDef *def) +{ + size_t i; + + if (!def->nPortForwards) + return 0; + + for (i = 0; i < def->nPortForwards; i++) { + virDomainNetPortForward *pf = def->portForwards[i]; + + virBufferAddLit(buf, "<portForward"); + virBufferAsprintf(buf, " proto='%s'", + virDomainNetProtoTypeToString(pf->proto)); + if (VIR_SOCKET_ADDR_VALID(&pf->address)) { + g_autofree char *ipStr = virSocketAddrFormat(&pf->address); + + if (!ipStr) + return -1; + + virBufferAsprintf(buf, " address='%s'", ipStr); + } + virBufferEscapeString(buf, " dev='%s'", pf->dev); + + if (pf->nRanges == 0) { + virBufferAddLit(buf, "/>\n"); + } else { + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); + virDomainNetPortForwardRangesFormat(buf, pf); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</portForward>\n"); + } + } + + return 0; +} + + int virDomainNetDefFormat(virBuffer *buf, virDomainNetDef *def, @@ -23533,6 +23768,9 @@ virDomainNetDefFormat(virBuffer *buf, if (virDomainNetIPInfoFormat(buf, &def->guestIP) < 0) return -1; + if (virDomainNetPortForwardsFormat(buf, def) < 0) + return -1; + virBufferEscapeString(buf, "<script path='%s'/>\n", def->script); virBufferEscapeString(buf, "<downscript path='%s'/>\n", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e57e70866a..65fa9cf6e4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1023,6 +1023,21 @@ typedef enum { VIR_DOMAIN_NET_INTERFACE_LINK_STATE_LAST } virDomainNetInterfaceLinkState; +typedef enum { + VIR_DOMAIN_NET_BACKEND_DEFAULT = 0, + VIR_DOMAIN_NET_BACKEND_PASST, + + VIR_DOMAIN_NET_BACKEND_LAST +} virDomainNetBackendType; + +typedef enum { + VIR_DOMAIN_NET_PROTO_NONE = 0, + VIR_DOMAIN_NET_PROTO_TCP, + VIR_DOMAIN_NET_PROTO_UDP, + + VIR_DOMAIN_NET_PROTO_LAST +} virDomainNetProto; + /* Config that was actually used to bring up interface, after * resolving network reference. This is private data, only used within * libvirt, but still must maintain backward compatibility, because @@ -1052,8 +1067,27 @@ struct _virDomainActualNetDef { }; struct _virDomainNetBackend { + virDomainNetBackendType type; char *tap; char *vhost; + /* The following are currently only valid/used when backend type='passt' */ + char *logFile; /* path to logfile used by passt process */ + char *upstream; /* host interface to use for traffic egress */ +}; + +struct _virDomainNetPortForwardRange { + unsigned int start; /* original dst port range start */ + unsigned int end; /* range end (0 for "single port") */ + unsigned int to; /* start of range to forward to (0 for "unchanged") */ + virTristateBool exclude; /* true if this is a range to *not* forward */ +}; + +struct _virDomainNetPortForward { + char *dev; /* host interface of incoming traffic */ + virDomainNetProto proto; /* tcp/udp */ + virSocketAddr address; /* original dst address (empty = wildcard) */ + size_t nRanges; + virDomainNetPortForwardRange **ranges; /* list of ranges to forward */ }; /* Stores the virtual network interface configuration */ @@ -1159,6 +1193,8 @@ struct _virDomainNetDef { char *ifname_guest_actual; char *ifname_guest; virNetDevIPInfo guestIP; + size_t nPortForwards; + virDomainNetPortForward **portForwards; virDomainDeviceInfo info; char *filter; GHashTable *filterparams; @@ -3440,6 +3476,8 @@ void virDomainVsockDefFree(virDomainVsockDef *vsock); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainVsockDef, virDomainVsockDefFree); void virDomainNetTeamingInfoFree(virDomainNetTeamingInfo *teaming); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainNetTeamingInfo, virDomainNetTeamingInfoFree); +void virDomainNetPortForwardFree(virDomainNetPortForward *pf); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainNetPortForward, virDomainNetPortForwardFree); void virDomainNetDefFree(virDomainNetDef *def); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainNetDef, virDomainNetDefFree); void virDomainSmartcardDefFree(virDomainSmartcardDef *def); @@ -4027,6 +4065,8 @@ VIR_ENUM_DECL(virDomainNetVirtioTxMode); VIR_ENUM_DECL(virDomainNetMacType); VIR_ENUM_DECL(virDomainNetTeaming); VIR_ENUM_DECL(virDomainNetInterfaceLinkState); +VIR_ENUM_DECL(virDomainNetBackend); +VIR_ENUM_DECL(virDomainNetProto); VIR_ENUM_DECL(virDomainNetModel); VIR_ENUM_DECL(virDomainChrDevice); VIR_ENUM_DECL(virDomainChrChannelTarget); diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 95b8d9b419..48a701bf93 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -2134,6 +2134,14 @@ virDomainNetDefValidate(const virDomainNetDef *net) return -1; } + if (net->type != VIR_DOMAIN_NET_TYPE_USER) { + if (net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("\"<backend type='passt'/>\" can only be used with \"<interface type='user'>\"")); + return -1; + } + } + switch (net->type) { case VIR_DOMAIN_NET_TYPE_VHOSTUSER: if (!virDomainNetIsVirtioModel(net)) { @@ -2150,6 +2158,29 @@ virDomainNetDefValidate(const virDomainNetDef *net) } break; + 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", + _("<portForward> <range> requires 'start' attribute if 'end', 'to', or 'exclude' is specified")); + return -1; + } + } + } + } + break; + case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_VDPA: case VIR_DOMAIN_NET_TYPE_BRIDGE: @@ -2162,7 +2193,6 @@ virDomainNetDefValidate(const virDomainNetDef *net) case VIR_DOMAIN_NET_TYPE_HOSTDEV: case VIR_DOMAIN_NET_TYPE_VDS: case VIR_DOMAIN_NET_TYPE_ETHERNET: - case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_NULL: case VIR_DOMAIN_NET_TYPE_LAST: break; diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index 7bd9aa8e0a..adb2496cba 100644 --- a/src/conf/virconftypes.h +++ b/src/conf/virconftypes.h @@ -174,6 +174,10 @@ typedef struct _virDomainNVRAMDef virDomainNVRAMDef; typedef struct _virDomainNetBackend virDomainNetBackend; +typedef struct _virDomainNetPortForwardRange virDomainNetPortForwardRange; + +typedef struct _virDomainNetPortForward virDomainNetPortForward; + typedef struct _virDomainNetDef virDomainNetDef; typedef struct _virDomainNetTeamingInfo virDomainNetTeamingInfo; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ae746a2d51..6a0c1d0972 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -551,6 +551,7 @@ virDomainNetIsVirtioModel; virDomainNetModelTypeFromString; virDomainNetModelTypeToString; virDomainNetNotifyActualDevice; +virDomainNetPortForwardFree; virDomainNetReleaseActualDevice; virDomainNetRemove; virDomainNetRemoveByObj; diff --git a/tests/qemuxml2xmloutdata/net-user-passt.xml b/tests/qemuxml2xmloutdata/net-user-passt.xml new file mode 120000 index 0000000000..cfbc023ada --- /dev/null +++ b/tests/qemuxml2xmloutdata/net-user-passt.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/net-user-passt.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index e13da8bd2c..c9cc3416d5 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -459,6 +459,7 @@ mymain(void) DO_TEST_NOCAPS("net-vhostuser"); DO_TEST_NOCAPS("net-user"); DO_TEST_NOCAPS("net-user-addr"); + DO_TEST_NOCAPS("net-user-passt"); DO_TEST_NOCAPS("net-virtio"); DO_TEST_NOCAPS("net-virtio-device"); DO_TEST_NOCAPS("net-virtio-disable-offloads"); -- 2.38.1
On a Sunday in 2023, Laine Stump wrote:
This implements XML config to represent a subset of the features supported by 'passt' (https://passt.top), which is an alternative backend for emulated network devices that requires no elevated privileges (similar to slirp, but "better").
Along with setting the backend to use passt (via <backend type='passt'/> when the interface type='user'), we also support passt's --log-file and --interface options (via the <backend> subelement logFile and upstream attributes) and its --tcp-ports and --udp-ports options (which selectively forward incoming connections to the host on to the guest) via the new <portForward> subelement of <interface>. Here is an example of the config for a network interface that uses passt to connect:
<interface type='user'> <mac address='52:54:00:a8:33:fc'/> <ip address='192.168.221.122' family='ipv4'/> <model type='virtio'/> <backend type='passt' logFile='/tmp/xyzzy.log' upstream='eth0'/> <portForward address='10.0.0.1' proto='tcp' dev='eth0'> <range start='2022' to='22'/> <range start='5000' end='5099' to='1000'/> <range start='5010' end='5029' exclude='yes'/> </portForward> <portForward proto='udp'> <range start='10101'/> </portForward> </interface>
In this case:
* the guest will be offered address 192.168.221.122 for its interface via DHCP
* the passt process will write all log messages to /tmp/xyzzy.log
* routes to the outside for the guest will be derived from the addresses and routes associated with the host interface "eth0".
* incoming tcp port 2022 to the host will be forwarded to port 22 on the guest.
* incoming tcp ports 5000-5099 (with the exception of ports 5010-5029) to the host will be forwarded to port 1000-1099 on the guest.
* incoming udp packets on port 10101 will be forwarded (unchanged) to the guest.
Signed-off-by: Laine Stump <laine@redhat.com> --- docs/formatdomain.rst | 95 +++++++- src/conf/domain_conf.c | 242 +++++++++++++++++++- src/conf/domain_conf.h | 40 ++++ src/conf/domain_validate.c | 32 ++- src/conf/virconftypes.h | 4 + src/libvirt_private.syms | 1 + tests/qemuxml2xmloutdata/net-user-passt.xml | 1 + tests/qemuxml2xmltest.c | 1 + 8 files changed, 401 insertions(+), 15 deletions(-) create mode 120000 tests/qemuxml2xmloutdata/net-user-passt.xml
Reviewed-by: Ján Tomko <jtomko@redhat.com> The XML looks reasonable to me. All my comments below are just nitpicking.
+static int +virDomainNetPortForwardRangesParseXML(virDomainNetPortForward *def, + xmlXPathContextPtr ctxt) +{ + int nRanges; + g_autofree xmlNodePtr *ranges = NULL; + size_t i; + + if ((nRanges = virXPathNodeSet("./range", + ctxt, &ranges)) <= 0) {
This would fit on one line. Also, the braces are not necessary, but allowed per our coding style.
+ return nRanges; + } + + def->ranges = g_new0(virDomainNetPortForwardRange *, nRanges); + + for (i = 0; i < nRanges; i++) { + g_autofree virDomainNetPortForwardRange *range = NULL; +
[...]
@@ -23274,17 +23435,91 @@ static void virDomainNetBackendFormat(virBuffer *buf, virDomainNetBackend *backend) { - - if (!(backend->tap || backend->vhost)) + if (!(backend->type || backend->tap || backend->vhost + || backend->logFile || backend->upstream)) {
The prevalent style is to put the operator on the preceding line. Best way to avoid it here is to use virXMLFormatElement.
return; + }
[...]
+ if (net->type != VIR_DOMAIN_NET_TYPE_USER) { + if (net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("\"<backend type='passt'/>\" can only be used with \"<interface type='user'>\""));
Including XML in the error message can be confusing if the error reaches users that did not use XML to configure the domain.
+ return -1; + } + } + switch (net->type) { case VIR_DOMAIN_NET_TYPE_VHOSTUSER: if (!virDomainNetIsVirtioModel(net)) {
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
passt support requires "-netdev stream", which was added to QEMU in qemu-7.2.0. Signed-off-by: Laine Stump <laine@redhat.com> --- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 3 +++ tests/qemucapabilitiesdata/caps_7.2.0.x86_64.xml | 1 + 3 files changed, 8 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 2553b5b3ad..89d849539e 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -680,6 +680,9 @@ VIR_ENUM_IMPL(virQEMUCaps, "sgx-epc", /* QEMU_CAPS_SGX_EPC */ "thread-context", /* QEMU_CAPS_THREAD_CONTEXT */ "screenshot-format-png", /* QEMU_CAPS_SCREENSHOT_FORMAT_PNG */ + + /* 440 */ + "netdev.stream", /* QEMU_CAPS_NETDEV_STREAM */ ); @@ -1545,6 +1548,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "migrate-set-parameters/arg-type/block-bitmap-mapping/bitmaps/transform", QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING }, { "nbd-server-start/arg-type/tls-creds", QEMU_CAPS_NBD_TLS }, { "nbd-server-add/arg-type/bitmap", QEMU_CAPS_NBD_BITMAP }, + { "netdev_add/arg-type/+stream", QEMU_CAPS_NETDEV_STREAM }, { "netdev_add/arg-type/+vhost-vdpa", QEMU_CAPS_NETDEV_VHOST_VDPA }, /* JSON support for -netdev was introduced for the 'dgram' netdev type */ { "netdev_add/arg-type/type/^dgram", QEMU_CAPS_NETDEV_JSON }, diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index cc8b3759ea..ab156a8df0 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -660,6 +660,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_THREAD_CONTEXT, /* -object thread-context */ QEMU_CAPS_SCREENSHOT_FORMAT_PNG, /* screendump command supports png format */ + /* 440 */ + QEMU_CAPS_NETDEV_STREAM, /* -netdev stream */ + QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_7.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_7.2.0.x86_64.xml index 6bc739065f..2dc6d1978f 100644 --- a/tests/qemucapabilitiesdata/caps_7.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.2.0.x86_64.xml @@ -201,6 +201,7 @@ <flag name='query-stats-schemas'/> <flag name='thread-context'/> <flag name='screenshot-format-png'/> + <flag name='netdev.stream'/> <version>7001091</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100245</microcodeVersion> -- 2.38.1
On a Sunday in 2023, Laine Stump wrote:
passt support requires "-netdev stream", which was added to QEMU in qemu-7.2.0.
Signed-off-by: Laine Stump <laine@redhat.com> --- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 3 +++ tests/qemucapabilitiesdata/caps_7.2.0.x86_64.xml | 1 + 3 files changed, 8 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
-- It seems a bit wasteful to allocate another string for this, since it is 100% always ${stateDir}/passt, but everyone else is doing this (e.g. slirpStateDir), so I'm just following along with the cult. Signed-off-by: Laine Stump <laine@redhat.com> --- src/qemu/qemu_conf.c | 2 ++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 12 ++++++++++++ 3 files changed, 15 insertions(+) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index ae5bbcd138..bb6a55738b 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -225,6 +225,7 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged, cfg->configDir = g_strdup_printf("%s/qemu", cfg->configBaseDir); cfg->autostartDir = g_strdup_printf("%s/qemu/autostart", cfg->configBaseDir); cfg->slirpStateDir = g_strdup_printf("%s/slirp", cfg->stateDir); + cfg->passtStateDir = g_strdup_printf("%s/passt", cfg->stateDir); cfg->dbusStateDir = g_strdup_printf("%s/dbus", cfg->stateDir); /* Set the default directory to find TLS X.509 certificates. @@ -310,6 +311,7 @@ static void virQEMUDriverConfigDispose(void *obj) g_free(cfg->stateDir); g_free(cfg->swtpmStateDir); g_free(cfg->slirpStateDir); + g_free(cfg->passtStateDir); g_free(cfg->dbusStateDir); g_free(cfg->libDir); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 8cf2dd2ec5..159fd61d2b 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -95,6 +95,7 @@ struct _virQEMUDriverConfig { char *stateDir; char *swtpmStateDir; char *slirpStateDir; + char *passtStateDir; char *dbusStateDir; /* These two directories are ones QEMU processes use (so must match * the QEMU user/group */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d509582719..3d8bea6462 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -663,6 +663,11 @@ qemuStateInitialize(bool privileged, cfg->slirpStateDir); goto error; } + if (g_mkdir_with_parents(cfg->passtStateDir, 0777) < 0) { + virReportSystemError(errno, _("Failed to create passt state dir %s"), + cfg->passtStateDir); + goto error; + } if (virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group, VIR_DIR_CREATE_ALLOW_EXIST) < 0) { @@ -811,6 +816,13 @@ qemuStateInitialize(bool privileged, (int)cfg->group); goto error; } + if (chown(cfg->passtStateDir, cfg->user, cfg->group) < 0) { + virReportSystemError(errno, + _("unable to set ownership of '%s' to %d:%d"), + cfg->passtStateDir, (int)cfg->user, + (int)cfg->group); + goto error; + } run_uid = cfg->user; run_gid = cfg->group; -- 2.38.1
On a Sunday in 2023, Laine Stump wrote:
--
It seems a bit wasteful to allocate another string for this, since it is 100% always ${stateDir}/passt, but everyone else is doing this (e.g. slirpStateDir), so I'm just following along with the cult.
Did you mean to mark this as a note under '---'?
Signed-off-by: Laine Stump <laine@redhat.com> --- src/qemu/qemu_conf.c | 2 ++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 12 ++++++++++++ 3 files changed, 15 insertions(+)
Either way: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On 1/9/23 2:23 AM, Ján Tomko wrote:
On a Sunday in 2023, Laine Stump wrote:
--
It seems a bit wasteful to allocate another string for this, since it is 100% always ${stateDir}/passt, but everyone else is doing this (e.g. slirpStateDir), so I'm just following along with the cult.
Did you mean to mark this as a note under '---'?
Yes and no :-) If it was me coming in later and seeing this new field I would wonder "why did they do that?" (just as I did with slirpStateDir, but that ship has sailed long ago), so I feel compelled to leave a note explaining myself. On the other hand, there's no *harm* to having it (other than the extra memory used), and commit logs referring to cults probably doesn't do much to inspire confidence, so I should probably leave it out when I push :-)
Signed-off-by: Laine Stump <laine@redhat.com> --- src/qemu/qemu_conf.c | 2 ++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 12 ++++++++++++ 3 files changed, 15 insertions(+)
Either way: Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
This consists of (1) adding the necessary args to the qemu commandline netdev option, and (2) starting a passt process prior to starting qemu, and making sure that it is terminated when it's no longer needed. Under normal circumstances, passt will terminate itself as soon as qemu closes its socket, but in case of some error where qemu is never started, or fails to startup completely, we need to terminate passt manually. Signed-off-by: Laine Stump <laine@redhat.com> --- meson.build | 1 + po/POTFILES | 1 + src/qemu/meson.build | 2 + src/qemu/qemu_command.c | 11 +- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_domain.c | 5 +- src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_extdevice.c | 25 +- src/qemu/qemu_hotplug.c | 26 +- src/qemu/qemu_passt.c | 284 ++++++++++++++++++ src/qemu/qemu_passt.h | 38 +++ src/qemu/qemu_process.c | 1 + src/qemu/qemu_validate.c | 9 +- tests/qemuxml2argvdata/net-user-passt.args | 34 +++ .../net-user-passt.x86_64-latest.args | 37 +++ tests/qemuxml2argvtest.c | 2 + 16 files changed, 470 insertions(+), 12 deletions(-) create mode 100644 src/qemu/qemu_passt.c create mode 100644 src/qemu/qemu_passt.h create mode 100644 tests/qemuxml2argvdata/net-user-passt.args create mode 100644 tests/qemuxml2argvdata/net-user-passt.x86_64-latest.args diff --git a/meson.build b/meson.build index 177009c44d..6604ba20c8 100644 --- a/meson.build +++ b/meson.build @@ -780,6 +780,7 @@ optional_programs = [ 'mm-ctl', 'modprobe', 'ovs-vsctl', + 'passt', 'pdwtags', 'rmmod', 'scrub', diff --git a/po/POTFILES b/po/POTFILES index 169e2a41dc..f979c9c4bc 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -180,6 +180,7 @@ src/qemu/qemu_monitor.c src/qemu/qemu_monitor_json.c src/qemu/qemu_monitor_text.c src/qemu/qemu_namespace.c +src/qemu/qemu_passt.c src/qemu/qemu_process.c src/qemu/qemu_qapi.c src/qemu/qemu_saveimage.c diff --git a/src/qemu/meson.build b/src/qemu/meson.build index 96952cc52d..c8806bbc36 100644 --- a/src/qemu/meson.build +++ b/src/qemu/meson.build @@ -28,6 +28,7 @@ qemu_driver_sources = [ 'qemu_monitor_json.c', 'qemu_monitor_text.c', 'qemu_namespace.c', + 'qemu_passt.c', 'qemu_process.c', 'qemu_qapi.c', 'qemu_saveimage.c', @@ -216,6 +217,7 @@ if conf.has('WITH_QEMU') localstatedir / 'log' / 'swtpm' / 'libvirt' / 'qemu', runstatedir / 'libvirt' / 'qemu', runstatedir / 'libvirt' / 'qemu' / 'dbus', + runstatedir / 'libvirt' / 'qemu' / 'passt', runstatedir / 'libvirt' / 'qemu' / 'slirp', runstatedir / 'libvirt' / 'qemu' / 'swtpm', ] diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 42bd7cb99f..89af798a31 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -27,6 +27,7 @@ #include "qemu_interface.h" #include "qemu_alias.h" #include "qemu_security.h" +#include "qemu_passt.h" #include "qemu_slirp.h" #include "qemu_block.h" #include "qemu_fd.h" @@ -3793,7 +3794,8 @@ qemuBuildNicDevProps(virDomainDef *def, virJSONValue * -qemuBuildHostNetProps(virDomainNetDef *net) +qemuBuildHostNetProps(virDomainObj *vm, + virDomainNetDef *net) { virDomainNetType netType = virDomainNetGetActualType(net); size_t i; @@ -3908,7 +3910,10 @@ qemuBuildHostNetProps(virDomainNetDef *net) break; case VIR_DOMAIN_NET_TYPE_USER: - if (netpriv->slirpfd) { + if (net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) { + if (qemuPasstAddNetProps(vm, net, &netprops) < 0) + return NULL; + } else if (netpriv->slirpfd) { if (virJSONValueObjectAdd(&netprops, "s:type", "socket", "s:fd", qemuFDPassDirectGetPath(netpriv->slirpfd), @@ -8452,7 +8457,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, qemuFDPassDirectTransferCommand(netpriv->slirpfd, cmd); qemuFDPassTransferCommand(netpriv->vdpafd, cmd); - if (!(hostnetprops = qemuBuildHostNetProps(net))) + if (!(hostnetprops = qemuBuildHostNetProps(vm, net))) goto cleanup; if (qemuBuildNetdevCommandlineFromJSON(cmd, hostnetprops, qemuCaps) < 0) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 761cc5d865..c49096a057 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -78,7 +78,8 @@ virJSONValue * qemuBuildChannelGuestfwdNetdevProps(virDomainChrDef *chr); virJSONValue * -qemuBuildHostNetProps(virDomainNetDef *net); +qemuBuildHostNetProps(virDomainObj *vm, + virDomainNetDef *net); int qemuBuildInterfaceConnect(virDomainObj *vm, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8892f28fce..ec55bc47c4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2351,6 +2351,7 @@ qemuDomainGetSlirpHelperOk(virDomainObj *vm) /* if there is a builtin slirp, prevent slirp-helper */ if (net->type == VIR_DOMAIN_NET_TYPE_USER && + net->backend.type == VIR_DOMAIN_NET_BACKEND_DEFAULT && !QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp) return false; } @@ -9906,7 +9907,8 @@ qemuDomainSupportsNicdev(virDomainDef *def, } bool -qemuDomainNetSupportsMTU(virDomainNetType type) +qemuDomainNetSupportsMTU(virDomainNetType type, + virDomainNetBackendType backend) { switch (type) { case VIR_DOMAIN_NET_TYPE_NETWORK: @@ -9915,6 +9917,7 @@ qemuDomainNetSupportsMTU(virDomainNetType type) case VIR_DOMAIN_NET_TYPE_VHOSTUSER: return true; case VIR_DOMAIN_NET_TYPE_USER: + return backend == VIR_DOMAIN_NET_BACKEND_PASST; case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 2f027fad87..c82d56191d 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -883,7 +883,8 @@ int qemuDomainRefreshVcpuHalted(virDomainObj *vm, bool qemuDomainSupportsNicdev(virDomainDef *def, virDomainNetDef *net); -bool qemuDomainNetSupportsMTU(virDomainNetType type); +bool qemuDomainNetSupportsMTU(virDomainNetType type, + virDomainNetBackendType backend); int qemuDomainSetPrivatePaths(virQEMUDriver *driver, virDomainObj *vm); diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index d5c3e8ed71..f7b2e2e653 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -25,6 +25,7 @@ #include "qemu_dbus.h" #include "qemu_domain.h" #include "qemu_tpm.h" +#include "qemu_passt.h" #include "qemu_slirp.h" #include "qemu_virtiofs.h" @@ -194,8 +195,16 @@ qemuExtDevicesStart(virQEMUDriver *driver, for (i = 0; i < def->nnets; i++) { virDomainNetDef *net = def->nets[i]; - if (qemuSlirpStart(vm, net, incomingMigration) < 0) - return -1; + if (net->type != VIR_DOMAIN_NET_TYPE_USER) + continue; + + if (net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) { + if (qemuPasstStart(vm, net) < 0) + return -1; + } else { + if (qemuSlirpStart(vm, net, incomingMigration) < 0) + return -1; + } } for (i = 0; i < def->nfss; i++) { @@ -254,6 +263,12 @@ qemuExtDevicesStop(virQEMUDriver *driver, if (slirp) qemuSlirpStop(slirp, vm, driver, net); + + if (net->type == VIR_DOMAIN_NET_TYPE_USER && + net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) { + qemuPasstStop(vm, net); + } + if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET && net->downscript) virNetDevRunEthernetScript(net->ifname, net->downscript); } @@ -319,6 +334,12 @@ qemuExtDevicesSetupCgroup(virQEMUDriver *driver, if (slirp && qemuSlirpSetupCgroup(slirp, cgroup) < 0) return -1; + + if (net->type == VIR_DOMAIN_NET_TYPE_USER && + net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST && + qemuPasstSetupCgroup(vm, net, cgroup) < 0) { + return -1; + } } for (i = 0; i < def->ntpms; i++) { diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6e300f547c..217230f845 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -31,6 +31,7 @@ #include "qemu_command.h" #include "qemu_hostdev.h" #include "qemu_interface.h" +#include "qemu_passt.h" #include "qemu_process.h" #include "qemu_security.h" #include "qemu_block.h" @@ -1201,8 +1202,16 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, break; case VIR_DOMAIN_NET_TYPE_USER: - if (!priv->disableSlirp && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) { + if (net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) { + + if (qemuPasstStart(vm, net) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to start passt")); + goto cleanup; + } + + } else if (!priv->disableSlirp && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) { if (qemuInterfacePrepareSlirp(driver, net) < 0) goto cleanup; @@ -1269,7 +1278,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, virNetDevSetMTU(net->ifname, net->mtu) < 0) goto cleanup; - if (!(netprops = qemuBuildHostNetProps(net))) + if (!(netprops = qemuBuildHostNetProps(vm, net))) goto cleanup; qemuDomainObjEnterMonitor(vm); @@ -1417,6 +1426,12 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, netdev_name = g_strdup_printf("host%s", net->info.alias); if (QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp) qemuSlirpStop(QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp, vm, driver, net); + + if (net->type == VIR_DOMAIN_NET_TYPE_USER && + net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) { + qemuPasstStop(vm, net); + } + qemuDomainObjEnterMonitor(vm); if (charDevPlugged && qemuMonitorDetachCharDev(priv->mon, charDevAlias) < 0) @@ -4618,6 +4633,11 @@ qemuDomainRemoveNetDevice(virQEMUDriver *driver, if (QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp) qemuSlirpStop(QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp, vm, driver, net); + if (net->type == VIR_DOMAIN_NET_TYPE_USER && + net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) { + qemuPasstStop(vm, net); + } + virDomainAuditNet(vm, net, NULL, "detach", true); for (i = 0; i < vm->def->nnets; i++) { diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c new file mode 100644 index 0000000000..5941594811 --- /dev/null +++ b/src/qemu/qemu_passt.c @@ -0,0 +1,284 @@ +/* + * qemu_passt.c: QEMU passt support + * + * Copyright (C) 2022 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "qemu_dbus.h" +#include "qemu_extdevice.h" +#include "qemu_security.h" +#include "qemu_passt.h" +#include "virenum.h" +#include "virerror.h" +#include "virjson.h" +#include "virlog.h" +#include "virpidfile.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("qemu.passt"); + + +static char * +qemuPasstCreatePidFilename(virDomainObj *vm, + virDomainNetDef *net) +{ + qemuDomainObjPrivate *priv = vm->privateData; + virQEMUDriver *driver = priv->driver; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autofree char *name = NULL; + + name = g_strdup_printf("%s-%s-passt", vm->def->name, net->info.alias); + + return virPidFileBuildPath(cfg->passtStateDir, name); +} + + +static char * +qemuPasstCreateSocketPath(virDomainObj *vm, + virDomainNetDef *net) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + qemuDomainObjPrivate *priv = vm->privateData; + virQEMUDriver *driver = priv->driver; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + + return g_strdup_printf("%s/%s-%s.socket", + cfg->passtStateDir, + virUUIDFormat(vm->def->uuid, uuidstr), + net->info.alias); +} + + +static int +qemuPasstGetPid(virDomainObj *vm, + virDomainNetDef *net, + pid_t *pid) +{ + g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net); + + return virPidFileReadPathIfLocked(pidfile, pid); +} + + +int +qemuPasstAddNetProps(virDomainObj *vm, + virDomainNetDef *net, + virJSONValue **netprops) +{ + g_autofree char *passtSocketName = qemuPasstCreateSocketPath(vm, net); + g_autoptr(virJSONValue) addrprops = NULL; + + if (virJSONValueObjectAdd(&addrprops, + "s:type", "unix", + "s:path", passtSocketName, + NULL) < 0) { + return -1; + } + + if (virJSONValueObjectAdd(netprops, + "s:type", "stream", + "a:addr", &addrprops, + "b:server", false, + /* "u:reconnect", 5, */ + NULL) < 0) { + return -1; + } + return 0; +} + + +void +qemuPasstStop(virDomainObj *vm, + virDomainNetDef *net) +{ + g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net); + virErrorPtr orig_err; + + virErrorPreserveLast(&orig_err); + + + if (virPidFileForceCleanupPath(pidfile) < 0) + VIR_WARN("Unable to kill passt process"); + + virErrorRestore(&orig_err); +} + + +int +qemuPasstSetupCgroup(virDomainObj *vm, + virDomainNetDef *net, + virCgroup *cgroup) +{ + pid_t pid = (pid_t) -1; + + if (qemuPasstGetPid(vm, net, &pid) < 0 || pid <= 0) + return -1; + + return virCgroupAddProcess(cgroup, pid); +} + + +int +qemuPasstStart(virDomainObj *vm, + virDomainNetDef *net) +{ + qemuDomainObjPrivate *priv = vm->privateData; + virQEMUDriver *driver = priv->driver; + g_autofree char *passtSocketName = qemuPasstCreateSocketPath(vm, net); + g_autoptr(virCommand) cmd = NULL; + g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net); + char macaddr[VIR_MAC_STRING_BUFLEN]; + size_t i; + pid_t pid = (pid_t) -1; + int exitstatus = 0; + int cmdret = 0; + VIR_AUTOCLOSE errfd = -1; + + cmd = virCommandNew(PASST); + + virCommandClearCaps(cmd); + virCommandSetPidFile(cmd, pidfile); + virCommandSetErrorFD(cmd, &errfd); + virCommandDaemonize(cmd); + + virCommandAddArgList(cmd, + "--one-off", + "--socket", passtSocketName, + "--mac-addr", virMacAddrFormat(&net->mac, macaddr), + NULL); + + if (net->mtu) { + virCommandAddArg(cmd, "--mtu"); + virCommandAddArgFormat(cmd, "%u", net->mtu); + } + + if (net->backend.upstream) + virCommandAddArgList(cmd, "--interface", net->backend.upstream, NULL); + + if (net->backend.logFile) + virCommandAddArgList(cmd, "--log-file", net->backend.logFile, NULL); + + /* Add IP address info */ + for (i = 0; i < net->guestIP.nips; i++) { + const virNetDevIPAddr *ip = net->guestIP.ips[i]; + g_autofree char *addr = NULL; + + /* validation has already limited us to + * a single IPv4 and single IPv6 address + */ + if (!(addr = virSocketAddrFormat(&ip->address))) + return -1; + + virCommandAddArgList(cmd, "--address", addr, NULL); + + if (ip->prefix && VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET)) { + /* validation already made sure no prefix is + * specified for IPv6 (not allowed by passt) + */ + virCommandAddArg(cmd, "--netmask"); + virCommandAddArgFormat(cmd, "%u", ip->prefix); + } + } + + /* Add port forwarding info */ + + for (i = 0; i < net->nPortForwards; i++) { + virDomainNetPortForward *pf = net->portForwards[i]; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + + if (pf->proto == VIR_DOMAIN_NET_PROTO_TCP) { + virCommandAddArg(cmd, "--tcp-ports"); + } else if (pf->proto == VIR_DOMAIN_NET_PROTO_UDP) { + virCommandAddArg(cmd, "--udp-ports"); + } else { + /* validation guarantees this will never happen */ + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid portForward proto value %u"), pf->proto); + goto error; + } + + if (VIR_SOCKET_ADDR_VALID(&pf->address)) { + g_autofree char *addr = NULL; + + if (!(addr = virSocketAddrFormat(&pf->address))) + goto error; + + virBufferAddStr(&buf, addr); + + if (pf->dev) + virBufferAsprintf(&buf, "%%%s", pf->dev); + + virBufferAddChar(&buf, '/'); + } + + if (!pf->nRanges) { + virBufferAddLit(&buf, "all"); + } else { + size_t r; + + for (r = 0; r < pf->nRanges; r++) { + virDomainNetPortForwardRange *range = pf->ranges[r]; + + if (r > 0) + virBufferAddChar(&buf, ','); + + if (range->exclude == VIR_TRISTATE_BOOL_YES) + virBufferAddChar(&buf, '~'); + + virBufferAsprintf(&buf, "%u", range->start); + + if (range->end) + virBufferAsprintf(&buf, "-%u", range->end); + if (range->to) { + virBufferAsprintf(&buf, ":%u", range->to); + if (range->end) { + virBufferAsprintf(&buf, "-%u", + range->end + range->to - range->start); + } + } + } + } + virCommandAddArg(cmd, virBufferCurrentContent(&buf)); + } + + + if (qemuExtDeviceLogCommand(driver, vm, cmd, "passt") < 0) + goto error; + + if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus, &cmdret) < 0) + goto error; + + if (cmdret < 0 || exitstatus != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not start 'passt'. exitstatus: %d"), exitstatus); + goto error; + } + + return 0; + + error: + ignore_value(virPidFileReadPathIfLocked(pidfile, &pid)); + if (pid != -1) + virProcessKillPainfully(pid, true); + if (pidfile) + unlink(pidfile); + + return -1; +} diff --git a/src/qemu/qemu_passt.h b/src/qemu/qemu_passt.h new file mode 100644 index 0000000000..623b494b7a --- /dev/null +++ b/src/qemu/qemu_passt.h @@ -0,0 +1,38 @@ +/* + * qemu_passt.h: QEMU passt support + * + * Copyright (C) 2022 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#pragma once + +#include "qemu_conf.h" + +int +qemuPasstAddNetProps(virDomainObj *vm, + virDomainNetDef *net, + virJSONValue **netprops); + +int qemuPasstStart(virDomainObj *vm, + virDomainNetDef *net); + +void qemuPasstStop(virDomainObj *vm, + virDomainNetDef *net); + +int qemuPasstSetupCgroup(virDomainObj *vm, + virDomainNetDef *net, + virCgroup *cgroup); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b6adcf2f2a..b5f1bcf557 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5794,6 +5794,7 @@ qemuProcessNetworkPrepareDevices(virQEMUDriver *driver, if (virDomainHostdevInsert(def, hostdev) < 0) return -1; } else if (actualType == VIR_DOMAIN_NET_TYPE_USER && + net->backend.type == VIR_DOMAIN_NET_BACKEND_DEFAULT && !priv->disableSlirp && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) { if (qemuInterfacePrepareSlirp(driver, net) < 0) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index c687df0bfc..6e04b22da4 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1830,6 +1830,13 @@ qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net, } hasIPv6 = true; + if (ip->prefix && ip->prefix != 64) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported IPv6 address prefix='%u' - must be 64"), + ip->prefix); + return -1; + } + if (ip->prefix > 120) { virReportError(VIR_ERR_XML_ERROR, "%s", _("prefix too long")); @@ -1892,7 +1899,7 @@ qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net, } if (net->mtu && - !qemuDomainNetSupportsMTU(net->type)) { + !qemuDomainNetSupportsMTU(net->type, net->backend.type)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("setting MTU on interface type %s is not supported yet"), virDomainNetTypeToString(net->type)); diff --git a/tests/qemuxml2argvdata/net-user-passt.args b/tests/qemuxml2argvdata/net-user-passt.args new file mode 100644 index 0000000000..8c887ca210 --- /dev/null +++ b/tests/qemuxml2argvdata/net-user-passt.args @@ -0,0 +1,34 @@ +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 \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc,usb=off,dump-guest-core=off \ +-accel tcg \ +-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=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ +-device ide-hd,bus=ide.0,unit=0,drive=libvirt-1-format,id=ide0-0-0,bootindex=1 \ +-netdev stream,addr.path=/var/run/libvirt/qemu/passt/UUID-net0.socket,server=off,reconnect=5,id=hostnet0 \ +-device rtl8139,netdev=hostnet0,id=net0,mac=00:11:22:33:44:55,bus=pci.0,addr=0x2 \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/net-user-passt.x86_64-latest.args b/tests/qemuxml2argvdata/net-user-passt.x86_64-latest.args new file mode 100644 index 0000000000..69194a2cfc --- /dev/null +++ b/tests/qemuxml2argvdata/net-user-passt.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 \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-accel tcg \ +-cpu qemu64 \ +-m 214 \ +-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 \ +-no-acpi \ +-boot strict=on \ +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ +-device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-format","id":"ide0-0-0","bootindex":1}' \ +-netdev '{"type":"stream","addr":{"type":"unix","path":"/bad-test-used-env-xdg-runtime-dir/libvirt/qemu/run/passt/c7a5fdbd-edaf-9455-926a-d65c16db1809-net0.socket"},"server":false,"id":"hostnet0"}' \ +-device '{"driver":"rtl8139","netdev":"hostnet0","id":"net0","mac":"00:11:22:33:44:55","bus":"pci.0","addr":"0x2"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4d563d3a09..a8b4a8d18c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -471,6 +471,7 @@ testCompareXMLToArgvCreateArgs(virQEMUDriver *drv, virDomainNetDef *net = vm->def->nets[i]; if (net->type == VIR_DOMAIN_NET_TYPE_USER && + net->backend.type == VIR_DOMAIN_NET_BACKEND_DEFAULT && virQEMUCapsGet(info->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) { qemuSlirp *slirp = qemuSlirpNew(); slirp->fd[0] = 42; @@ -1460,6 +1461,7 @@ mymain(void) DO_TEST_NOCAPS("net-user"); DO_TEST_CAPS_ARCH_LATEST_FULL("net-user", "x86_64", ARG_FLAGS, FLAG_SLIRP_HELPER); DO_TEST_NOCAPS("net-user-addr"); + DO_TEST_CAPS_LATEST("net-user-passt"); DO_TEST_NOCAPS("net-virtio"); DO_TEST_NOCAPS("net-virtio-device"); DO_TEST_NOCAPS("net-virtio-disable-offloads"); -- 2.38.1
On a Sunday in 2023, Laine Stump wrote:
This consists of (1) adding the necessary args to the qemu commandline netdev option, and (2) starting a passt process prior to starting qemu, and making sure that it is terminated when it's no longer needed. Under normal circumstances, passt will terminate itself as soon as qemu closes its socket, but in case of some error where qemu is never started, or fails to startup completely, we need to terminate passt manually.
Signed-off-by: Laine Stump <laine@redhat.com> --- meson.build | 1 + po/POTFILES | 1 + src/qemu/meson.build | 2 + src/qemu/qemu_command.c | 11 +- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_domain.c | 5 +- src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_extdevice.c | 25 +- src/qemu/qemu_hotplug.c | 26 +- src/qemu/qemu_passt.c | 284 ++++++++++++++++++ src/qemu/qemu_passt.h | 38 +++ src/qemu/qemu_process.c | 1 + src/qemu/qemu_validate.c | 9 +- tests/qemuxml2argvdata/net-user-passt.args | 34 +++ .../net-user-passt.x86_64-latest.args | 37 +++ tests/qemuxml2argvtest.c | 2 + 16 files changed, 470 insertions(+), 12 deletions(-) create mode 100644 src/qemu/qemu_passt.c create mode 100644 src/qemu/qemu_passt.h create mode 100644 tests/qemuxml2argvdata/net-user-passt.args create mode 100644 tests/qemuxml2argvdata/net-user-passt.x86_64-latest.args
diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c new file mode 100644 index 0000000000..5941594811 --- /dev/null +++ b/src/qemu/qemu_passt.c @@ -0,0 +1,284 @@ +/* + * qemu_passt.c: QEMU passt support + * + * Copyright (C) 2022 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "qemu_dbus.h" +#include "qemu_extdevice.h" +#include "qemu_security.h" +#include "qemu_passt.h" +#include "virenum.h" +#include "virerror.h" +#include "virjson.h" +#include "virlog.h" +#include "virpidfile.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("qemu.passt"); + + +static char * +qemuPasstCreatePidFilename(virDomainObj *vm, + virDomainNetDef *net) +{ + qemuDomainObjPrivate *priv = vm->privateData; + virQEMUDriver *driver = priv->driver; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autofree char *name = NULL; + + name = g_strdup_printf("%s-%s-passt", vm->def->name, net->info.alias);
Please use virDomainDefGetShortName for filename purposes.
+ + return virPidFileBuildPath(cfg->passtStateDir, name); +} + +
[...]
+int +qemuPasstAddNetProps(virDomainObj *vm, + virDomainNetDef *net, + virJSONValue **netprops) +{ + g_autofree char *passtSocketName = qemuPasstCreateSocketPath(vm, net); + g_autoptr(virJSONValue) addrprops = NULL; + + if (virJSONValueObjectAdd(&addrprops, + "s:type", "unix", + "s:path", passtSocketName, + NULL) < 0) { + return -1; + } + + if (virJSONValueObjectAdd(netprops, + "s:type", "stream", + "a:addr", &addrprops, + "b:server", false,
+ /* "u:reconnect", 5, */
Debugging leftover?
+ NULL) < 0) { + return -1; + } + return 0; +} + + +void +qemuPasstStop(virDomainObj *vm, + virDomainNetDef *net) +{ + g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net); + virErrorPtr orig_err; + + virErrorPreserveLast(&orig_err); + +
Extra whitespace
+ if (virPidFileForceCleanupPath(pidfile) < 0) + VIR_WARN("Unable to kill passt process"); + + virErrorRestore(&orig_err); +} + +
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On 1/9/23 2:32 AM, Ján Tomko wrote:
On a Sunday in 2023, Laine Stump wrote:
This consists of (1) adding the necessary args to the qemu commandline netdev option, and (2) starting a passt process prior to starting qemu, and making sure that it is terminated when it's no longer needed. Under normal circumstances, passt will terminate itself as soon as qemu closes its socket, but in case of some error where qemu is never started, or fails to startup completely, we need to terminate passt manually.
Signed-off-by: Laine Stump <laine@redhat.com> --- meson.build | 1 + po/POTFILES | 1 + src/qemu/meson.build | 2 + src/qemu/qemu_command.c | 11 +- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_domain.c | 5 +- src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_extdevice.c | 25 +- src/qemu/qemu_hotplug.c | 26 +- src/qemu/qemu_passt.c | 284 ++++++++++++++++++ src/qemu/qemu_passt.h | 38 +++ src/qemu/qemu_process.c | 1 + src/qemu/qemu_validate.c | 9 +- tests/qemuxml2argvdata/net-user-passt.args | 34 +++ .../net-user-passt.x86_64-latest.args | 37 +++ tests/qemuxml2argvtest.c | 2 + 16 files changed, 470 insertions(+), 12 deletions(-) create mode 100644 src/qemu/qemu_passt.c create mode 100644 src/qemu/qemu_passt.h create mode 100644 tests/qemuxml2argvdata/net-user-passt.args create mode 100644 tests/qemuxml2argvdata/net-user-passt.x86_64-latest.args
diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c new file mode 100644 index 0000000000..5941594811 --- /dev/null +++ b/src/qemu/qemu_passt.c @@ -0,0 +1,284 @@ +/* + * qemu_passt.c: QEMU passt support + * + * Copyright (C) 2022 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "qemu_dbus.h" +#include "qemu_extdevice.h" +#include "qemu_security.h" +#include "qemu_passt.h" +#include "virenum.h" +#include "virerror.h" +#include "virjson.h" +#include "virlog.h" +#include "virpidfile.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("qemu.passt"); + + +static char * +qemuPasstCreatePidFilename(virDomainObj *vm, + virDomainNetDef *net) +{ + qemuDomainObjPrivate *priv = vm->privateData; + virQEMUDriver *driver = priv->driver; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autofree char *name = NULL; + + name = g_strdup_printf("%s-%s-passt", vm->def->name, net->info.alias);
Please use virDomainDefGetShortName for filename purposes.
Why? If I use GetShortName, then there's the possibility that two domains would want to use the same name for the pidfile. Would it be better to use the domain's UUID (as I did for the socket path?) The advantage of using the name is that it's easier for a human to find, but while the uuid is usually longer, its length is at least predictable/consistent, and I suppose a human will probably never need to find the pidfile anyway...
+ + return virPidFileBuildPath(cfg->passtStateDir, name); +} + +
[...]
+int +qemuPasstAddNetProps(virDomainObj *vm, + virDomainNetDef *net, + virJSONValue **netprops) +{ + g_autofree char *passtSocketName = qemuPasstCreateSocketPath(vm, net); + g_autoptr(virJSONValue) addrprops = NULL; + + if (virJSONValueObjectAdd(&addrprops, + "s:type", "unix", + "s:path", passtSocketName, + NULL) < 0) { + return -1; + } + + if (virJSONValueObjectAdd(netprops, + "s:type", "stream", + "a:addr", &addrprops, + "b:server", false,
+ /* "u:reconnect", 5, */
Debugging leftover?
It's a new option that hasn't been pushed to QEMU upstream yet. I had meant to completely remove it, but forgot. I'll get rid of it before I push.
On a Monday in 2023, Laine Stump wrote:
On 1/9/23 2:32 AM, Ján Tomko wrote:
On a Sunday in 2023, Laine Stump wrote:
+static char * +qemuPasstCreatePidFilename(virDomainObj *vm, + virDomainNetDef *net) +{ + qemuDomainObjPrivate *priv = vm->privateData; + virQEMUDriver *driver = priv->driver; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autofree char *name = NULL; + + name = g_strdup_printf("%s-%s-passt", vm->def->name, net->info.alias);
Please use virDomainDefGetShortName for filename purposes.
Why? If I use GetShortName, then there's the possibility that two domains would want to use the same name for the pidfile.
Because otherwise the PID filename might exceed maximum path length for domains with very long names. The short name should be unique since it contains the domain ID. The ShortName function is also used for slirp and virtiofsd pid filenames, so we have the cult argument too :) (If you know of an issue with this usage, please share it - it could be the cause of us not cleaning up virtiofsd properly sometimes [0]) Jano
Would it be better to use the domain's UUID (as I did for the socket path?) The advantage of using the name is that it's easier for a human to find, but while the uuid is usually longer, its length is at least predictable/consistent, and I suppose a human will probably never need to find the pidfile anyway...
+ + return virPidFileBuildPath(cfg->passtStateDir, name); +} + +
On 1/9/23 9:51 AM, Ján Tomko wrote:
On a Monday in 2023, Laine Stump wrote:
On 1/9/23 2:32 AM, Ján Tomko wrote:
On a Sunday in 2023, Laine Stump wrote:
+static char * +qemuPasstCreatePidFilename(virDomainObj *vm, + virDomainNetDef *net) +{ + qemuDomainObjPrivate *priv = vm->privateData; + virQEMUDriver *driver = priv->driver; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autofree char *name = NULL; + + name = g_strdup_printf("%s-%s-passt", vm->def->name, net->info.alias);
Please use virDomainDefGetShortName for filename purposes.
Why? If I use GetShortName, then there's the possibility that two domains would want to use the same name for the pidfile.
Because otherwise the PID filename might exceed maximum path length for domains with very long names.
The short name should be unique since it contains the domain ID.
Ah, I missed that bit. I guess I *can* use it for the passt socket path then...
The only reason we need it at build time is to find its location in $PATH so it can be hardcoded into the libvirt binary (and avoid the possibility of someone adding in a malicious binary somewhere earlier in the path, I guess). Only 'recommend' passt during installation though, since it is not needed unless someone is actually using it. There is no need to add in a build-time "WITH_PASST" option (IMO), since it adds very little to the size of the code - "PASST" (the path to the binary) will just be set to "passt", so if someone does manage to build and install passt on an older version of Fedora or RHEL, it will still work (as long as it's installed somewhere in the path). Signed-off-by: Laine Stump <laine@redhat.com> --- libvirt.spec.in | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index 6b8acf252e..d9529fc76c 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -347,6 +347,9 @@ BuildRequires: libssh2-devel >= 1.3.0 %if %{with_netcf} BuildRequires: netcf-devel >= 0.2.2 %endif +%if (0%{?fedora} >= 36) || (0%{?rhel} >= 9) +BuildRequires: passt +%endif %if %{with_esx} BuildRequires: libcurl-devel %endif @@ -717,6 +720,9 @@ Requires: lzop Requires: xz Requires: systemd-container Requires: swtpm-tools + %if (0%{?fedora} >= 36) || (0%{?rhel} >= 9) +Recommends: passt + %endif %description daemon-driver-qemu The qemu driver plugin for the libvirtd daemon, providing @@ -1968,6 +1974,7 @@ exit 0 %config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.qemu %ghost %dir %{_rundir}/libvirt/qemu/ %ghost %dir %{_rundir}/libvirt/qemu/dbus/ +%ghost %dir %{_rundir}/libvirt/qemu/passt/ %ghost %dir %{_rundir}/libvirt/qemu/slirp/ %ghost %dir %{_rundir}/libvirt/qemu/swtpm/ %dir %attr(0751, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/ -- 2.38.1
On a Sunday in 2023, Laine Stump wrote:
The only reason we need it at build time is to find its location in $PATH so it can be hardcoded into the libvirt binary (and avoid the possibility of someone adding in a malicious binary somewhere earlier in the path, I guess).
Only 'recommend' passt during installation though, since it is not needed unless someone is actually using it.
There is no need to add in a build-time "WITH_PASST" option (IMO), since it adds very little to the size of the code - "PASST" (the path to the binary) will just be set to "passt", so if someone does manage to build and install passt on an older version of Fedora or RHEL, it will still work (as long as it's installed somewhere in the path).
Signed-off-by: Laine Stump <laine@redhat.com> --- libvirt.spec.in | 7 +++++++ 1 file changed, 7 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (5)
-
Daniel P. Berrangé -
Jiri Denemark -
Ján Tomko -
Laine Stump -
Stefano Brivio