On Mon, Jul 07, 2025 at 14:52:09 +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>
> ---
> docs/formatdomain.rst | 40 +++++++++
> src/conf/domain_conf.c | 84 ++++++++++++++++++-
> src/conf/domain_conf.h | 6 ++
> src/qemu/qemu_passt.c | 9 ++
> ...-user-passt-custom-args.x86_64-latest.args | 39 +++++++++
> ...t-user-passt-custom-args.x86_64-latest.xml | 57 +++++++++++++
> .../net-user-passt-custom-args.xml | 52 ++++++++++++
> tests/qemuxmlconftest.c | 1 +
> 8 files changed, 287 insertions(+), 1 deletion(-)
> 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
$ git apply ~/.am
Applying: qemu: passt: add support for custom command line arguments
.git/rebase-apply/patch:97: trailing whitespace.
if (cur->ns &&
.git/rebase-apply/patch:102: trailing whitespace.
.git/rebase-apply/patch:114: trailing whitespace.
.git/rebase-apply/patch:142: trailing whitespace.
.git/rebase-apply/patch:182: trailing whitespace.
warning: squelched 3 whitespace errors
warning: 8 lines add whitespace errors.
Following test failures happen also when this patch is applied:
202/310 libvirt:bin / virschematest
FAIL 1.79s exit status 1
203/310 libvirt:syntax-check / prohibit_loop_var_decl
FAIL 0.20s exit status 2
222/310 libvirt:syntax-check / prohibit_int_ijk
FAIL 0.63s exit status 2
287/310 libvirt:syntax-check / prohibit_empty_lines_at_EOF
FAIL 0.26s exit status 2
299/310 libvirt:syntax-check / trailing_blank
FAIL 0.48s exit status 2
308/310 libvirt:bin / qemuxmlconftest
FAIL 4.01s exit status 1
In few cases it's coding style/whitespace errors, but virschematest
shows that you didn't update the XML schemas and qemuxmlconftest fails
as it mentions wrong emulator (one that isn't present in our existing
capabilities dumps).
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 9a2f065590..59899aaa4a 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -5464,6 +5464,46 @@ ports **with the exception of some subset**.
> </devices>
> ...
>
> +Custom passt commandline arguments
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +: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:
So if we ever want to document this in formatdomain.rst it must start
with the disclaimer that it's unsupported from our point of view even
before any example.
> +
> +::
> +
> + ...
> + <devices>
> + ...
> + <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>
> + </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.
What libvirt does is that it appends everything to the existing
commandline. If anything is overriden it's 'passt's interpretation.
Do you want to document it that way?
> +
> +**This feature is intended for development and testing only.**
> +Arguments are used as specified in the XML, and no validation beyond
> +basic libvirt XML schema validation is performed. It is expected that
XML schema validation is optional.
> +this feature will primarily be used for testing new passt options, so
> +that they can be evaluated for inclusion in libvirt's schema in a
> +future release. It should not be used as a substitute for configuring
> +passt with the proper libvirt XML elements and attributes. **When a
> +domain uses this feature, it will be marked as "tainted" in the logs and
> +the ``virsh dominfo`` output will include "custom-argv" in the list
> +of reasons.**
Missing that this is not supported and one must not file any bugs if
this argument is in use.
> +
> Generic ethernet connection
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 1e24e41a48..9d39bb39bd 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,40 @@ 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;
> + GPtrArray *args = g_ptr_array_new();
Use autoptr here?
> +
> + 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_free(args, FALSE);
libvirt doesn't use the uppercase boolean definitions.
> + } else {
> + g_ptr_array_unref(args);
so that this is not needed?
> + }
> + }
> + }
> + cur = cur->next;
> + }
> +
> return 0;
> }
>
> @@ -20802,6 +20841,33 @@ virDomainNetBackendIsEqual(virDomainNetBackend *src,
> STRNEQ_NULLABLE(src->logFile, dst->logFile)) {
> return false;
> }
> +
> + /* Compare passt commandline */
> + if ((src->passtCommandline && dst->passtCommandline) ||
> + (!src->passtCommandline && !dst->passtCommandline)) {
> + if (src->passtCommandline && dst->passtCommandline) {
> + /* Compare argument arrays */
> + char **srcArgs = src->passtCommandline->args;
> + char **dstArgs = dst->passtCommandline->args;
> +
> + if (!srcArgs && !dstArgs) {
> + /* Both are NULL, continue */
> + } else if (!srcArgs || !dstArgs) {
> + return false; /* One is NULL, other is not */
> + } else {
> + /* Compare each argument */
> + int i = 0;
> + while (srcArgs[i] || dstArgs[i]) {
> + if (!srcArgs[i] || !dstArgs[i] || STRNEQ(srcArgs[i],
> dstArgs[i]))
> + return false;
> + i++;
> + }
> + }
> + }
> + } else {
> + return false;
> + }
I don't think this comparison should be done. This is for libvirt-known
parameters.
> +
> return true;
> }
>
> @@ -24926,6 +24992,7 @@ virDomainNetBackendFormat(virBuffer *buf,
> virDomainNetBackend *backend)
> {
> g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
> + g_auto(virBuffer) childBuf = VIR_BUFFER_INITIALIZER;
>
> if (backend->type) {
> virBufferAsprintf(&attrBuf, " type='%s'",
> @@ -24934,7 +25001,22 @@ 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);
> +
> + /* Format passt commandline with namespace */
> + if (backend->passtCommandline && backend->passtCommandline->args) {
> + virBufferAddLit(&childBuf, "<passt:commandline
> xmlns:passt='http://libvirt.org/schemas/domain/passt/1.0'>\n");
> + virBufferAdjustIndent(&childBuf, 2);
> +
> + for (int i = 0; backend->passtCommandline->args[i]; i++) {
> + virBufferAsprintf(&childBuf, "<passt:arg value='%s'/>\n",
> + backend->passtCommandline->args[i]);
You must use virBufferEscapeString to ensure proper XML entity escaping
for any XML output which can be arbitrarily provided by users.
> + }
> +
> + virBufferAdjustIndent(&childBuf, -2);
> + virBufferAddLit(&childBuf, "</passt:commandline>\n");
> + }
> +
> + 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/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
> index fcc34de384..e64ccfa5aa 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/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..1715d4253c
> --- /dev/null
> +++ b/tests/qemuxmlconfdata/net-user-passt-custom-args.x86_64-latest.args
> @@ -0,0 +1,39 @@
> +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-i386 \
> +-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 qemu32 \
> +-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 \
> +-device
> '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
> +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
-drive? How did you come up with this output file?!?!
support for -drive was deleted LOOOOOOOOOOOOOOONG time ago
