10:17 a.m.
Migrating qemu 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.
This series uses similar aproach to forbidding unsafe PCI passthrough
or disk format probing when we forbade those by default with the
possibility to force them.
Domain configuration is only checked on source, which makes migrating
affected domains from an old libvirt to the new one possible. Migrating
back is impossible since destination libvirtd would complain about
unknown flag (the flag is not filtered so it gets to the destination
even though it's not really used there).
However, users of clustered filesystems now have to always pass the new
flag to be able to migrate because libvirtd would think they are doing
something unsafe. Perhaps we should provide a system wide (i.e.,
/etc/libvirt/qemu.conf) tunable which would disable cache mode checking
for all domains at once?
I was also wondering if we should rather use more specific name for both
the error code and flag, such as VIR(_ERR)?_MIGRATE_UNSAFE_CACHE
(or ...UNSAFE_DISK) in the case we find other unsafe conditions...
Jiri Denemark (3):
Add support for unsafe migration
virsh: Add --unsafe option to migrate command
qemu: Forbid migration with cache != none
include/libvirt/libvirt.h.in | 2 +-
include/libvirt/virterror.h | 1 +
src/libvirt.c | 4 ++++
src/qemu/qemu_driver.c | 3 ++-
src/qemu/qemu_migration.c | 36 ++++++++++++++++++++++++++++++++----
src/qemu/qemu_migration.h | 6 ++++--
src/util/virterror.c | 6 ++++++
tools/virsh.c | 4 ++++
tools/virsh.pod | 10 +++++++++-
9 files changed, 63 insertions(+), 9 deletions(-)
--
1.7.8.4
10:17 a.m.
New subject: [libvirt] [PATCH 1/3] Add support for unsafe migration
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.
---
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 a55d823..17c4d15 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
10:17 a.m.
New subject: [libvirt] [PATCH 2/3] virsh: Add --unsafe option to migrate command
---
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 66ba61c..4e30325 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -6733,6 +6733,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)")},
@@ -6806,6 +6807,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 0d5a41d..c9c7b13 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -765,7 +765,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>]
@@ -786,6 +786,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
10:17 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: Forbid migration with cache != none
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.
---
src/qemu/qemu_driver.c | 3 ++-
src/qemu/qemu_migration.c | 36 ++++++++++++++++++++++++++++++++----
src/qemu/qemu_migration.h | 6 ++++--
3 files changed, 38 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..127b35c 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -817,6 +817,27 @@ 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->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE &&
+ (disk->cachemode || !disk->shared || disk->readonly)) {
+ 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 +1031,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 +1040,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 +1054,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 +2095,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 +2379,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
12:58 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: Forbid migration with cache != none
On 2012-2-22 0:17, 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.
---
src/qemu/qemu_driver.c | 3 ++-
src/qemu/qemu_migration.c | 36 ++++++++++++++++++++++++++++++++----
src/qemu/qemu_migration.h | 6 ++++--
3 files changed, 38 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..127b35c 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -817,6 +817,27 @@ 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->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE&&
how about disk write through flag here? This flag should also imply
cache=none.
i.e, VIR_DOMAIN_DISK_CACHE_WRITETHRU
+ (disk->cachemode || !disk->shared ||
disk->readonly)) {
+ 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 +1031,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 +1040,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 +1054,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 +2095,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 +2379,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,
--
Shu Ming<shuming(a)linux.vnet.ibm.com>
IBM China Systems and Technology Laboratory
7:57 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: Forbid migration with cache != none
On Wed, Feb 22, 2012 at 14:58:31 +0800, Shu Ming wrote:
On 2012-2-22 0:17, Jiri Denemark wrote:
> @@ -817,6 +817,27 @@ 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->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE&&
how about disk write through flag here? This flag should also imply
cache=none.
i.e, VIR_DOMAIN_DISK_CACHE_WRITETHRU
AFAIK, only VIR_DOMAIN_DISK_CACHE_DISABLE translates into cache=none unless
qemu binary is old enough to only support cache=on|off
Jirka
1:02 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: Forbid migration with cache != none
On 02/22/2012 02:57 PM, Jiri Denemark wrote:
> > how about disk write through flag here? This flag should
also imply
> > cache=none.
> >
> > i.e, VIR_DOMAIN_DISK_CACHE_WRITETHRU
AFAIK, only VIR_DOMAIN_DISK_CACHE_DISABLE translates into cache=none unless
qemu binary is old enough to only support cache=on|off
VIR_DOMAIN_DISK_CACHE_DIRECTSYNC too.
Paolo
10:25 a.m.
On Tue, Feb 21, 2012 at 05:17:20PM +0100, Jiri Denemark wrote:
Migrating qemu 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.
This series uses similar aproach to forbidding unsafe PCI passthrough
or disk format probing when we forbade those by default with the
possibility to force them.
Domain configuration is only checked on source, which makes migrating
affected domains from an old libvirt to the new one possible. Migrating
back is impossible since destination libvirtd would complain about
unknown flag (the flag is not filtered so it gets to the destination
even though it's not really used there).
However, users of clustered filesystems now have to always pass the new
flag to be able to migrate because libvirtd would think they are doing
something unsafe. Perhaps we should provide a system wide (i.e.,
/etc/libvirt/qemu.conf) tunable which would disable cache mode checking
for all domains at once?
I think we should add a virStorageFileIsClusterFS(const char *path)
and whitelist the filesystem magic that we know is ok. I expect GFS,
GFS2, OCFS2 would be a good starting point. We already do this for
virStorageFileIsSharedFS() in migration, so this is no worse
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4692
days inactive
4694
days old
7 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Jiri Denemark -
Paolo Bonzini -
Shu Ming