Re: [PATCH v3 1/2] qemu: add support for shmem-{plain, doorbell} role

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

On Fri, Jul 24, 2020 at 11:34:11AM +0800, Wang Xin wrote:
> Role(master or peer) controls how the domain behaves on migration.
> For more details about migration with ivshmem, see
> https://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/system/ivshmem.rst;hb=HEAD
>
> It's a optional attribute in libvirt, and qemu will choose default
> role for ivshmem device if the user is not specified.
>
> With device property 'role', the value can be 'master' or 'peer'.
> - 'master' (means 'master=on' in qemu), the guest will copy
>   the shared memory on migration to the destination host.
> - 'peer' (means 'master=off' in qemu), the migration is disabled.
>
> Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> Signed-off-by: Yang Hang <yanghang44@xxxxxxxxxx>
> Signed-off-by: Wang Xin <wangxinxin.wang@xxxxxxxxxx>
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 2c7bf349c3..e588b74c2f 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1261,10 +1261,22 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver,
>             }
>         }
>
> -        if (vm->def->nshmems) {
> -            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> -                           _("migration with shmem device is not supported"));
> -            return false;
> +        for (i = 0; i < vm->def->nshmems; i++) {
> +            virDomainShmemDefPtr shmem = vm->def->shmems[i];
> +
> +            if (shmem->model == VIR_DOMAIN_SHMEM_MODEL_IVSHMEM) {
> +                virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                               _("migration with legacy shmem device is not supported"));
> +                return false;
> +            }
> +            if (shmem->role == VIR_DOMAIN_SHMEM_ROLE_PEER) {

This allows migration with the default role which you leave to be chosen by the
hypervisor.  In that case we need to change this to:

  if (shmem->role != VIR_DOMAIN_SHMEM_ROLE_MASTER) {

> +                virReportError(VIR_ERR_OPERATION_INVALID,
> +                               _("shmem device '%s' with role='%s', "
> +                                 "cannot be migrated"),

and this accordingly.

But that's fine, I can change that before pushing.  With that change:

Reviewed-by: Martin Kletzander <mkletzan@xxxxxxxxxx>

but I'll wait after the release since this is a feature.

Attachment: signature.asc
Description: PGP signature