On Thu, Jul 10, 2025 at 12:04 PM Peter Krempa <pkre...@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 <ellor...@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/PHRHCEDTRDHSR3MY6YWPD3J3NC47LHAI/ > https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/VAAJFYGHEK4CTS6FOFEWBCDAAUIZYIFT/
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 ellor...@redhat.com @RedHat Red Hat Red Hat
