Re: [libvirt] [PATCH v2 4/4] qemu: Forbid migration with cache != none

On 02/22/2012 07:51 AM, Jiri Denemark wrote: > Migrating domains with disks using cache != none is unsafe unless the > disk images are stored on coherent clustered filesystem. Thus we forbid > migrating such domains unless VIR_MIGRATE_UNSAFE flags is used. > --- > Notes: > Version 2: > - use virStorageFileIsClusterFS > > src/qemu/qemu_driver.c | 3 ++- > src/qemu/qemu_migration.c | 39 +++++++++++++++++++++++++++++++++++---- > src/qemu/qemu_migration.h | 6 ++++-- > 3 files changed, 41 insertions(+), 7 deletions(-) > > > +static bool > +qemuMigrationIsSafe(virDomainDefPtr def) > +{ > + int i; > + > + for (i = 0 ; i < def->ndisks ; i++) { > + virDomainDiskDefPtr disk = def->disks[i]; > + > + /* shared && !readonly implies cache=none */ > + if (disk->src && > + disk->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE && > + (disk->cachemode || !disk->shared || disk->readonly) && > + virStorageFileIsClusterFS(disk->src) == 1) { Other than Dan's comment about the logic bug here, ACK.

Since the check for safety is only on the source, and the destination doesn't care, is there a way to add a driver feature flag, and add logic to libvirt.c to mask the VIR_MIGRATE_UNSAFE flag from the destination if it does not support the feature, similar to how we handled VIR_MIGRATE_CHANGE_PROTECTION via the VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION feature, as another example of a source-only flag? Of course, this would be a followup patch, if we decide it is worth allowing an unsafe migration from 1.9.11 back to 1.9.10 (the upgrade migration from 1.9.10 to 1.9.11 will be unsafe automatically, because we weren't checking in 1.9.10).