[libvirt] [PATCH 2/4] qemuMigrationEatCookie: Pass virDomainDef instead of virDomainObj

[Jiri Denemark]

On Thu, Nov 22, 2018 at 14:16:16 +0100, Michal Privoznik wrote:
> The function currently takes virDomainObjPtr because it's using
> both: the domain definition and domain private data.
> Unfortunately, this means that in prepare phase we can't parse
> migration cookie before putting incoming domain def onto domain
> objects list (addressed in the very next commit). Change the
> arguments so that virDomainDef and private data are passed
> instead of virDomainObjPtr.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_migration.c        | 16 ++++++++++------
>  src/qemu/qemu_migration_cookie.c | 23 ++++++++++++-----------
>  src/qemu/qemu_migration_cookie.h |  4 +++-
>  3 files changed, 25 insertions(+), 18 deletions(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 317df4bed5..28d3a72e32 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -2041,7 +2041,8 @@ qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver,
>      if (!(flags & VIR_MIGRATE_OFFLINE))
>          cookieFlags |= QEMU_MIGRATION_COOKIE_CAPS;
>  
> -    if (!(mig = qemuMigrationEatCookie(driver, vm, NULL, 0, 0)))
> +    if (!(mig = qemuMigrationEatCookie(driver, vm->def,
> +                                       priv->origname, NULL, NULL, 0, 0)))

I think this patch should always pass priv instead of NULL. In the
following patch you can replace priv with NULL in the only place where
qemuMigrationEatCookie will be called when there's no priv pointer yet.

I know we only use priv inside qemuMigrationEatCookie when
QEMU_MIGRATION_COOKIE_STATS flag is set. But I think it would be safer
to pass priv everywhere anyway.

>          goto cleanup;
>  
>      if (qemuMigrationBakeCookie(mig, driver, vm,
...
> @@ -4948,8 +4952,8 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,

A little bit more lines of context:

       cookie_flags = QEMU_MIGRATION_COOKIE_NETWORK |
                      QEMU_MIGRATION_COOKIE_STATS |
                      QEMU_MIGRATION_COOKIE_NBD;
       /* Some older versions of libvirt always send persistent XML in the cookie
>       * even though VIR_MIGRATE_PERSIST_DEST was not used. */
>      cookie_flags |= QEMU_MIGRATION_COOKIE_PERSISTENT;
>  
> -    if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein,
> -                                       cookieinlen, cookie_flags)))
> +    if (!(mig = qemuMigrationEatCookie(driver, vm->def, priv->origname, NULL,
> +                                       cookiein, cookieinlen, cookie_flags)))
>          goto endjob;

cookie_flags contains QEMU_MIGRATION_COOKIE_STATS at this point so
passing NULL would just crash libvirtd.

Jirka