[PATCH] During a migrate, grab xml, do migration, define at the end
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1205785376 25200 # Node ID 802a5fbe87ed0dff07f2dc75622a9e674d891156 # Parent 47f5fc1ac66dad65785dd050d6b4b9fe95948f8a During a migrate, grab xml, do migration, define at the end ...in a more consistent manner that makes offline migration an effective NOP. Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r 47f5fc1ac66d -r 802a5fbe87ed src/Virt_VSMigrationService.c --- a/src/Virt_VSMigrationService.c Mon Mar 17 13:22:42 2008 -0700 +++ b/src/Virt_VSMigrationService.c Mon Mar 17 13:22:56 2008 -0700 @@ -860,6 +860,23 @@ static CMPIStatus handle_migrate(virConn { CMPIStatus s = {CMPI_RC_OK, NULL}; virDomainPtr ddom = NULL; + virDomainInfo info; + int ret; + + ret = virDomainGetInfo(dom, &info); + if (ret == -1) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Error getting domain info"); + goto out; + } + + if ((const int)info.state == VIR_DOMAIN_SHUTOFF) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_INVALID_PARAMETER, + "Domain must not be shut off for live migration"); + goto out; + } CU_DEBUG("Migrating %s -> %s", job->domain, uri); ddom = virDomainMigrate(dom, dconn, type, NULL, NULL, 0); @@ -869,71 +886,75 @@ static CMPIStatus handle_migrate(virConn CMPI_RC_ERR_FAILED, "Migration Failed"); } - + out: virDomainFree(ddom); return s; } -static CMPIStatus prepare_offline_migrate(virDomainPtr dom, - char **xml) +static CMPIStatus prepare_migrate(virDomainPtr dom, + char **xml) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + + *xml = virDomainGetXMLDesc(dom, 0); + if (*xml == NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to retrieve domain XML."); + goto out; + } + + out: + return s; +} + +static CMPIStatus complete_migrate(virDomainPtr ldom, + virConnectPtr rconn, + const char *xml) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + virDomainPtr newdom = NULL; + + if (virDomainUndefine(ldom) == -1) { + CU_DEBUG("Undefine of local domain failed"); + } + + newdom = virDomainDefineXML(rconn, xml); + if (newdom == NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Failed to define domain"); + goto out; + } + + CU_DEBUG("Defined domain on destination host"); + out: + virDomainFree(newdom); + + return s; +} + +static CMPIStatus ensure_dom_offline(virDomainPtr dom) { CMPIStatus s = {CMPI_RC_OK, NULL}; virDomainInfo info; int ret; ret = virDomainGetInfo(dom, &info); - if (ret != 0) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unabled to get info for domain."); + if (ret == -1) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Error getting domain info"); goto out; } if ((const int)info.state != VIR_DOMAIN_SHUTOFF) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_INVALID_PARAMETER, - "Domain must be shutoff for offline migration."); - goto out; - } - - *xml = virDomainGetXMLDesc(dom, 0); - if (*xml == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unable to retrieve domain XML."); - goto out; - } - - out: - return s; -} - -static CMPIStatus handle_offline_migrate(virConnectPtr dconn, - virDomainPtr dom, - char *uri, - char *xml, - struct migration_job *job) -{ - CMPIStatus s = {CMPI_RC_OK, NULL}; - virDomainPtr new_dom; - - if (domain_exists(dconn, job->domain)) { - CU_DEBUG("This domain already exists on the target system."); - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "This domain already exists on the target system"); - goto out; - } - - new_dom = virDomainDefineXML(dconn, xml); - if (new_dom == NULL) { - CU_DEBUG("Failed to define domain from XML"); - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Failed to create domain"); - } - + "Domain must be shut off for offline migration"); + goto out; + } out: return s; } @@ -959,27 +980,33 @@ static CMPIStatus migrate_vs(struct migr goto out; } - if (job->type == CIM_MIGRATE_OTHER) { - s = prepare_offline_migrate(dom, &xml); - if (s.rc != CMPI_RC_OK) - goto out; - } + if (domain_exists(job->conn, job->domain)) { + CU_DEBUG("Remote domain `%s' exists", job->domain); + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Remote already has domain `%s'", job->domain); + goto out; + } + + s = prepare_migrate(dom, &xml); + if (s.rc != CMPI_RC_OK) + goto out; switch(job->type) { case CIM_MIGRATE_OTHER: - CU_DEBUG("Preparing for offline migration"); - s = handle_offline_migrate(job->conn, dom, uri, xml, job); + CU_DEBUG("Offline migration"); + s = ensure_dom_offline(dom); break; case CIM_MIGRATE_LIVE: - CU_DEBUG("Preparing for live migration"); + CU_DEBUG("Live migration"); s = handle_migrate(job->conn, dom, uri, VIR_MIGRATE_LIVE, job); break; case CIM_MIGRATE_RESUME: case CIM_MIGRATE_RESTART: - CU_DEBUG("Preparing for static migration"); + CU_DEBUG("Static migration"); s = handle_migrate(job->conn, dom, uri, 0, job); break; - default: + default: CU_DEBUG("Unsupported migration type (%d)", job->type); cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, @@ -990,11 +1017,13 @@ static CMPIStatus migrate_vs(struct migr if (s.rc != CMPI_RC_OK) goto out; - CU_DEBUG("Migration succeeded"); - cu_statusf(_BROKER, &s, - CMPI_RC_OK, - ""); - + s = complete_migrate(dom, job->conn, xml); + if (s.rc == CMPI_RC_OK) { + CU_DEBUG("Migration succeeded"); + } else { + CU_DEBUG("Migration failed: %s", + CMGetCharPtr(s.msg)); + } out: raise_deleted_ind(job);
Dan Smith wrote:
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1205785376 25200 # Node ID 802a5fbe87ed0dff07f2dc75622a9e674d891156 # Parent 47f5fc1ac66dad65785dd050d6b4b9fe95948f8a During a migrate, grab xml, do migration, define at the end ...in a more consistent manner that makes offline migration an effective NOP.
Signed-off-by: Dan Smith <danms@us.ibm.com>
+static CMPIStatus complete_migrate(virDomainPtr ldom, + virConnectPtr rconn, + const char *xml) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + virDomainPtr newdom = NULL; + + if (virDomainUndefine(ldom) == -1) { + CU_DEBUG("Undefine of local domain failed"); + }
And we're okay with that failing? Not questioning the validity, just confirming intent. Other than that, not much to talk about. A bit of it falls under the category of me not seeing glaring code issues and not knowing enough about migration to question the deeper logic behind the change, although I do like the more coherent switch block. So assuming this CU_DEBUG doesn't also need to do some error handling, I'll +1 this. -- -Jay
JG> And we're okay with that failing? Not questioning the validity, JG> just confirming intent. Yeah, I think so, because it means that either (1) the domain has already been undefined (which is okay) or (2) that we'll end up with the domain on both the remote and local machine. Either of these are better than dropping the domain on the floor (i.e. not having it defined in either place and it appearing to just, well, disappear. Agreed? -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
JG> And we're okay with that failing? Not questioning the validity, JG> just confirming intent.
Yeah, I think so, because it means that either (1) the domain has already been undefined (which is okay) or (2) that we'll end up with the domain on both the remote and local machine. Either of these are better than dropping the domain on the floor (i.e. not having it defined in either place and it appearing to just, well, disappear. Agreed?
It's pretty clear that worst-case is dropping it on the floor, I just wasn't sure what our position was on having a domain on both machines. I suppose there's no data loss, and the user can always go, "What are you doing there?" and get rid of the stale one later. Works for me. -- -Jay
participants (2)
-
Dan Smith -
Jay Gagnon