On Mon, Feb 26, 2018 at 11:05:49AM +0100, Michal Privoznik wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1494454 > > If a domain disk is stored on local filesystem (e.g. ext4) but is > not being migrated it is very likely that domain is not able to > run on destination. Regardless of share/cache mode. > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > --- > src/qemu/qemu_migration.c | 28 ++++++++++++++++++---------- > 1 file changed, 18 insertions(+), 10 deletions(-) > > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c > index b22a327b5..e98b1e4ce 100644 > --- a/src/qemu/qemu_migration.c > +++ b/src/qemu/qemu_migration.c > @@ -1215,25 +1215,26 @@ qemuMigrationSrcIsSafe(virDomainDefPtr def, > virDomainDiskDefPtr disk = def->disks[i]; > const char *src = virDomainDiskGetSource(disk); > > - /* Our code elsewhere guarantees shared disks are either readonly (in > - * which case cache mode doesn't matter) or used with cache=none or used with cache=directsync */ > + /* Disks without any source (i.e. floppies and CD-ROMs) > + * OR readonly are safe. */ > if (virStorageSourceIsEmpty(disk->src) || > - disk->src->readonly || > - disk->src->shared || > - disk->cachemode == VIR_DOMAIN_DISK_CACHE_DISABLE || > - disk->cachemode == VIR_DOMAIN_DISK_CACHE_DIRECTSYNC) > + disk->src->readonly) > continue; > > - /* disks which are migrated by qemu are safe too */ > + /* Disks which are migrated by qemu are safe too. */ > if (storagemigration && > qemuMigrationAnyCopyDisk(disk, nmigrate_disks, migrate_disks)) > continue; > > + /* However, disks on local FS (e.g. ext4) are not safe. */ > if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_FILE) { > - if ((rc = virFileIsSharedFS(src)) < 0) > + if ((rc = virFileIsSharedFS(src)) < 0) { > return false; > - else if (rc == 0) > - continue; > + } else if (rc == 0) { > + virReportError(VIR_ERR_MIGRATE_UNSAFE, "%s", > + _("Migration without shared storage is unsafe")); > + return false; > + } > if ((rc = virStorageFileIsClusterFS(src)) < 0) > return false; > else if (rc == 1) > @@ -1243,6 +1244,13 @@ qemuMigrationSrcIsSafe(virDomainDefPtr def, > continue; > } > > + /* Our code elsewhere guarantees shared disks are either readonly (in > + * which case cache mode doesn't matter) or used with cache=none or used with cache=directsync */ > + if (disk->src->shared || > + disk->cachemode == VIR_DOMAIN_DISK_CACHE_DISABLE || > + disk->cachemode == VIR_DOMAIN_DISK_CACHE_DIRECTSYNC) > + continue; > + > virReportError(VIR_ERR_MIGRATE_UNSAFE, "%s", > _("Migration may lead to data corruption if disks" > " use cache != none or cache != directsync")); A little hard to follow but the key difference is that the old code would immediately return saying it is "safe" if the cache mode was set to a reasonable value, and not get to checking whether disk is local or not. We've essentially reversed that so we detect unsafe scenarios first, and then later check safe options. Reviewed-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
