Re: [libvirt PATCH v2 8/9] qemu-slirp: register helper for migration

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 2/25/20 10:55 AM, marcandre.lureau@xxxxxxxxxx wrote:
> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
>
> When the helper supports DBus, connect it to the bus and set its ID.
>
> If the helper supports migration, register its ID to the list of
> dbus-vmstate ID to migrate, and specify --dbus-incoming when
> restoring the VM.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> ---
>   src/qemu/qemu_slirp.c | 36 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
>
> diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c
> index 8e001f0d10..e9b23f72a5 100644
> --- a/src/qemu/qemu_slirp.c
> +++ b/src/qemu/qemu_slirp.c
> @@ -18,6 +18,7 @@
>   
>   #include <config.h>
>   
> +#include "qemu_dbus.h"
>   #include "qemu_extdevice.h"
>   #include "qemu_security.h"
>   #include "qemu_slirp.h"
> @@ -202,6 +203,16 @@ qemuSlirpGetFD(qemuSlirpPtr slirp)
>   }
>   
>   
> +static char *
> +qemuSlirpGetDBusVMStateId(virDomainNetDefPtr net)
> +{
> +    char macstr[VIR_MAC_STRING_BUFLEN] = "";
> +
> +    /* can't use alias, because it's not stable across restarts */
> +    return g_strdup_printf("slirp-%s", virMacAddrFormat(&net->mac, macstr));
> +}
> +
> +
>   void
>   qemuSlirpStop(qemuSlirpPtr slirp,
>                 virDomainObjPtr vm,
> @@ -209,11 +220,14 @@ qemuSlirpStop(qemuSlirpPtr slirp,
>                 virDomainNetDefPtr net)
>   {
>       g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
> +    g_autofree char *id = qemuSlirpGetDBusVMStateId(net);
>       g_autofree char *pidfile = NULL;
>       virErrorPtr orig_err;
>       pid_t pid;
>       int rc;
>   
> +    qemuDBusVMStateRemove(vm, id);
> +
>       if (!(pidfile = qemuSlirpCreatePidFilename(cfg, vm->def, net->info.alias))) {
>           VIR_WARN("Unable to construct slirp pidfile path");
>           return;
> @@ -310,6 +324,28 @@ qemuSlirpStart(qemuSlirpPtr slirp,
>           }
>       }
>   
> +    if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_DBUS_ADDRESS)) {
> +        g_autofree char *id = qemuSlirpGetDBusVMStateId(net);
> +        g_autofree char *dbus_addr = qemuDBusGetAddress(driver, vm);
> +
> +        if (qemuDBusStart(driver, vm) < 0)
> +            return -1;
> +

So at this point we've started the dbus daemon.

> +        virCommandAddArgFormat(cmd, "--dbus-id=%s", id);
> +
> +        virCommandAddArgFormat(cmd, "--dbus-address=%s", dbus_addr);
> +
> +        if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE)) {
> +            if (qemuDBusVMStateAdd(vm, id) < 0) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("Failed to register slirp migration"));
> +                return -1;

And if this fails, we leave it behind. All these 'return -1' (even those 
ones not visible in this hunk need to become 'goto error'. And we also 
need qemuDBusStop() call under the error label.

> +            }
> +            if (incoming)
> +                virCommandAddArg(cmd, "--dbus-incoming");
> +        }
> +    }
> +
>       if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_EXIT_WITH_PARENT))
>           virCommandAddArg(cmd, "--exit-with-parent");
>   

Michal