> 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. > Get it, thanks for explaining. :) > >> > >> >+ 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: Sure. > >> > >> Reviewed-by: Martin Kletzander <mkletzan@xxxxxxxxxx> > >> > >> but I'll wait after the release since this is a feature. > > > > Thanks for your review! Best regards, Alexander.
