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

> 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(a)redhat.com&gt; > >Signed-off-by: Yang Hang <yanghang44(a)huawei.com&gt; > >Signed-off-by: Wang Xin <wangxinxin.wang(a)huawei.com&gt; > > > >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) { In fact, the hypervisor will also check weather the ivshmem device is migratable(by using migration blocker) or not. If 'role' is not specified in libvirt, the default role may 'master' or 'peer', we don't kown it, so, I think it's better to check it in hypervisor.

> > >+ 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(a)redhat.com&gt; > > but I'll wait after the release since this is a feature.