This patch adds VIR_MIGRATE_UNSAFE flag for migration APIs and new VIR_ERR_MIGRATION_UNSAFE error code. The error code should be returned whenever migrating a domain is considered unsafe (e.g., it's configured in a way that does not ensure data integrity once it is migrated). VIR_MIGRATE_UNSAFE flag may be used to force migration even though it would normally be considered unsafe and forbidden. --- Notes: Version 2: - no change include/libvirt/libvirt.h.in | 2 +- include/libvirt/virterror.h | 1 + src/libvirt.c | 4 ++++ src/util/virterror.c | 6 ++++++ 4 files changed, 12 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 798ab07..e29df2a 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -934,7 +934,7 @@ typedef enum { VIR_MIGRATE_CHANGE_PROTECTION = (1 << 8), /* protect for changing domain configuration through the * whole migration process; this will be used automatically * when supported */ - + VIR_MIGRATE_UNSAFE = (1 << 9), /* force migration even if it is considered unsafe */ } virDomainMigrateFlags; /* Domain migration. */ diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 9dbadfe..a3f9199 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -245,6 +245,7 @@ typedef enum { canceled/aborted by user */ VIR_ERR_AUTH_CANCELLED = 79, /* authentication cancelled */ VIR_ERR_NO_DOMAIN_METADATA = 80, /* The metadata is not present */ + VIR_ERR_MIGRATE_UNSAFE = 81, /* Migration is not safe */ } virErrorNumber; /** diff --git a/src/libvirt.c b/src/libvirt.c index 6294196..a3bd4f4 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -5110,6 +5110,7 @@ virDomainMigrateDirect (virDomainPtr domain, * VIR_MIGRATE_CHANGE_PROTECTION Protect against domain configuration * changes during the migration process (set * automatically when supported). + * VIR_MIGRATE_UNSAFE Force migration even if it is considered unsafe. * * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set. * Applications using the VIR_MIGRATE_PEER2PEER flag will probably @@ -5301,6 +5302,7 @@ error: * VIR_MIGRATE_CHANGE_PROTECTION Protect against domain configuration * changes during the migration process (set * automatically when supported). + * VIR_MIGRATE_UNSAFE Force migration even if it is considered unsafe. * * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set. * Applications using the VIR_MIGRATE_PEER2PEER flag will probably @@ -5509,6 +5511,7 @@ error: * VIR_MIGRATE_CHANGE_PROTECTION Protect against domain configuration * changes during the migration process (set * automatically when supported). + * VIR_MIGRATE_UNSAFE Force migration even if it is considered unsafe. * * The operation of this API hinges on the VIR_MIGRATE_PEER2PEER flag. * If the VIR_MIGRATE_PEER2PEER flag is NOT set, the duri parameter @@ -5633,6 +5636,7 @@ error: * VIR_MIGRATE_CHANGE_PROTECTION Protect against domain configuration * changes during the migration process (set * automatically when supported). + * VIR_MIGRATE_UNSAFE Force migration even if it is considered unsafe. * * The operation of this API hinges on the VIR_MIGRATE_PEER2PEER flag. * diff --git a/src/util/virterror.c b/src/util/virterror.c index fb5ca6f..de60185 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -1232,6 +1232,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("metadata not found: %s"); break; + case VIR_ERR_MIGRATE_UNSAFE: + if (!info) + errmsg = _("Unsafe migration"); + else + errmsg = _("Unsafe migration: %s"); + break; } return (errmsg); } -- 1.7.8.4

--- Notes: Version 2: - no change tools/virsh.c | 4 ++++ tools/virsh.pod | 10 +++++++++- 2 files changed, 13 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index e712e53..3be86ed 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -6819,6 +6819,7 @@ static const vshCmdOptDef opts_migrate[] = { {"copy-storage-inc", VSH_OT_BOOL, 0, N_("migration with non-shared storage with incremental copy (same base image shared between source and destination)")}, {"change-protection", VSH_OT_BOOL, 0, N_("prevent any configuration changes to domain until migration ends)")}, + {"unsafe", VSH_OT_BOOL, 0, N_("force migration even if it may be unsafe")}, {"verbose", VSH_OT_BOOL, 0, N_("display the progress of migration")}, {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, {"desturi", VSH_OT_DATA, VSH_OFLAG_REQ, N_("connection URI of the destination host as seen from the client(normal migration) or source(p2p migration)")}, @@ -6892,6 +6893,9 @@ doMigrate (void *opaque) if (vshCommandOptBool (cmd, "change-protection")) flags |= VIR_MIGRATE_CHANGE_PROTECTION; + if (vshCommandOptBool(cmd, "unsafe")) + flags |= VIR_MIGRATE_UNSAFE; + if (xmlfile && virFileReadAll(xmlfile, 8192, &xml) < 0) { vshError(ctl, _("file '%s' doesn't exist"), xmlfile); diff --git a/tools/virsh.pod b/tools/virsh.pod index f35244f..0f1ea91 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -781,7 +781,7 @@ type attribute for the <domain> element of XML. =item B<migrate> [I<--live>] [I<--direct>] [I<--p2p> [I<--tunnelled>]] [I<--persistent>] [I<--undefinesource>] [I<--suspend>] [I<--copy-storage-all>] -[I<--copy-storage-inc>] [I<--change-protection>] [I<--verbose>] +[I<--copy-storage-inc>] [I<--change-protection>] [I<--unsafe>] [I<--verbose>] I<domain-id> I<desturi> [I<migrateuri>] [I<dname>] [I<--timeout> B<seconds>] [I<--xml> B<file>] @@ -802,6 +802,14 @@ is implicitly enabled when supported by the hypervisor, but can be explicitly used to reject the migration if the hypervisor lacks change protection support. I<--verbose> displays the progress of migration. +In some cases libvirt may refuse to migrate the domain because doing so may +lead, e.g., to data corruption and thus the migration is considered unsafe. +For QEMU domain, this may happen if the domain uses disks without explicitly +setting cache mode to "none". Migrating such domains is unsafe unless the disk +images are stored on coherent clustered filesystem, such as GFS2 or GPFS. If +you are sure the migration is safe or you just do not care, use I<--unsafe> +to force the migration. + The I<desturi> is the connection URI of the destination host, and I<migrateuri> is the migration URI, which usually can be omitted. I<dname> is used for renaming the domain to new name during migration, which -- 1.7.8.4

--- Notes: Version 2: - new patch src/libvirt_private.syms | 1 + src/util/storage_file.c | 10 ++++++++++ src/util/storage_file.h | 1 + 3 files changed, 12 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9e3573f..18a24e4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1011,6 +1011,7 @@ virStorageFileFormatTypeToString; virStorageFileFreeMetadata; virStorageFileGetMetadata; virStorageFileGetMetadataFromFD; +virStorageFileIsClusterFS; virStorageFileIsSharedFS; virStorageFileIsSharedFSType; virStorageFileProbeFormat; diff --git a/src/util/storage_file.c b/src/util/storage_file.c index a8661e3..cd1c142 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -1063,3 +1063,13 @@ int virStorageFileIsSharedFS(const char *path) VIR_STORAGE_FILE_SHFS_OCFS | VIR_STORAGE_FILE_SHFS_AFS); } + +int virStorageFileIsClusterFS(const char *path) +{ + /* These are coherent cluster filesystems known to be safe for + * migration with cache != none + */ + return virStorageFileIsSharedFSType(path, + VIR_STORAGE_FILE_SHFS_GFS2 | + VIR_STORAGE_FILE_SHFS_OCFS); +} diff --git a/src/util/storage_file.h b/src/util/storage_file.h index 96afb12..13d0e87 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -82,6 +82,7 @@ enum { }; int virStorageFileIsSharedFS(const char *path); +int virStorageFileIsClusterFS(const char *path); int virStorageFileIsSharedFSType(const char *path, int fstypes); -- 1.7.8.4

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(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 717bdf1..63a0703 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8767,7 +8767,8 @@ qemuDomainMigrateBegin3(virDomainPtr domain, goto endjob; if (!(xml = qemuMigrationBegin(driver, vm, xmlin, dname, - cookieout, cookieoutlen))) + cookieout, cookieoutlen, + flags))) goto endjob; if ((flags & VIR_MIGRATE_CHANGE_PROTECTION)) { diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f0af494..09494d6 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -45,6 +45,7 @@ #include "virtime.h" #include "locking/domain_lock.h" #include "rpc/virnetsocket.h" +#include "storage_file.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -817,6 +818,29 @@ qemuMigrationIsAllowed(struct qemud_driver *driver, virDomainObjPtr vm, return true; } +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) { + qemuReportError(VIR_ERR_MIGRATE_UNSAFE, "%s", + _("Migration may lead to data corruption if disks" + " use cache != none")); + return false; + } + } + + return true; +} + /** qemuMigrationSetOffline * Pause domain for non-live migration. */ @@ -1010,7 +1034,8 @@ char *qemuMigrationBegin(struct qemud_driver *driver, const char *xmlin, const char *dname, char **cookieout, - int *cookieoutlen) + int *cookieoutlen, + unsigned long flags) { char *rv = NULL; qemuMigrationCookiePtr mig = NULL; @@ -1018,9 +1043,9 @@ char *qemuMigrationBegin(struct qemud_driver *driver, qemuDomainObjPrivatePtr priv = vm->privateData; VIR_DEBUG("driver=%p, vm=%p, xmlin=%s, dname=%s," - " cookieout=%p, cookieoutlen=%p", + " cookieout=%p, cookieoutlen=%p, flags=%lx", driver, vm, NULLSTR(xmlin), NULLSTR(dname), - cookieout, cookieoutlen); + cookieout, cookieoutlen, flags); /* Only set the phase if we are inside QEMU_ASYNC_JOB_MIGRATION_OUT. * Otherwise we will start the async job later in the perform phase losing @@ -1032,6 +1057,9 @@ char *qemuMigrationBegin(struct qemud_driver *driver, if (!qemuMigrationIsAllowed(driver, vm, NULL)) goto cleanup; + if (!(flags & VIR_MIGRATE_UNSAFE) && !qemuMigrationIsSafe(vm->def)) + goto cleanup; + if (!(mig = qemuMigrationEatCookie(driver, vm, NULL, 0, 0))) goto cleanup; @@ -2070,7 +2098,7 @@ static int doPeer2PeerMigrate3(struct qemud_driver *driver, * a single job. */ dom_xml = qemuMigrationBegin(driver, vm, xmlin, dname, - &cookieout, &cookieoutlen); + &cookieout, &cookieoutlen, flags); if (!dom_xml) goto cleanup; @@ -2354,6 +2382,9 @@ qemuMigrationPerformJob(struct qemud_driver *driver, if (!qemuMigrationIsAllowed(driver, vm, NULL)) goto cleanup; + if (!(flags & VIR_MIGRATE_UNSAFE) && !qemuMigrationIsSafe(vm->def)) + goto cleanup; + resume = virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING; if ((flags & (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) { diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index f806ca1..41e4eac 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -35,7 +35,8 @@ VIR_MIGRATE_PAUSED | \ VIR_MIGRATE_NON_SHARED_DISK | \ VIR_MIGRATE_NON_SHARED_INC | \ - VIR_MIGRATE_CHANGE_PROTECTION) + VIR_MIGRATE_CHANGE_PROTECTION | \ + VIR_MIGRATE_UNSAFE) enum qemuMigrationJobPhase { QEMU_MIGRATION_PHASE_NONE = 0, @@ -81,7 +82,8 @@ char *qemuMigrationBegin(struct qemud_driver *driver, const char *xmlin, const char *dname, char **cookieout, - int *cookieoutlen); + int *cookieoutlen, + unsigned long flags); int qemuMigrationPrepareTunnel(struct qemud_driver *driver, virConnectPtr dconn, -- 1.7.8.4

On Wed, Feb 22, 2012 at 14:57:00 +0000, Daniel P. Berrange wrote: Eh, sure. Thanks. Jirka