* [libvirt PATCH 1/9] conf: rename virDomainNetBackend* to virDomainNetDriver*
2023-01-09 4:11 [libvirt PATCH 0/9] Support libvirt-managed QEMU domain <interface> backed by a passt process Laine Stump
@ 2023-01-09 4:11 ` Laine Stump
2023-01-09 5:40 ` Ján Tomko
2023-01-09 4:11 ` [libvirt PATCH 2/9] conf: move anonymous backend struct from virDomainNetDef into its own struct Laine Stump
` (7 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Laine Stump @ 2023-01-09 4:11 UTC (permalink / raw)
To: libvir-list; +Cc: sbrivio, passt-dev
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;
--
@@ -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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [libvirt PATCH 1/9] conf: rename virDomainNetBackend* to virDomainNetDriver*
2023-01-09 4:11 ` [libvirt PATCH 1/9] conf: rename virDomainNetBackend* to virDomainNetDriver* Laine Stump
@ 2023-01-09 5:40 ` Ján Tomko
0 siblings, 0 replies; 28+ messages in thread
From: Ján Tomko @ 2023-01-09 5:40 UTC (permalink / raw)
To: Laine Stump; +Cc: libvir-list, sbrivio, passt-dev
[-- Attachment #1: Type: text/plain, Size: 1940 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* [libvirt PATCH 2/9] conf: move anonymous backend struct from virDomainNetDef into its own struct
2023-01-09 4:11 [libvirt PATCH 0/9] Support libvirt-managed QEMU domain <interface> backed by a passt process Laine Stump
2023-01-09 4:11 ` [libvirt PATCH 1/9] conf: rename virDomainNetBackend* to virDomainNetDriver* Laine Stump
@ 2023-01-09 4:11 ` Laine Stump
2023-01-09 5:41 ` Ján Tomko
2023-01-09 4:11 ` [libvirt PATCH 3/9] conf: put interface <backend> parsing/formatting separate functions Laine Stump
` (6 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Laine Stump @ 2023-01-09 4:11 UTC (permalink / raw)
To: libvir-list; +Cc: sbrivio, passt-dev
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;
--
@@ -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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [libvirt PATCH 3/9] conf: put interface <backend> parsing/formatting separate functions
2023-01-09 4:11 [libvirt PATCH 0/9] Support libvirt-managed QEMU domain <interface> backed by a passt process Laine Stump
2023-01-09 4:11 ` [libvirt PATCH 1/9] conf: rename virDomainNetBackend* to virDomainNetDriver* Laine Stump
2023-01-09 4:11 ` [libvirt PATCH 2/9] conf: move anonymous backend struct from virDomainNetDef into its own struct Laine Stump
@ 2023-01-09 4:11 ` Laine Stump
2023-01-09 5:47 ` Ján Tomko
2023-01-09 4:11 ` [libvirt PATCH 4/9] conf: add passt XML additions to schema Laine Stump
` (5 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Laine Stump @ 2023-01-09 4:11 UTC (permalink / raw)
To: libvir-list; +Cc: sbrivio, passt-dev
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)
--
@@ -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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [libvirt PATCH 3/9] conf: put interface <backend> parsing/formatting separate functions
2023-01-09 4:11 ` [libvirt PATCH 3/9] conf: put interface <backend> parsing/formatting separate functions Laine Stump
@ 2023-01-09 5:47 ` Ján Tomko
2023-01-09 7:04 ` Laine Stump
0 siblings, 1 reply; 28+ messages in thread
From: Ján Tomko @ 2023-01-09 5:47 UTC (permalink / raw)
To: Laine Stump; +Cc: libvir-list, sbrivio, passt-dev
[-- Attachment #1: Type: text/plain, Size: 3250 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [libvirt PATCH 3/9] conf: put interface <backend> parsing/formatting separate functions
2023-01-09 5:47 ` Ján Tomko
@ 2023-01-09 7:04 ` Laine Stump
0 siblings, 0 replies; 28+ messages in thread
From: Laine Stump @ 2023-01-09 7:04 UTC (permalink / raw)
To: Libvirt; +Cc: sbrivio, passt-dev, Ján Tomko
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* [libvirt PATCH 4/9] conf: add passt XML additions to schema
2023-01-09 4:11 [libvirt PATCH 0/9] Support libvirt-managed QEMU domain <interface> backed by a passt process Laine Stump
` (2 preceding siblings ...)
2023-01-09 4:11 ` [libvirt PATCH 3/9] conf: put interface <backend> parsing/formatting separate functions Laine Stump
@ 2023-01-09 4:11 ` Laine Stump
2023-01-09 5:48 ` Ján Tomko
2023-01-11 18:33 ` Daniel P. Berrangé
2023-01-09 4:11 ` [libvirt PATCH 5/9] conf: parse/format passt-related XML additions Laine Stump
` (4 subsequent siblings)
8 siblings, 2 replies; 28+ messages in thread
From: Laine Stump @ 2023-01-09 4:11 UTC (permalink / raw)
To: libvir-list; +Cc: sbrivio, passt-dev
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>
--
@@ -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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [libvirt PATCH 4/9] conf: add passt XML additions to schema
2023-01-09 4:11 ` [libvirt PATCH 4/9] conf: add passt XML additions to schema Laine Stump
@ 2023-01-09 5:48 ` Ján Tomko
2023-01-11 18:33 ` Daniel P. Berrangé
1 sibling, 0 replies; 28+ messages in thread
From: Ján Tomko @ 2023-01-09 5:48 UTC (permalink / raw)
To: Laine Stump; +Cc: libvir-list, sbrivio, passt-dev
[-- Attachment #1: Type: text/plain, Size: 907 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [libvirt PATCH 4/9] conf: add passt XML additions to schema
2023-01-09 4:11 ` [libvirt PATCH 4/9] conf: add passt XML additions to schema Laine Stump
2023-01-09 5:48 ` Ján Tomko
@ 2023-01-11 18:33 ` Daniel P. Berrangé
2023-01-12 14:45 ` Laine Stump
1 sibling, 1 reply; 28+ messages in thread
From: Daniel P. Berrangé @ 2023-01-11 18:33 UTC (permalink / raw)
To: Laine Stump; +Cc: libvir-list, sbrivio, passt-dev
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 :|
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [libvirt PATCH 4/9] conf: add passt XML additions to schema
2023-01-11 18:33 ` Daniel P. Berrangé
@ 2023-01-12 14:45 ` Laine Stump
2023-01-12 17:28 ` Stefano Brivio
2023-01-12 18:12 ` Jiri Denemark
0 siblings, 2 replies; 28+ messages in thread
From: Laine Stump @ 2023-01-12 14:45 UTC (permalink / raw)
To: Libvirt; +Cc: sbrivio, Daniel P. Berrangé, passt-dev
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [libvirt PATCH 4/9] conf: add passt XML additions to schema
2023-01-12 14:45 ` Laine Stump
@ 2023-01-12 17:28 ` Stefano Brivio
2023-01-12 18:12 ` Jiri Denemark
1 sibling, 0 replies; 28+ messages in thread
From: Stefano Brivio @ 2023-01-12 17:28 UTC (permalink / raw)
To: Laine Stump; +Cc: Libvirt, Daniel P. Berrangé, passt-dev
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [libvirt PATCH 4/9] conf: add passt XML additions to schema
2023-01-12 14:45 ` Laine Stump
2023-01-12 17:28 ` Stefano Brivio
@ 2023-01-12 18:12 ` Jiri Denemark
1 sibling, 0 replies; 28+ messages in thread
From: Jiri Denemark @ 2023-01-12 18:12 UTC (permalink / raw)
To: Laine Stump; +Cc: Libvirt, sbrivio, passt-dev
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* [libvirt PATCH 5/9] conf: parse/format passt-related XML additions
2023-01-09 4:11 [libvirt PATCH 0/9] Support libvirt-managed QEMU domain <interface> backed by a passt process Laine Stump
` (3 preceding siblings ...)
2023-01-09 4:11 ` [libvirt PATCH 4/9] conf: add passt XML additions to schema Laine Stump
@ 2023-01-09 4:11 ` Laine Stump
2023-01-09 6:18 ` Ján Tomko
2023-01-09 4:11 ` [libvirt PATCH 6/9] qemu: new capability QEMU_CAPS_NETDEV_STREAM Laine Stump
` (3 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Laine Stump @ 2023-01-09 4:11 UTC (permalink / raw)
To: libvir-list; +Cc: sbrivio, passt-dev
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");
--
@@ -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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [libvirt PATCH 5/9] conf: parse/format passt-related XML additions
2023-01-09 4:11 ` [libvirt PATCH 5/9] conf: parse/format passt-related XML additions Laine Stump
@ 2023-01-09 6:18 ` Ján Tomko
0 siblings, 0 replies; 28+ messages in thread
From: Ján Tomko @ 2023-01-09 6:18 UTC (permalink / raw)
To: Laine Stump; +Cc: libvir-list, sbrivio, passt-dev
[-- Attachment #1: Type: text/plain, Size: 4624 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* [libvirt PATCH 6/9] qemu: new capability QEMU_CAPS_NETDEV_STREAM
2023-01-09 4:11 [libvirt PATCH 0/9] Support libvirt-managed QEMU domain <interface> backed by a passt process Laine Stump
` (4 preceding siblings ...)
2023-01-09 4:11 ` [libvirt PATCH 5/9] conf: parse/format passt-related XML additions Laine Stump
@ 2023-01-09 4:11 ` Laine Stump
2023-01-09 6:20 ` Ján Tomko
2023-01-09 4:11 ` [libvirt PATCH 7/9] qemu: add passtStateDir to qemu driver config Laine Stump
` (2 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Laine Stump @ 2023-01-09 4:11 UTC (permalink / raw)
To: libvir-list; +Cc: sbrivio, passt-dev
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>
--
@@ -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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [libvirt PATCH 7/9] qemu: add passtStateDir to qemu driver config
2023-01-09 4:11 [libvirt PATCH 0/9] Support libvirt-managed QEMU domain <interface> backed by a passt process Laine Stump
` (5 preceding siblings ...)
2023-01-09 4:11 ` [libvirt PATCH 6/9] qemu: new capability QEMU_CAPS_NETDEV_STREAM Laine Stump
@ 2023-01-09 4:11 ` Laine Stump
2023-01-09 6:23 ` Ján Tomko
2023-01-09 4:11 ` [libvirt PATCH 8/9] qemu: hook up passt config to qemu domains Laine Stump
2023-01-09 4:11 ` [libvirt PATCH 9/9] specfile: require passt for the build if fedora >= 36 or rhel >= 9 Laine Stump
8 siblings, 1 reply; 28+ messages in thread
From: Laine Stump @ 2023-01-09 4:11 UTC (permalink / raw)
To: libvir-list; +Cc: sbrivio, passt-dev
--
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;
--
@@ -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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [libvirt PATCH 7/9] qemu: add passtStateDir to qemu driver config
2023-01-09 4:11 ` [libvirt PATCH 7/9] qemu: add passtStateDir to qemu driver config Laine Stump
@ 2023-01-09 6:23 ` Ján Tomko
2023-01-09 14:02 ` Laine Stump
0 siblings, 1 reply; 28+ messages in thread
From: Ján Tomko @ 2023-01-09 6:23 UTC (permalink / raw)
To: Laine Stump; +Cc: libvir-list, sbrivio, passt-dev
[-- Attachment #1: Type: text/plain, Size: 582 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [libvirt PATCH 7/9] qemu: add passtStateDir to qemu driver config
2023-01-09 6:23 ` Ján Tomko
@ 2023-01-09 14:02 ` Laine Stump
0 siblings, 0 replies; 28+ messages in thread
From: Laine Stump @ 2023-01-09 14:02 UTC (permalink / raw)
To: Libvirt; +Cc: sbrivio, passt-dev, Ján Tomko
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* [libvirt PATCH 8/9] qemu: hook up passt config to qemu domains
2023-01-09 4:11 [libvirt PATCH 0/9] Support libvirt-managed QEMU domain <interface> backed by a passt process Laine Stump
` (6 preceding siblings ...)
2023-01-09 4:11 ` [libvirt PATCH 7/9] qemu: add passtStateDir to qemu driver config Laine Stump
@ 2023-01-09 4:11 ` Laine Stump
2023-01-09 6:31 ` Ján Tomko
2023-01-09 4:11 ` [libvirt PATCH 9/9] specfile: require passt for the build if fedora >= 36 or rhel >= 9 Laine Stump
8 siblings, 1 reply; 28+ messages in thread
From: Laine Stump @ 2023-01-09 4:11 UTC (permalink / raw)
To: libvir-list; +Cc: sbrivio, passt-dev
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");
--
@@ -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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [libvirt PATCH 8/9] qemu: hook up passt config to qemu domains
2023-01-09 4:11 ` [libvirt PATCH 8/9] qemu: hook up passt config to qemu domains Laine Stump
@ 2023-01-09 6:31 ` Ján Tomko
2023-01-09 14:14 ` Laine Stump
0 siblings, 1 reply; 28+ messages in thread
From: Ján Tomko @ 2023-01-09 6:31 UTC (permalink / raw)
To: Laine Stump; +Cc: libvir-list, sbrivio, passt-dev
[-- Attachment #1: Type: text/plain, Size: 5070 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [libvirt PATCH 8/9] qemu: hook up passt config to qemu domains
2023-01-09 6:31 ` Ján Tomko
@ 2023-01-09 14:14 ` Laine Stump
2023-01-09 14:51 ` Ján Tomko
0 siblings, 1 reply; 28+ messages in thread
From: Laine Stump @ 2023-01-09 14:14 UTC (permalink / raw)
To: Libvirt; +Cc: sbrivio, passt-dev, Ján Tomko
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [libvirt PATCH 8/9] qemu: hook up passt config to qemu domains
2023-01-09 14:14 ` Laine Stump
@ 2023-01-09 14:51 ` Ján Tomko
2023-01-09 16:05 ` Laine Stump
0 siblings, 1 reply; 28+ messages in thread
From: Ján Tomko @ 2023-01-09 14:51 UTC (permalink / raw)
To: Laine Stump; +Cc: Libvirt, sbrivio, passt-dev
[-- Attachment #1: Type: text/plain, Size: 1682 bytes --]
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);
>>>+}
>>>+
>>>+
[0] https://bugzilla.redhat.com/show_bug.cgi?id=2151808
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [libvirt PATCH 8/9] qemu: hook up passt config to qemu domains
2023-01-09 14:51 ` Ján Tomko
@ 2023-01-09 16:05 ` Laine Stump
0 siblings, 0 replies; 28+ messages in thread
From: Laine Stump @ 2023-01-09 16:05 UTC (permalink / raw)
To: Libvirt; +Cc: sbrivio, passt-dev, Ján Tomko
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...
^ permalink raw reply [flat|nested] 28+ messages in thread
* [libvirt PATCH 9/9] specfile: require passt for the build if fedora >= 36 or rhel >= 9
2023-01-09 4:11 [libvirt PATCH 0/9] Support libvirt-managed QEMU domain <interface> backed by a passt process Laine Stump
` (7 preceding siblings ...)
2023-01-09 4:11 ` [libvirt PATCH 8/9] qemu: hook up passt config to qemu domains Laine Stump
@ 2023-01-09 4:11 ` Laine Stump
2023-01-09 6:32 ` Ján Tomko
8 siblings, 1 reply; 28+ messages in thread
From: Laine Stump @ 2023-01-09 4:11 UTC (permalink / raw)
To: libvir-list; +Cc: sbrivio, passt-dev
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/
--
@@ -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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [libvirt PATCH 9/9] specfile: require passt for the build if fedora >= 36 or rhel >= 9
2023-01-09 4:11 ` [libvirt PATCH 9/9] specfile: require passt for the build if fedora >= 36 or rhel >= 9 Laine Stump
@ 2023-01-09 6:32 ` Ján Tomko
0 siblings, 0 replies; 28+ messages in thread
From: Ján Tomko @ 2023-01-09 6:32 UTC (permalink / raw)
To: Laine Stump; +Cc: libvir-list, sbrivio, passt-dev
[-- Attachment #1: Type: text/plain, Size: 932 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread