5:08 a.m.
Maximilian Wilhelm wrote:
Anno domini 2009 Daniel Veillard scripsit:
> On Mon, Oct 12, 2009 at 01:32:26PM +0200, Chris Lalancette wrote:
>> Normally, when you migrate a domain from host A to host B,
>> the domain on host A remains defined but shutoff and the domain
>> on host B remains running but is a "transient". Add a new
>> flag to virDomainMigrate() to allow the original domain to be
>> undefined on source host A, and a new flag to virDomainMigrate() to
>> allow the new domain to be persisted on the destination host B.
>> Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
Thanks for the ground work!
Attached you can find a patch implementing the same flags for Xen:
* src/xen/xen_driver.c: Add support for VIR_MIGRATE_PERSIST_DEST flag
* src/xen/xend_internal.c: Add support for VIR_MIGRATE_UNDEFINE_SOURCE flag
I'm not totaly sure if there are better ways to handle all the error
cases. The current solution seemed to be a same one for me.
Well, I do think that we need to return a proper error in all cases except for
the one with the long comment. I've pointed out where I think we need to add
error messages below. Otherwise, I think it looks good.
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 5273a11..18882c0 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1204,9 +1204,53 @@ xenUnifiedDomainMigrateFinish (virConnectPtr dconn,
const char *cookie ATTRIBUTE_UNUSED,
int cookielen ATTRIBUTE_UNUSED,
const char *uri ATTRIBUTE_UNUSED,
- unsigned long flags ATTRIBUTE_UNUSED)
+ unsigned long flags)
{
- return xenUnifiedDomainLookupByName (dconn, dname);
+ virDomainPtr dom = NULL;
+ char *domain_xml = NULL;
+ virDomainPtr dom_new = NULL;
+
+ dom = xenUnifiedDomainLookupByName (dconn, dname);
+ if (! dom) {
You are probably going to want to raise some error here, otherwise the user will
get back "unknown error", which isn't very helpful.
+ return NULL;
+ }
+
+ if (flags & VIR_MIGRATE_PERSIST_DEST) {
+ domain_xml = xenDaemonDomainDumpXML (dom, 0, NULL);
+ if (! domain_xml) {
+ goto failure;
+ }
This seems whitespace damaged (using tabs instead of spaces), and also needs to
raise a proper error.
+
+ dom_new = xenDaemonDomainDefineXML (dconn, domain_xml);
+ if (! dom_new) {
+ /* Hmpf. Migration was successful, but making it persistent
+ * was not. If we report successful, then when this domain
+ * shuts down, management tools are in for a surprise. On the
+ * other hand, if we report failure, then the management tools
+ * might try to restart the domain on the source side, even
+ * though the domain is actually running on the destination.
+ * Return a NULL dom pointer, and hope that this is a rare
+ * situation and management tools are smart.
+ */
+ goto failure;
+ }
+
+ /* Free additional reference added by Define */
+ virDomainFree (dom_new);
+ }
+
+ VIR_FREE (domain_xml);
+
+ return dom;
+
+
+failure:
+ virDomainFree (dom);
+
+ VIR_FREE (domain_xml);
+
+ return NULL;
+
}
static int
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 27d215e..9fbb616 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -4386,6 +4386,8 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
int ret;
char *p, *hostname = NULL;
+ int undefined_source = 0;
+
/* Xen doesn't support renaming domains during migration. */
if (dname) {
virXendError (conn, VIR_ERR_NO_SUPPORT,
@@ -4404,11 +4406,25 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
return -1;
}
- /* Check the flags. */
+ /*
+ * Check the flags.
+ */
if ((flags & VIR_MIGRATE_LIVE)) {
strcpy (live, "1");
flags &= ~VIR_MIGRATE_LIVE;
}
+
+ /* Undefine the VM on the source host after migration ? */
+ if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) {
+ undefined_source = 1;
+ flags &= ~VIR_MIGRATE_UNDEFINE_SOURCE;
+ }
+
+ /* Ignore the persist_dest flag here */
+ if (flags & VIR_MIGRATE_PERSIST_DEST) {
+ flags &= ~VIR_MIGRATE_PERSIST_DEST;
+ }
Again a nit, but I think the libvirt style currently is to leave braces off for
single line statements. That is, this should be:
if (flags & VIR_MIGRATE_PERSIST_DEST)
flags &= ~VIR_MIGRATE_PERSIST_DEST;
+
/* XXX we could easily do tunnelled & peer2peer migration too
if we want to. support these... */
if (flags != 0) {
@@ -4494,6 +4510,10 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
NULL);
VIR_FREE (hostname);
+ if (ret == 0 && undefined_source) {
+ xenDaemonDomainUndefine (domain);
+ }
+
Same here.
DEBUG0("migration done");
return ret;
--
Chris Lalancette