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@redhat.com>
---
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"));
--
2.16.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com>
> ---
> 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@redhat.com>
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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.