Re: [libvirt PATCH 8/9] qemu: hook up passt config to qemu domains

On 1/9/23 2:32 AM, Ján Tomko wrote:

On a Sunday in 2023, Laine Stump wrote:

...

This consists of (1) adding the necessary args to the qemu commandline netdev option, and (2) starting a passt process prior to starting qemu, and making sure that it is terminated when it's no longer needed. Under normal circumstances, passt will terminate itself as soon as qemu closes its socket, but in case of some error where qemu is never started, or fails to startup completely, we need to terminate passt manually.

Signed-off-by: Laine Stump --- meson.build                                   |   1 + po/POTFILES                                   |   1 + src/qemu/meson.build                          |   2 + src/qemu/qemu_command.c                       |  11 +- src/qemu/qemu_command.h                       |   3 +- src/qemu/qemu_domain.c                        |   5 +- src/qemu/qemu_domain.h                        |   3 +- src/qemu/qemu_extdevice.c                     |  25 +- src/qemu/qemu_hotplug.c                       |  26 +- src/qemu/qemu_passt.c                         | 284 ++++++++++++++++++ src/qemu/qemu_passt.h                         |  38 +++ src/qemu/qemu_process.c                       |   1 + src/qemu/qemu_validate.c                      |   9 +- tests/qemuxml2argvdata/net-user-passt.args    |  34 +++ .../net-user-passt.x86_64-latest.args         |  37 +++ tests/qemuxml2argvtest.c                      |   2 + 16 files changed, 470 insertions(+), 12 deletions(-) create mode 100644 src/qemu/qemu_passt.c create mode 100644 src/qemu/qemu_passt.h create mode 100644 tests/qemuxml2argvdata/net-user-passt.args create mode 100644 tests/qemuxml2argvdata/net-user-passt.x86_64-latest.args

diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c new file mode 100644 index 0000000000..5941594811 --- /dev/null +++ b/src/qemu/qemu_passt.c @@ -0,0 +1,284 @@ +/* + * qemu_passt.c: QEMU passt support + * + * Copyright (C) 2022 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 + +#include "qemu_dbus.h" +#include "qemu_extdevice.h" +#include "qemu_security.h" +#include "qemu_passt.h" +#include "virenum.h" +#include "virerror.h" +#include "virjson.h" +#include "virlog.h" +#include "virpidfile.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("qemu.passt"); + + +static char * +qemuPasstCreatePidFilename(virDomainObj *vm, +                           virDomainNetDef *net) +{ +    qemuDomainObjPrivate *priv = vm->privateData; +    virQEMUDriver *driver = priv->driver; +    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); +    g_autofree char *name = NULL; + +    name = g_strdup_printf("%s-%s-passt", vm->def->name, net->info.alias);

Please use virDomainDefGetShortName for filename purposes.

Why? If I use GetShortName, then there's the possibility that two domains would want to use the same name for the pidfile. Would it be better to use the domain's UUID (as I did for the socket path?) The advantage of using the name is that it's easier for a human to find, but while the uuid is usually longer, its length is at least predictable/consistent, and I suppose a human will probably never need to find the pidfile anyway...

...

+ +    return virPidFileBuildPath(cfg->passtStateDir, name); +} + +

[...]

...

+int +qemuPasstAddNetProps(virDomainObj *vm, +                     virDomainNetDef *net, +                     virJSONValue **netprops) +{ +    g_autofree char *passtSocketName = qemuPasstCreateSocketPath(vm, net); +    g_autoptr(virJSONValue) addrprops = NULL; + +    if (virJSONValueObjectAdd(&addrprops, +                              "s:type", "unix", +                              "s:path", passtSocketName, +                              NULL) < 0) { +        return -1; +    } + +    if (virJSONValueObjectAdd(netprops, +                              "s:type", "stream", +                              "a:addr", &addrprops, +                              "b:server", false,

...

+                              /*  "u:reconnect", 5, */

Debugging leftover?

It's a new option that hasn't been pushed to QEMU upstream yet. I had meant to completely remove it, but forgot. I'll get rid of it before I push.