On Mon, Jan 26, 2026 at 11:05:13 +0100, Guy Godfroy via Devel wrote:
> Currently, the qemu hook script with the "migrate" operation is only
> called on the destination host at the beginning of incoming migration.
> This makes it impossible for the source host to prepare for migration,
> for example to change storage locks from exclusive to shared mode when
> using shared LVM storage with lvmlockd.
> 
> Add a hook call on the source host at the beginning of outgoing
> migration. The hook is called with "source" as the extra argument to
> distinguish it from the destination hook which uses "-".

Well, this has IMO still the potential to break things. I suggest that
the second argument to the hook program is something else than
'migrate'. Perhaps 'migrate-outgoing' to prevent breaking programs which
ignore the third argument.

I'm also worried a bit about the usability of the hook for cases when
the migration fails ...

> The same change is also applied to the libxl driver for consistency.
> 
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/37
> Signed-off-by: Guy Godfroy <[email protected]>
> ---
>  docs/hooks.rst              | 36 ++++++++++++++++++++++++++++++++----
>  src/libxl/libxl_migration.c | 17 +++++++++++++++++
>  src/qemu/qemu_migration.c   | 17 +++++++++++++++++
>  3 files changed, 66 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/hooks.rst b/docs/hooks.rst
> index e1745b8cc7..4c7cac56ba 100644
> --- a/docs/hooks.rst
> +++ b/docs/hooks.rst
> @@ -244,7 +244,7 @@ operation. There is no specific operation to indicate a 
> "restart" is occurring.
>  
>  
>  -  :since:`Since 0.9.11`, the qemu hook script is also called at the 
> beginning
> -   of incoming migration. It is called as:
> +   of incoming migration on the **destination** host. It is called as:

(These doc changes make sense on its own. Ideally separate them to
another patch)
>  
>     ::
>  
> @@ -257,6 +257,18 @@ operation. There is no specific operation to indicate a 
> "restart" is occurring.
>     is not valid, incoming migration will be canceled. This hook may be used,
>     e.g., to change location of disk images for incoming domains.
>  
> +-  :since:`Since 12.1.0`, the qemu hook script is also called at the 
> beginning
> +   of outgoing migration on the **source** host. It is called as:
> +
> +   ::
> +
> +      /etc/libvirt/hooks/qemu guest_name migrate begin source
> +
> +   with domain XML sent to standard input of the script. Unlike the 
> destination
> +   hook, this script does not act as a filter and any output is ignored. If 
> the
> +   script returns failure, the migration will be canceled. This hook may be 
> used,
> +   e.g., to change storage lock mode from exclusive to shared before 
> migration.
> +
>  -  :since:`Since 1.2.9`, the qemu hook script is also called when restoring a
>     saved image either via the API or automatically when restoring a managed 
> save
>     machine. It is called as:
> @@ -417,7 +429,7 @@ operation. There is no specific operation to indicate a 
> "restart" is occurring.
>        /etc/libvirt/hooks/libxl guest_name release end -
>  
>  -  :since:`Since 2.1.0`, the libxl hook script is also called at the 
> beginning
> -   of incoming migration. It is called as:
> +   of incoming migration on the **destination** host. It is called as:
>  
>     ::
>  
> @@ -430,6 +442,18 @@ operation. There is no specific operation to indicate a 
> "restart" is occurring.
>     is not valid, incoming migration will be canceled. This hook may be used,
>     e.g., to change location of disk images for incoming domains.
>  
> +-  :since:`Since 12.1.0`, the libxl hook script is also called at the 
> beginning
> +   of outgoing migration on the **source** host. It is called as:
> +
> +   ::
> +
> +      /etc/libvirt/hooks/libxl guest_name migrate begin source
> +
> +   with domain XML sent to standard input of the script. Unlike the 
> destination
> +   hook, this script does not act as a filter and any output is ignored. If 
> the
> +   script returns failure, the migration will be canceled. This hook may be 
> used,
> +   e.g., to change storage lock mode from exclusive to shared before 
> migration.
> +
>  -  :since:`Since 6.5.0`, you can also place several hook scripts in the
>     directory ``/etc/libvirt/hooks/libxl.d/``. They are executed in 
> alphabetical
>     order after main script. In this case each script also acts as filter and 
> can
> @@ -566,10 +590,14 @@ Migration of a QEMU guest involves running hook scripts 
> on both the source and
>  destination hosts:
>  
>  #. At the beginning of the migration, the *qemu* hook script on the
> -   **destination** host is executed with the "migrate" operation.
> +   **source** host is executed with the "migrate" operation and "source"
> +   as the extra argument (:since:`since 12.1.0`). This allows the source
> +   host to prepare for migration, e.g., changing storage locks.
> +#. Then, the *qemu* hook script on the **destination** host is executed
> +   with the "migrate" operation.
>  #. Before QEMU process is spawned, the two operations ("prepare" and "start")
>     called for domain start are executed on **destination** host.
> -#. If both of these hook script executions exit successfully (exit status 0),
> +#. If all of these hook script executions exit successfully (exit status 0),
>     the migration continues. Any other exit code indicates failure, and the
>     migration is aborted.
>  #. The QEMU guest is then migrated to the destination host.
> diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
> index f5dee7627b..76f3c7ceb7 100644
> --- a/src/libxl/libxl_migration.c
> +++ b/src/libxl/libxl_migration.c
> @@ -411,6 +411,23 @@ libxlDomainMigrationSrcBegin(virConnectPtr conn,
>      if (!libxlDomainMigrationIsAllowed(def))
>          goto endjob;
>  
> +    /* Call hook to allow source host to prepare for migration */
> +    if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) {
> +        g_autofree char *hookxml = NULL;
> +        int hookret;
> +
> +        if (!(hookxml = virDomainDefFormat(def, driver->xmlopt,
> +                                           VIR_DOMAIN_DEF_FORMAT_SECURE)))
> +            goto endjob;
> +
> +        hookret = virHookCall(VIR_HOOK_DRIVER_LIBXL, def->name,
> +                              VIR_HOOK_LIBXL_OP_MIGRATE, 
> VIR_HOOK_SUBOP_BEGIN,
> +                              "source", hookxml, NULL);
> +
> +        if (hookret < 0)
> +            goto endjob;
> +    }
> +
>      xml = virDomainDefFormat(def, driver->xmlopt, 
> VIR_DOMAIN_DEF_FORMAT_SECURE);
>      /* Valid xml means success! EndJob in the confirm phase */
>      if (xml)
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 6dd022163b..60e370319c 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -2791,6 +2791,23 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver,
>      if (!qemuMigrationSrcIsAllowed(vm, true, vm->job->asyncJob, flags))
>          return NULL;
>  
> +    /* Call hook to allow source host to prepare for migration */
> +    if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
> +        g_autofree char *xml = NULL;
> +        int hookret;
> +
> +        if (!(xml = qemuDomainDefFormatLive(driver, priv->qemuCaps, vm->def,
> +                                            priv->origCPU, false, true)))
> +            return NULL;
> +
> +        hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
> +                              VIR_HOOK_QEMU_OP_MIGRATE, VIR_HOOK_SUBOP_BEGIN,
> +                              "source", xml, NULL);
> +
> +        if (hookret < 0)
> +            return NULL;
> +    }

... specifically this is *very* early in the migration. So early that
...


> +
>      if (!(flags & (VIR_MIGRATE_UNSAFE | VIR_MIGRATE_OFFLINE)) &&
>          !qemuMigrationSrcIsSafe(vm, migrate_disks, flags))

... we didn't do even the pre-checks if migration will happen. That is
not the problem per-se, but there is no hook action if the migration
fails, in which case the hook program will very likely want to undo what
it changed.

At the very least the source migration hook, or however it's called
really needs another parameter which will for this invocation be e.g.
'begin'. We can later add a 'fail' action if needed. I've seen also
ideas of having a hook call during switchover phase so that one could
then be added too if we get a compelling reason to have it. (all hook
calls add migration latency so a switchover hook will need to be
implemented in a different way).

Also don't forget to bump version numbers to at least 12.3.0 when you'll
re-send.

Reply via email to