[libvirt PATCH 0/3] Remove <backend upstream='blah'/> per danpb's request, and a couple small cleanups
The first patch implements what danpb suggested yesterday - using <source dev='blah'/> instead of <backend upstream='blah'/> to populate the passt --interface option. The other two are small cleanups I noticed while making the changes in patch 1. jdenemar: feel free to push once they've been reviewed, rather than waiting for me to get up, so that you can get RC2 tagged. Laine Stump (3): conf: remove <backend upstream='xxx'/> attribute qemu: remove commented-out option in passt qemu commandline setup tests: remove unused qemu .args file docs/formatdomain.rst | 9 +++--- src/conf/domain_conf.c | 15 ++++++++-- src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 13 +++++---- src/qemu/qemu_passt.c | 5 ++-- tests/qemuxml2argvdata/net-user-passt.args | 34 ---------------------- tests/qemuxml2argvdata/net-user-passt.xml | 3 +- 7 files changed, 31 insertions(+), 49 deletions(-) delete mode 100644 tests/qemuxml2argvdata/net-user-passt.args -- 2.38.1
This attribute was added to support setting the --interface option for passt, but in a post-push/pre-9.0-release review, danpb pointed out that it would be better to use the existing <source dev='xxx'/> attribute to set --interface rather than creating a new attribute (in the wrong place). So we remove backend/upstream, and change the passt commandline creation to grab the name for --interface from source/dev. Signed-off-by: Laine Stump <laine@redhat.com> --- docs/formatdomain.rst | 9 +++++---- src/conf/domain_conf.c | 15 +++++++++++++-- src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 13 ++++++++----- src/qemu/qemu_passt.c | 4 ++-- tests/qemuxml2argvdata/net-user-passt.xml | 3 ++- 6 files changed, 31 insertions(+), 14 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 2c44f77ab6..490a954745 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -4821,9 +4821,9 @@ 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. +where to write its message log, and the ``<source>`` attribute ``dev`` +can tell it to use a particular host interface to derive the routes +given to the guest for forwarding traffic upstream. Additionally, when passt is used, multiple ``<portForward>`` elements can be added to forward incoming network traffic for the host to this @@ -4855,8 +4855,9 @@ ports **with the exception of some subset**. <devices> ... <interface type='user'> - <backend type='passt' logFile='/var/log/passt.log' upstream='eth0'/> + <backend type='passt' logFile='/var/log/passt.log'/> <mac address="00:11:22:33:44:55"/> + <source dev='eth0'/> <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'> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e43dee1a60..5379204f89 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9038,7 +9038,6 @@ virDomainNetBackendParseXML(xmlNodePtr node, } def->backend.logFile = virXMLPropString(node, "logFile"); - def->backend.upstream = virXMLPropString(node, "upstream"); if (tap) def->backend.tap = virFileSanitizePath(tap); @@ -9464,6 +9463,9 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, break; case VIR_DOMAIN_NET_TYPE_USER: + def->sourceDev = virXMLPropString(source_node, "dev"); + break; + case VIR_DOMAIN_NET_TYPE_NULL: case VIR_DOMAIN_NET_TYPE_LAST: break; @@ -23277,6 +23279,9 @@ virDomainActualNetDefContentsFormat(virBuffer *buf, return -1; } virBufferAsprintf(buf, " mode='%s'", mode); + } else if (actualType == VIR_DOMAIN_NET_TYPE_USER) { + + virBufferEscapeString(buf, " dev='%s'", def->sourceDev); } virBufferAddLit(buf, "/>\n"); @@ -23487,7 +23492,6 @@ virDomainNetBackendFormat(virBuffer *buf, virBufferEscapeString(&attrBuf, " tap='%s'", backend->tap); virBufferEscapeString(&attrBuf, " vhost='%s'", backend->vhost); virBufferEscapeString(&attrBuf, " logFile='%s'", backend->logFile); - virBufferEscapeString(&attrBuf, " upstream='%s'", backend->upstream); virXMLFormatElement(buf, "backend", &attrBuf, NULL); } @@ -23752,6 +23756,13 @@ virDomainNetDefFormat(virBuffer *buf, } case VIR_DOMAIN_NET_TYPE_USER: + if (def->backend.type == VIR_DOMAIN_NET_BACKEND_PASST && + def->sourceDev) { + virBufferEscapeString(buf, "<source dev='%s'", def->sourceDev); + sourceLines++; + } + break; + case VIR_DOMAIN_NET_TYPE_NULL: case VIR_DOMAIN_NET_TYPE_LAST: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 60fc7c4eb2..3e4985a67d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1192,6 +1192,7 @@ struct _virDomainNetDef { virNetDevIPInfo hostIP; char *ifname_guest_actual; char *ifname_guest; + char *sourceDev; virNetDevIPInfo guestIP; size_t nPortForwards; virDomainNetPortForward **portForwards; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index bd2e789327..6cb0a20e1e 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -3318,6 +3318,14 @@ <value>user</value> </attribute> <interleave> + <optional> + <element name="source"> + <attribute name="dev"> + <ref name="deviceName"/> + </attribute> + <empty/> + </element> + </optional> <ref name="interface-options"/> </interleave> </group> @@ -3643,11 +3651,6 @@ <ref name="absFilePath"/> </attribute> </optional> - <optional> - <attribute name="upstream"> - <ref name="deviceName"/> - </attribute> - </optional> </element> </optional> <optional> diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c index a7ee841af7..b1215a1bba 100644 --- a/src/qemu/qemu_passt.c +++ b/src/qemu/qemu_passt.c @@ -167,8 +167,8 @@ qemuPasstStart(virDomainObj *vm, virCommandAddArgFormat(cmd, "%u", net->mtu); } - if (net->backend.upstream) - virCommandAddArgList(cmd, "--interface", net->backend.upstream, NULL); + if (net->sourceDev) + virCommandAddArgList(cmd, "--interface", net->sourceDev, NULL); if (net->backend.logFile) virCommandAddArgList(cmd, "--log-file", net->backend.logFile, NULL); diff --git a/tests/qemuxml2argvdata/net-user-passt.xml b/tests/qemuxml2argvdata/net-user-passt.xml index b82eebd089..20c9f50542 100644 --- a/tests/qemuxml2argvdata/net-user-passt.xml +++ b/tests/qemuxml2argvdata/net-user-passt.xml @@ -27,6 +27,7 @@ <controller type='pci' index='0' model='pci-root'/> <interface type='user'> <mac address='00:11:22:33:44:55'/> + <source dev='eth42'/> <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'> @@ -46,7 +47,7 @@ <range start='443' to='344'/> </portForward> <model type='rtl8139'/> - <backend type='passt' logFile='/var/log/loglaw.blog' upstream='eth42'/> + <backend type='passt' logFile='/var/log/loglaw.blog'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </interface> <input type='mouse' bus='ps2'/> -- 2.38.1
On Thu, Jan 12, 2023 at 23:42:16 -0500, Laine Stump wrote:
This attribute was added to support setting the --interface option for passt, but in a post-push/pre-9.0-release review, danpb pointed out that it would be better to use the existing <source dev='xxx'/> attribute to set --interface rather than creating a new attribute (in the wrong place). So we remove backend/upstream, and change the passt commandline creation to grab the name for --interface from source/dev.
Signed-off-by: Laine Stump <laine@redhat.com> --- docs/formatdomain.rst | 9 +++++---- src/conf/domain_conf.c | 15 +++++++++++++-- src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 13 ++++++++----- src/qemu/qemu_passt.c | 4 ++-- tests/qemuxml2argvdata/net-user-passt.xml | 3 ++- 6 files changed, 31 insertions(+), 14 deletions(-)
...
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e43dee1a60..5379204f89 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9038,7 +9038,6 @@ virDomainNetBackendParseXML(xmlNodePtr node, }
def->backend.logFile = virXMLPropString(node, "logFile"); - def->backend.upstream = virXMLPropString(node, "upstream");
if (tap) def->backend.tap = virFileSanitizePath(tap); @@ -9464,6 +9463,9 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, break;
case VIR_DOMAIN_NET_TYPE_USER: + def->sourceDev = virXMLPropString(source_node, "dev"); + break; + case VIR_DOMAIN_NET_TYPE_NULL: case VIR_DOMAIN_NET_TYPE_LAST: break; @@ -23277,6 +23279,9 @@ virDomainActualNetDefContentsFormat(virBuffer *buf, return -1; } virBufferAsprintf(buf, " mode='%s'", mode); + } else if (actualType == VIR_DOMAIN_NET_TYPE_USER) { + + virBufferEscapeString(buf, " dev='%s'", def->sourceDev); }
Extra empty line.
virBufferAddLit(buf, "/>\n");
...
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 60fc7c4eb2..3e4985a67d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1192,6 +1192,7 @@ struct _virDomainNetDef { virNetDevIPInfo hostIP; char *ifname_guest_actual; char *ifname_guest; + char *sourceDev; virNetDevIPInfo guestIP; size_t nPortForwards; virDomainNetPortForward **portForwards;
The pointer stored in sourceDev is not freed in virDomainNetDefFree. Jirka
This commented-out option was pointed out by jtomko during review, but I missed taking it out when addressing his comments. Signed-off-by: Laine Stump <laine@redhat.com> --- src/qemu/qemu_passt.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c index b1215a1bba..0f09bf3db8 100644 --- a/src/qemu/qemu_passt.c +++ b/src/qemu/qemu_passt.c @@ -95,7 +95,6 @@ qemuPasstAddNetProps(virDomainObj *vm, "s:type", "stream", "a:addr", &addrprops, "b:server", false, - /* "u:reconnect", 5, */ NULL) < 0) { return -1; } -- 2.38.1
net-user-passt.args was generated early during testing of the passt qemu commandline, when qemuxml2argvtest was using DO_TEST("net-user-passt"). This was later changed to DO_TEST_CAPS_LATEST(), so the file net-user-passt.x86_64-latest.args is used instead, but the original (now unused) test file was accidentally added to the original patch. This patch removes it. Signed-off-by: Laine Stump <laine@redhat.com> --- tests/qemuxml2argvdata/net-user-passt.args | 34 ---------------------- 1 file changed, 34 deletions(-) delete mode 100644 tests/qemuxml2argvdata/net-user-passt.args diff --git a/tests/qemuxml2argvdata/net-user-passt.args b/tests/qemuxml2argvdata/net-user-passt.args deleted file mode 100644 index 8c887ca210..0000000000 --- a/tests/qemuxml2argvdata/net-user-passt.args +++ /dev/null @@ -1,34 +0,0 @@ -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 -- 2.38.1
On Thu, Jan 12, 2023 at 23:42:15 -0500, Laine Stump wrote:
The first patch implements what danpb suggested yesterday - using <source dev='blah'/> instead of <backend upstream='blah'/> to populate the passt --interface option. The other two are small cleanups I noticed while making the changes in patch 1.
jdenemar: feel free to push once they've been reviewed, rather than waiting for me to get up, so that you can get RC2 tagged.
Laine Stump (3): conf: remove <backend upstream='xxx'/> attribute qemu: remove commented-out option in passt qemu commandline setup tests: remove unused qemu .args file
docs/formatdomain.rst | 9 +++--- src/conf/domain_conf.c | 15 ++++++++-- src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 13 +++++---- src/qemu/qemu_passt.c | 5 ++-- tests/qemuxml2argvdata/net-user-passt.args | 34 ---------------------- tests/qemuxml2argvdata/net-user-passt.xml | 3 +- 7 files changed, 31 insertions(+), 49 deletions(-) delete mode 100644 tests/qemuxml2argvdata/net-user-passt.args
I fixed the two small issues in 1/3 and pushed this series. Thanks. Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
participants (2)
-
Jiri Denemark -
Laine Stump