[PATCH v3] qemu: passt: add support for custom command line arguments
This adds support for custom command line arguments for the passt backend, similar to qemu:commandline. The feature allows passing additional arguments to the passt process for development and testing purposes. The implementation: - Adds a passt XML namespace for custom arguments - Properly taints the domain when custom args are used - Includes comprehensive test coverage - Adds documentation for the new feature Usage example: <interface type='user'> <backend type='passt' xmlns:passt='http://libvirt.org/schemas/domain/passt/1.0'> <passt:commandline> <passt:arg value='--debug'/> <passt:arg value='--verbose'/> </passt:commandline> </backend> </interface> Signed-off-by: Enrique Llorente <ellorent@redhat.com> --- v3: - Fix all test problems - Refactor domain_conf.c to use libvirt xml constructs to have proper indent - Rework documentation and make it more concise - Add domainpassttest.c to check that arguments are passed to passt docs/formatdomain.rst | 38 ++++ src/conf/domain_conf.c | 61 ++++++- src/conf/domain_conf.h | 6 + src/conf/schemas/domaincommon.rng | 15 ++ src/qemu/qemu_passt.c | 9 + tests/meson.build | 1 + tests/qemupassttest.c | 162 ++++++++++++++++++ ...-user-passt-custom-args.x86_64-latest.args | 35 ++++ ...t-user-passt-custom-args.x86_64-latest.xml | 67 ++++++++ .../net-user-passt-custom-args.xml | 64 +++++++ tests/qemuxmlconftest.c | 1 + 11 files changed, 458 insertions(+), 1 deletion(-) create mode 100644 tests/qemupassttest.c create mode 100644 tests/qemuxmlconfdata/net-user-passt-custom-args.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/net-user-passt-custom-args.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/net-user-passt-custom-args.xml diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 9a2f065590..4c01a07135 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -5464,6 +5464,44 @@ ports **with the exception of some subset**. </devices> ... +Custom passt commandline arguments +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +.. warning:: + + **This is an unsupported feature for development and testing only.** + Using it will taint the domain. Users are strongly advised to use the + proper libvirt XML elements for configuring passt instead. + + +:since:`Since 11.7.0` For development and testing purposes, it is +sometimes useful to be able to pass additional command-line arguments +directly to the passt process. This can be accomplished using a +special passt namespace in the domain XML that is similar to the qemu +commandline namespace: + +:: + + ... + <devices> + ... + <interface type='user'> + <backend type='passt'> + <passt:commandline xmlns:passt='http://libvirt.org/schemas/domain/passt/1.0'> + <passt:arg value='--debug'/> + <passt:arg value='--verbose'/> + </passt:commandline> + </backend> + </interface> + </devices> + ... + +Any arguments provided using this method will be appended to the passt +command line, and will therefore override any default options set by +libvirt in the case of conflicts. **This can lead to unexpected behavior +and libvirt cannot guarantee functionality when its default configuration +is overridden.** + Generic ethernet connection ^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1e24e41a48..9721763622 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2918,6 +2918,10 @@ virDomainNetDefFree(virDomainNetDef *def) g_free(def->backend.tap); g_free(def->backend.vhost); g_free(def->backend.logFile); + if (def->backend.passtCommandline) { + g_strfreev(def->backend.passtCommandline->args); + g_free(def->backend.passtCommandline); + } virDomainNetTeamingInfoFree(def->teaming); g_free(def->virtPortProfile); g_free(def->script); @@ -9772,6 +9776,7 @@ virDomainNetBackendParseXML(xmlNodePtr node, { g_autofree char *tap = virXMLPropString(node, "tap"); g_autofree char *vhost = virXMLPropString(node, "vhost"); + xmlNodePtr cur; /* In the case of NET_TYPE_USER, backend type can be unspecified * (i.e. VIR_DOMAIN_NET_BACKEND_DEFAULT) and that means 'use @@ -9808,6 +9813,38 @@ virDomainNetBackendParseXML(xmlNodePtr node, def->backend.vhost = virFileSanitizePath(vhost); } + /* Parse passt namespace commandline */ + cur = node->children; + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE) { + if (cur->ns && + STREQ((const char *)cur->ns->href, "http://libvirt.org/schemas/domain/passt/1.0") && + STREQ((const char *)cur->name, "commandline")) { + xmlNodePtr arg_node = cur->children; + g_autoptr(GPtrArray) args = g_ptr_array_new(); + + while (arg_node != NULL) { + if (arg_node->type == XML_ELEMENT_NODE && + arg_node->ns && + STREQ((const char *)arg_node->ns->href, "http://libvirt.org/schemas/domain/passt/1.0") && + STREQ((const char *)arg_node->name, "arg")) { + g_autofree char *value = virXMLPropString(arg_node, "value"); + if (value) + g_ptr_array_add(args, g_strdup(value)); + } + arg_node = arg_node->next; + } + + if (args->len > 0) { + def->backend.passtCommandline = g_new0(virDomainNetBackendPasstCommandline, 1); + g_ptr_array_add(args, NULL); /* NULL-terminate */ + def->backend.passtCommandline->args = (char **)g_ptr_array_steal(args, NULL); + } + } + } + cur = cur->next; + } + return 0; } @@ -20802,6 +20839,7 @@ virDomainNetBackendIsEqual(virDomainNetBackend *src, STRNEQ_NULLABLE(src->logFile, dst->logFile)) { return false; } + return true; } @@ -24921,11 +24959,29 @@ virDomainNetTeamingInfoFormat(virDomainNetTeamingInfo *teaming, } +static void +virDomainNetBackendPasstCommandLineFormat(virBuffer *buf, + virDomainNetBackend *backend) +{ + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + GStrv n; + + if (backend->passtCommandline && backend->passtCommandline->args) { + for (n = backend->passtCommandline->args; n && *n; n++) + virBufferEscapeString(&childBuf, "<passt:arg value='%s'/>\n", *n); + virBufferAddLit(&attrBuf, " xmlns:passt='http://libvirt.org/schemas/domain/passt/1.0'"); + virXMLFormatElement(buf, "passt:commandline", &attrBuf, &childBuf); + } + +} + static void virDomainNetBackendFormat(virBuffer *buf, virDomainNetBackend *backend) { g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); if (backend->type) { virBufferAsprintf(&attrBuf, " type='%s'", @@ -24934,7 +24990,10 @@ virDomainNetBackendFormat(virBuffer *buf, virBufferEscapeString(&attrBuf, " tap='%s'", backend->tap); virBufferEscapeString(&attrBuf, " vhost='%s'", backend->vhost); virBufferEscapeString(&attrBuf, " logFile='%s'", backend->logFile); - virXMLFormatElement(buf, "backend", &attrBuf, NULL); + + virDomainNetBackendPasstCommandLineFormat(&childBuf, backend); + + virXMLFormatElement(buf, "backend", &attrBuf, &childBuf); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6997cf7c09..1f51bad546 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1070,12 +1070,18 @@ struct _virDomainActualNetDef { unsigned int class_id; /* class ID for bandwidth 'floor' */ }; +typedef struct _virDomainNetBackendPasstCommandline virDomainNetBackendPasstCommandline; +struct _virDomainNetBackendPasstCommandline { + char **args; /* NULL-terminated array of arguments */ +}; + 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 */ + virDomainNetBackendPasstCommandline *passtCommandline; /* for passt overrides */ }; struct _virDomainNetPortForwardRange { diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 183dd5db5e..e176073c6a 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -3908,6 +3908,9 @@ </optional> <optional> <element name="backend"> + <optional> + <ref name="passtcmdline"/> + </optional> <optional> <attribute name="type"> <choice> @@ -8877,6 +8880,18 @@ </attribute> </define> + <define name="passtcmdline"> + <element name="commandline" ns="http://libvirt.org/schemas/domain/passt/1.0"> + <interleave> + <zeroOrMore> + <element name="arg"> + <attribute name="value"/> + </element> + </zeroOrMore> + </interleave> + </element> + </define> + <define name="coalesce"> <element name="coalesce"> <interleave> diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c index fcc34de384..0163553cee 100644 --- a/src/qemu/qemu_passt.c +++ b/src/qemu/qemu_passt.c @@ -317,6 +317,15 @@ qemuPasstStart(virDomainObj *vm, virCommandAddArg(cmd, virBufferCurrentContent(&buf)); } + /* Add custom passt arguments from namespace */ + if (net->backend.passtCommandline && net->backend.passtCommandline->args) { + for (i = 0; net->backend.passtCommandline->args[i]; i++) { + virCommandAddArg(cmd, net->backend.passtCommandline->args[i]); + } + + /* Taint the domain when using custom passt arguments */ + qemuDomainObjTaint(driver, vm, VIR_DOMAIN_TAINT_CUSTOM_ARGV, NULL); + } if (qemuExtDeviceLogCommand(driver, vm, cmd, "passt") < 0) return -1; diff --git a/tests/meson.build b/tests/meson.build index 0d76d37959..fe9013b600 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -269,6 +269,7 @@ tests += [ { 'name': 'networkxml2xmlupdatetest' }, { 'name': 'nodedevxml2xmltest' }, { 'name': 'nwfilterxml2xmltest' }, + { 'name': 'qemupassttest' }, { 'name': 'seclabeltest' }, { 'name': 'secretxml2xmltest' }, { 'name': 'sockettest' }, diff --git a/tests/qemupassttest.c b/tests/qemupassttest.c new file mode 100644 index 0000000000..84f4c1510a --- /dev/null +++ b/tests/qemupassttest.c @@ -0,0 +1,162 @@ +/* + * Copyright (C) 2024 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 "testutils.h" +#include "conf/domain_conf.h" +#include "viralloc.h" +#include "virstring.h" +#include "virlog.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +VIR_LOG_INIT("tests.qemupassttest"); + +struct testPasstData { + const char *name; + const char *xmlfile; + const char * const *expectedArgs; + size_t nExpectedArgs; + bool expectCustomArgs; +}; + +static virDomainDef * +testParseDomainXML(const char *xmlfile) +{ + g_autofree char *xmlpath = NULL; + g_autofree char *xmldata = NULL; + virDomainDef *def = NULL; + g_autoptr(virDomainXMLOption) xmlopt = NULL; + + xmlpath = g_strdup_printf("%s/qemuxmlconfdata/%s", abs_srcdir, xmlfile); + + if (virTestLoadFile(xmlpath, &xmldata) < 0) + return NULL; + + if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL, NULL))) + return NULL; + + def = virDomainDefParseString(xmldata, xmlopt, NULL, + VIR_DOMAIN_DEF_PARSE_INACTIVE); + + return def; +} + +static int +testPasstParseCustomArgs(const void *opaque) +{ + const struct testPasstData *data = opaque; + g_autoptr(virDomainDef) def = NULL; + virDomainNetDef *net = NULL; + size_t i; + + if (!(def = testParseDomainXML(data->xmlfile))) { + VIR_TEST_DEBUG("Failed to parse domain XML"); + return -1; + } + + /* Find the interface with passt backend */ + for (i = 0; i < def->nnets; i++) { + if (def->nets[i]->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) { + net = def->nets[i]; + break; + } + } + + if (!net) { + VIR_TEST_DEBUG("No passt interface found in domain XML"); + return -1; + } + + /* Check if we have custom arguments */ + if (data->expectCustomArgs) { + char **args; + + if (!net->backend.passtCommandline || !net->backend.passtCommandline->args) { + VIR_TEST_DEBUG("Expected custom args but none found"); + return -1; + } + + args = net->backend.passtCommandline->args; + + if (g_strv_length(args) != data->nExpectedArgs) { + VIR_TEST_DEBUG("Expected %zu arguments but found %u", + data->nExpectedArgs, g_strv_length(args)); + return -1; + } + + /* Verify all expected arguments are present */ + for (i = 0; i < data->nExpectedArgs; i++) { + if (!g_strv_contains((const char * const *)args, data->expectedArgs[i])) { + VIR_TEST_DEBUG("Missing expected argument: %s", data->expectedArgs[i]); + return -1; + } + } + } else { + /* Should not have custom arguments */ + if (net->backend.passtCommandline && + net->backend.passtCommandline->args && + *net->backend.passtCommandline->args) { + VIR_TEST_DEBUG("Found custom args but none expected"); + return -1; + } + } + + return 0; +} + +static int +mymain(void) +{ + int ret = 0; + + static const char * const customArgsExpected[] = { + "--debug", + "--verbose", + "--socket=/tmp/foo.socket" + }; + + struct testPasstData customArgsData = { + .name = "custom-args", + .xmlfile = "net-user-passt-custom-args.xml", + .expectedArgs = customArgsExpected, + .nExpectedArgs = G_N_ELEMENTS(customArgsExpected), + .expectCustomArgs = true, + }; + + struct testPasstData noCustomArgsData = { + .name = "no-custom-args", + .xmlfile = "net-user-passt.xml", + .expectedArgs = NULL, + .nExpectedArgs = 0, + .expectCustomArgs = false, + }; + + if (virTestRun("passt XML parsing with custom args", + testPasstParseCustomArgs, &customArgsData) < 0) + ret = -1; + + if (virTestRun("passt XML parsing without custom args", + testPasstParseCustomArgs, &noCustomArgsData) < 0) + ret = -1; + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIR_TEST_MAIN(mymain) diff --git a/tests/qemuxmlconfdata/net-user-passt-custom-args.x86_64-latest.args b/tests/qemuxmlconfdata/net-user-passt-custom-args.x86_64-latest.args new file mode 100644 index 0000000000..48d2596594 --- /dev/null +++ b/tests/qemuxmlconfdata/net-user-passt-custom-args.x86_64-latest.args @@ -0,0 +1,35 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \ +-accel tcg \ +-cpu qemu64 \ +-m size=219136k \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","read-only":false}' \ +-device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-storage","id":"ide0-0-0","bootindex":1}' \ +-netdev '{"type":"stream","addr":{"type":"unix","path":"/var/run/libvirt/qemu/passt/-1-QEMUGuest1-net0.socket"},"server":false,"reconnect-ms":5000,"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/qemuxmlconfdata/net-user-passt-custom-args.x86_64-latest.xml b/tests/qemuxmlconfdata/net-user-passt-custom-args.x86_64-latest.xml new file mode 100644 index 0000000000..6718893a52 --- /dev/null +++ b/tests/qemuxmlconfdata/net-user-passt-custom-args.x86_64-latest.xml @@ -0,0 +1,67 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0' model='none'/> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <interface type='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'> + <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'> + <passt:commandline xmlns:passt='http://libvirt.org/schemas/domain/passt/1.0'> + <passt:arg value='--debug'/> + <passt:arg value='--verbose'/> + <passt:arg value='--socket=/tmp/foo.socket'/> + </passt:commandline> + </backend> + <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> diff --git a/tests/qemuxmlconfdata/net-user-passt-custom-args.xml b/tests/qemuxmlconfdata/net-user-passt-custom-args.xml new file mode 100644 index 0000000000..a2a0f4c245 --- /dev/null +++ b/tests/qemuxmlconfdata/net-user-passt-custom-args.xml @@ -0,0 +1,64 @@ +<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'/> + <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'> + <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'> + <passt:commandline xmlns:passt='http://libvirt.org/schemas/domain/passt/1.0'> + <passt:arg value='--debug'/> + <passt:arg value='--verbose'/> + <passt:arg value='--socket=/tmp/foo.socket'/> + </passt:commandline> + </backend> + <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> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 9fba984290..839ae49ed4 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -1805,6 +1805,7 @@ mymain(void) DO_TEST_CAPS_LATEST("net-user-addr"); DO_TEST_CAPS_LATEST("net-user-passt"); DO_TEST_CAPS_VER("net-user-passt", "7.2.0"); + DO_TEST_CAPS_LATEST("net-user-passt-custom-args"); DO_TEST_CAPS_LATEST_PARSE_ERROR("net-user-slirp-portforward"); DO_TEST_CAPS_LATEST("net-vhostuser-passt"); DO_TEST_CAPS_LATEST_PARSE_ERROR("net-vhostuser-passt-no-shmem"); -- 2.50.0
On Wed, Jul 09, 2025 at 13:38:12 +0200, Enrique Llorente via Devel wrote:
This adds support for custom command line arguments for the passt backend, similar to qemu:commandline. The feature allows passing additional arguments to the passt process for development and testing purposes.
The implementation: - Adds a passt XML namespace for custom arguments - Properly taints the domain when custom args are used - Includes comprehensive test coverage - Adds documentation for the new feature
Usage example: <interface type='user'> <backend type='passt' xmlns:passt='http://libvirt.org/schemas/domain/passt/1.0'> <passt:commandline> <passt:arg value='--debug'/> <passt:arg value='--verbose'/> </passt:commandline> </backend> </interface>
Signed-off-by: Enrique Llorente <ellorent@redhat.com>
Based on the discussion on v2 of this patch, since you used AI to generate (at least parts of) this patch I don't think you can claim conformance with D-c-o. I'm also unwiling to jeopardize the project by knowingly allowing any form of code with unclear licensing so I will not be able to give R-b nor merge this patch. https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/PHRH... https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/VAAJ...
--- v3: - Fix all test problems - Refactor domain_conf.c to use libvirt xml constructs to have proper indent - Rework documentation and make it more concise - Add domainpassttest.c to check that arguments are passed to passt
docs/formatdomain.rst | 38 ++++ src/conf/domain_conf.c | 61 ++++++- src/conf/domain_conf.h | 6 + src/conf/schemas/domaincommon.rng | 15 ++ src/qemu/qemu_passt.c | 9 + tests/meson.build | 1 + tests/qemupassttest.c | 162 ++++++++++++++++++ ...-user-passt-custom-args.x86_64-latest.args | 35 ++++ ...t-user-passt-custom-args.x86_64-latest.xml | 67 ++++++++ .../net-user-passt-custom-args.xml | 64 +++++++ tests/qemuxmlconftest.c | 1 + 11 files changed, 458 insertions(+), 1 deletion(-) create mode 100644 tests/qemupassttest.c create mode 100644 tests/qemuxmlconfdata/net-user-passt-custom-args.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/net-user-passt-custom-args.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/net-user-passt-custom-args.xml
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 9a2f065590..4c01a07135 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -5464,6 +5464,44 @@ ports **with the exception of some subset**. </devices> ...
+Custom passt commandline arguments +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +.. warning::
Our "styling engine" doesn't support the warning tag yet. While it's possible to make this work with a bit of CSS that will certainly be a separate patch.
+ + **This is an unsupported feature for development and testing only.** + Using it will taint the domain. Users are strongly advised to use the + proper libvirt XML elements for configuring passt instead. + + +:since:`Since 11.7.0` For development and testing purposes, it is +sometimes useful to be able to pass additional command-line arguments +directly to the passt process. This can be accomplished using a +special passt namespace in the domain XML that is similar to the qemu +commandline namespace: + +:: + + ... + <devices> + ... + <interface type='user'> + <backend type='passt'> + <passt:commandline xmlns:passt='http://libvirt.org/schemas/domain/passt/1.0'> + <passt:arg value='--debug'/> + <passt:arg value='--verbose'/> + </passt:commandline> + </backend> + </interface> + </devices> + ... + +Any arguments provided using this method will be appended to the passt +command line, and will therefore override any default options set by +libvirt in the case of conflicts. **This can lead to unexpected behavior +and libvirt cannot guarantee functionality when its default configuration +is overridden.**
I thought about this and I think this docs should go into docs/drvqemu.rst in the sub-section where we generate other overrides. I don't think we want it in the main documentation
+ Generic ethernet connection ^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1e24e41a48..9721763622 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2918,6 +2918,10 @@ virDomainNetDefFree(virDomainNetDef *def) g_free(def->backend.tap); g_free(def->backend.vhost); g_free(def->backend.logFile); + if (def->backend.passtCommandline) { + g_strfreev(def->backend.passtCommandline->args); + g_free(def->backend.passtCommandline); + } virDomainNetTeamingInfoFree(def->teaming); g_free(def->virtPortProfile); g_free(def->script); @@ -9772,6 +9776,7 @@ virDomainNetBackendParseXML(xmlNodePtr node, { g_autofree char *tap = virXMLPropString(node, "tap"); g_autofree char *vhost = virXMLPropString(node, "vhost"); + xmlNodePtr cur;
/* In the case of NET_TYPE_USER, backend type can be unspecified * (i.e. VIR_DOMAIN_NET_BACKEND_DEFAULT) and that means 'use @@ -9808,6 +9813,38 @@ virDomainNetBackendParseXML(xmlNodePtr node, def->backend.vhost = virFileSanitizePath(vhost); }
+ /* Parse passt namespace commandline */ + cur = node->children; + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE) { + if (cur->ns && + STREQ((const char *)cur->ns->href, "http://libvirt.org/schemas/domain/passt/1.0") && + STREQ((const char *)cur->name, "commandline")) { + xmlNodePtr arg_node = cur->children; + g_autoptr(GPtrArray) args = g_ptr_array_new(); + + while (arg_node != NULL) { + if (arg_node->type == XML_ELEMENT_NODE && + arg_node->ns && + STREQ((const char *)arg_node->ns->href, "http://libvirt.org/schemas/domain/passt/1.0") && + STREQ((const char *)arg_node->name, "arg")) { + g_autofree char *value = virXMLPropString(arg_node, "value"); + if (value) + g_ptr_array_add(args, g_strdup(value)); + } + arg_node = arg_node->next; + } + + if (args->len > 0) { + def->backend.passtCommandline = g_new0(virDomainNetBackendPasstCommandline, 1); + g_ptr_array_add(args, NULL); /* NULL-terminate */ + def->backend.passtCommandline->args = (char **)g_ptr_array_steal(args, NULL); + } + } + } + cur = cur->next; + } + return 0; }
@@ -20802,6 +20839,7 @@ virDomainNetBackendIsEqual(virDomainNetBackend *src, STRNEQ_NULLABLE(src->logFile, dst->logFile)) { return false; } + return true;
Spurious whitespace change.
}
[...]
diff --git a/tests/qemupassttest.c b/tests/qemupassttest.c new file mode 100644 index 0000000000..84f4c1510a --- /dev/null +++ b/tests/qemupassttest.c @@ -0,0 +1,162 @@ +/* + * Copyright (C) 2024 Red Hat, Inc.
2025?
+ * + * 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 "testutils.h" +#include "conf/domain_conf.h" +#include "viralloc.h" +#include "virstring.h" +#include "virlog.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +VIR_LOG_INIT("tests.qemupassttest"); + +struct testPasstData { + const char *name; + const char *xmlfile; + const char * const *expectedArgs; + size_t nExpectedArgs; + bool expectCustomArgs; +}; + +static virDomainDef * +testParseDomainXML(const char *xmlfile) +{ + g_autofree char *xmlpath = NULL; + g_autofree char *xmldata = NULL; + virDomainDef *def = NULL; + g_autoptr(virDomainXMLOption) xmlopt = NULL; + + xmlpath = g_strdup_printf("%s/qemuxmlconfdata/%s", abs_srcdir, xmlfile); + + if (virTestLoadFile(xmlpath, &xmldata) < 0) + return NULL; + + if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL, NULL))) + return NULL; + + def = virDomainDefParseString(xmldata, xmlopt, NULL, + VIR_DOMAIN_DEF_PARSE_INACTIVE);
So this parses the definition ...
+ + return def; +} + +static int +testPasstParseCustomArgs(const void *opaque) +{ + const struct testPasstData *data = opaque; + g_autoptr(virDomainDef) def = NULL; + virDomainNetDef *net = NULL; + size_t i; + + if (!(def = testParseDomainXML(data->xmlfile))) { + VIR_TEST_DEBUG("Failed to parse domain XML"); + return -1; + } + + /* Find the interface with passt backend */ + for (i = 0; i < def->nnets; i++) { + if (def->nets[i]->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) { + net = def->nets[i]; + break; + } + } + + if (!net) { + VIR_TEST_DEBUG("No passt interface found in domain XML"); + return -1; + } + + /* Check if we have custom arguments */ + if (data->expectCustomArgs) { + char **args; + + if (!net->backend.passtCommandline || !net->backend.passtCommandline->args) { + VIR_TEST_DEBUG("Expected custom args but none found"); + return -1; + } + + args = net->backend.passtCommandline->args; + + if (g_strv_length(args) != data->nExpectedArgs) { + VIR_TEST_DEBUG("Expected %zu arguments but found %u", + data->nExpectedArgs, g_strv_length(args)); + return -1; + } + + /* Verify all expected arguments are present */ + for (i = 0; i < data->nExpectedArgs; i++) { + if (!g_strv_contains((const char * const *)args, data->expectedArgs[i])) { + VIR_TEST_DEBUG("Missing expected argument: %s", data->expectedArgs[i]); + return -1;
... and just checks if the parsr parsed these elements? The same is done by qemuxmlconftest which parses and formats back the XML. If the output contains them it's fine. I originally expected that the purpose of this test is to actually check the generated commandline.
On Thu, Jul 10, 2025 at 12:04 PM Peter Krempa <pkrempa@redhat.com> wrote:
On Wed, Jul 09, 2025 at 13:38:12 +0200, Enrique Llorente via Devel wrote:
This adds support for custom command line arguments for the passt backend, similar to qemu:commandline. The feature allows passing additional arguments to the passt process for development and testing purposes.
The implementation: - Adds a passt XML namespace for custom arguments - Properly taints the domain when custom args are used - Includes comprehensive test coverage - Adds documentation for the new feature
Usage example: <interface type='user'> <backend type='passt' xmlns:passt='http://libvirt.org/schemas/domain/passt/1.0'> <passt:commandline> <passt:arg value='--debug'/> <passt:arg value='--verbose'/> </passt:commandline> </backend> </interface>
Signed-off-by: Enrique Llorente <ellorent@redhat.com>
Based on the discussion on v2 of this patch, since you used AI to generate (at least parts of) this patch I don't think you can claim conformance with D-c-o.
I'm also unwiling to jeopardize the project by knowingly allowing any form of code with unclear licensing so I will not be able to give R-b nor merge this patch.
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/PHRH... https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/VAAJ...
Make sense, let's disregard this change, I will create a non assisted AI v4 and send it.
--- v3: - Fix all test problems - Refactor domain_conf.c to use libvirt xml constructs to have proper indent - Rework documentation and make it more concise - Add domainpassttest.c to check that arguments are passed to passt
docs/formatdomain.rst | 38 ++++ src/conf/domain_conf.c | 61 ++++++- src/conf/domain_conf.h | 6 + src/conf/schemas/domaincommon.rng | 15 ++ src/qemu/qemu_passt.c | 9 + tests/meson.build | 1 + tests/qemupassttest.c | 162 ++++++++++++++++++ ...-user-passt-custom-args.x86_64-latest.args | 35 ++++ ...t-user-passt-custom-args.x86_64-latest.xml | 67 ++++++++ .../net-user-passt-custom-args.xml | 64 +++++++ tests/qemuxmlconftest.c | 1 + 11 files changed, 458 insertions(+), 1 deletion(-) create mode 100644 tests/qemupassttest.c create mode 100644 tests/qemuxmlconfdata/net-user-passt-custom-args.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/net-user-passt-custom-args.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/net-user-passt-custom-args.xml
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 9a2f065590..4c01a07135 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -5464,6 +5464,44 @@ ports **with the exception of some subset**. </devices> ...
+Custom passt commandline arguments +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +.. warning::
Our "styling engine" doesn't support the warning tag yet.
While it's possible to make this work with a bit of CSS that will certainly be a separate patch.
+ + **This is an unsupported feature for development and testing only.** + Using it will taint the domain. Users are strongly advised to use the + proper libvirt XML elements for configuring passt instead. + + +:since:`Since 11.7.0` For development and testing purposes, it is +sometimes useful to be able to pass additional command-line arguments +directly to the passt process. This can be accomplished using a +special passt namespace in the domain XML that is similar to the qemu +commandline namespace: + +:: + + ... + <devices> + ... + <interface type='user'> + <backend type='passt'> + <passt:commandline xmlns:passt='http://libvirt.org/schemas/domain/passt/1.0'> + <passt:arg value='--debug'/> + <passt:arg value='--verbose'/> + </passt:commandline> + </backend> + </interface> + </devices> + ... + +Any arguments provided using this method will be appended to the passt +command line, and will therefore override any default options set by +libvirt in the case of conflicts. **This can lead to unexpected behavior +and libvirt cannot guarantee functionality when its default configuration +is overridden.**
I thought about this and I think this docs should go into docs/drvqemu.rst in the sub-section where we generate other overrides.
I don't think we want it in the main documentation
+ Generic ethernet connection ^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1e24e41a48..9721763622 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2918,6 +2918,10 @@ virDomainNetDefFree(virDomainNetDef *def) g_free(def->backend.tap); g_free(def->backend.vhost); g_free(def->backend.logFile); + if (def->backend.passtCommandline) { + g_strfreev(def->backend.passtCommandline->args); + g_free(def->backend.passtCommandline); + } virDomainNetTeamingInfoFree(def->teaming); g_free(def->virtPortProfile); g_free(def->script); @@ -9772,6 +9776,7 @@ virDomainNetBackendParseXML(xmlNodePtr node, { g_autofree char *tap = virXMLPropString(node, "tap"); g_autofree char *vhost = virXMLPropString(node, "vhost"); + xmlNodePtr cur;
/* In the case of NET_TYPE_USER, backend type can be unspecified * (i.e. VIR_DOMAIN_NET_BACKEND_DEFAULT) and that means 'use @@ -9808,6 +9813,38 @@ virDomainNetBackendParseXML(xmlNodePtr node, def->backend.vhost = virFileSanitizePath(vhost); }
+ /* Parse passt namespace commandline */ + cur = node->children; + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE) { + if (cur->ns && + STREQ((const char *)cur->ns->href, "http://libvirt.org/schemas/domain/passt/1.0") && + STREQ((const char *)cur->name, "commandline")) { + xmlNodePtr arg_node = cur->children; + g_autoptr(GPtrArray) args = g_ptr_array_new(); + + while (arg_node != NULL) { + if (arg_node->type == XML_ELEMENT_NODE && + arg_node->ns && + STREQ((const char *)arg_node->ns->href, "http://libvirt.org/schemas/domain/passt/1.0") && + STREQ((const char *)arg_node->name, "arg")) { + g_autofree char *value = virXMLPropString(arg_node, "value"); + if (value) + g_ptr_array_add(args, g_strdup(value)); + } + arg_node = arg_node->next; + } + + if (args->len > 0) { + def->backend.passtCommandline = g_new0(virDomainNetBackendPasstCommandline, 1); + g_ptr_array_add(args, NULL); /* NULL-terminate */ + def->backend.passtCommandline->args = (char **)g_ptr_array_steal(args, NULL); + } + } + } + cur = cur->next; + } + return 0; }
@@ -20802,6 +20839,7 @@ virDomainNetBackendIsEqual(virDomainNetBackend *src, STRNEQ_NULLABLE(src->logFile, dst->logFile)) { return false; } + return true;
Spurious whitespace change.
}
[...]
diff --git a/tests/qemupassttest.c b/tests/qemupassttest.c new file mode 100644 index 0000000000..84f4c1510a --- /dev/null +++ b/tests/qemupassttest.c @@ -0,0 +1,162 @@ +/* + * Copyright (C) 2024 Red Hat, Inc.
2025?
+ * + * 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 "testutils.h" +#include "conf/domain_conf.h" +#include "viralloc.h" +#include "virstring.h" +#include "virlog.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +VIR_LOG_INIT("tests.qemupassttest"); + +struct testPasstData { + const char *name; + const char *xmlfile; + const char * const *expectedArgs; + size_t nExpectedArgs; + bool expectCustomArgs; +}; + +static virDomainDef * +testParseDomainXML(const char *xmlfile) +{ + g_autofree char *xmlpath = NULL; + g_autofree char *xmldata = NULL; + virDomainDef *def = NULL; + g_autoptr(virDomainXMLOption) xmlopt = NULL; + + xmlpath = g_strdup_printf("%s/qemuxmlconfdata/%s", abs_srcdir, xmlfile); + + if (virTestLoadFile(xmlpath, &xmldata) < 0) + return NULL; + + if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL, NULL))) + return NULL; + + def = virDomainDefParseString(xmldata, xmlopt, NULL, + VIR_DOMAIN_DEF_PARSE_INACTIVE);
So this parses the definition ...
+ + return def; +} + +static int +testPasstParseCustomArgs(const void *opaque) +{ + const struct testPasstData *data = opaque; + g_autoptr(virDomainDef) def = NULL; + virDomainNetDef *net = NULL; + size_t i; + + if (!(def = testParseDomainXML(data->xmlfile))) { + VIR_TEST_DEBUG("Failed to parse domain XML"); + return -1; + } + + /* Find the interface with passt backend */ + for (i = 0; i < def->nnets; i++) { + if (def->nets[i]->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) { + net = def->nets[i]; + break; + } + } + + if (!net) { + VIR_TEST_DEBUG("No passt interface found in domain XML"); + return -1; + } + + /* Check if we have custom arguments */ + if (data->expectCustomArgs) { + char **args; + + if (!net->backend.passtCommandline || !net->backend.passtCommandline->args) { + VIR_TEST_DEBUG("Expected custom args but none found"); + return -1; + } + + args = net->backend.passtCommandline->args; + + if (g_strv_length(args) != data->nExpectedArgs) { + VIR_TEST_DEBUG("Expected %zu arguments but found %u", + data->nExpectedArgs, g_strv_length(args)); + return -1; + } + + /* Verify all expected arguments are present */ + for (i = 0; i < data->nExpectedArgs; i++) { + if (!g_strv_contains((const char * const *)args, data->expectedArgs[i])) { + VIR_TEST_DEBUG("Missing expected argument: %s", data->expectedArgs[i]); + return -1;
... and just checks if the parsr parsed these elements?
The same is done by qemuxmlconftest which parses and formats back the XML. If the output contains them it's fine.
I originally expected that the purpose of this test is to actually check the generated commandline.
-- Quique Llorente CNV networking Senior Software Engineer Red Hat EMEA ellorent@redhat.com @RedHat Red Hat Red Hat
participants (3)
-
Enrique Llorente -
Enrique Llorente Pastora
-
Peter Krempa