On Mon, Aug 03, 2020 at 09:26:19AM +0000, Wangxin (Alexander) wrote:
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) {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.
We strive to prevent any errors in QEMU "just in case". Users could start doubting our backwards compatibility if their `default` role shmem stopped migrating in the future, for example. And by being strict we avoid any possible future issues like this.
>+ 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
