Re: [PATCH v3] qemu: passt: add support for custom command line arguments

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(a)redhat.com&gt; 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/P... https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/V...

> --- > 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.