[Libvirt-cim] [PATCH 1 of 3] Define guest before migrating

[Dan Smith]

KR> +static CMPIStatus define_guest_on_target(virConnectPtr dconn,
KR> +                                         virDomainPtr dom,
KR> +                                         struct migration_job *job,
KR> +                                         bool is_offline_migrate)
KR> +{
KR> +        CMPIStatus s = {CMPI_RC_OK, NULL};
KR> +        virDomainPtr ddom = NULL;
KR> +        char *xml = NULL;
KR> +
KR> +        if (!is_offline_migrate) {
KR> +                ddom = virDomainLookupByName(dconn, job->domain);
KR> +                if (ddom == NULL) {
KR> +                        CU_DEBUG("Failed to lookup `%s'", job->domain);
KR> +                        cu_statusf(_BROKER, &s,
KR> +                                   CMPI_RC_ERR_FAILED,
KR> +                                   "Failed to lookup domain `%s'", job->domain);
KR> +                        goto out;
KR> +                }
KR> +
KR> +                xml = virDomainGetXMLDesc(ddom, 0);
KR> +        }
KR> +        else

That 'else' is broken.

KR> +                xml = virDomainGetXMLDesc(dom, 0);
KR> +
KR> +        if (xml == NULL) {
KR> +                cu_statusf(_BROKER, &s,
KR> +                           CMPI_RC_ERR_FAILED,
KR> +                           "Unable to retrieve domain XML.");
KR> +                goto out;
KR> +        }
KR> +
KR> +        ddom = virDomainDefineXML(dconn, xml);
KR> +        if (ddom == NULL) {
KR> +                CU_DEBUG("Failed to define domain from XML");
KR> +                cu_statusf(_BROKER, &s,
KR> +                           CMPI_RC_ERR_FAILED,
KR> +                           "Failed to create domain");
KR> +        }
KR> +
KR> + out:
KR> +        free(xml);
KR> +        virDomainFree(ddom);
KR> +        return s;
KR> +}
KR> +

I really don't like the way this function re-defines an existing guest
on the remote or copies one from local to remote depending on the
flag.  To me it looks like it should be able to just fail if it can't
virDomainGetXMLDesc(dom).  If you pass in a remote dom then it will do
the redefine.  If you pass in a local dom, it will effectively copy.

KR>          switch(job->type) {
KR>          case CIM_MIGRATE_OTHER:
KR>                  CU_DEBUG("Preparing for offline migration");
KR> -                s = handle_offline_migrate(job->conn, dom, uri, xml, job);
KR> +                s = handle_offline_migrate(job->conn, dom, uri, job);
KR>                  break;
KR>          case CIM_MIGRATE_LIVE:
KR>                  CU_DEBUG("Preparing for live migration");
KR> @@ -717,6 +729,15 @@ static CMPIStatus migrate_vs(struct migr
KR>                  goto out;

KR>          CU_DEBUG("Migration succeeded");
KR> +
KR> +        s = define_guest_on_target(job->conn, NULL, job, false);
KR> +        if (s.rc != CMPI_RC_OK) {
KR> +                CU_DEBUG("Define failed");
KR> +                cu_statusf(_BROKER, &s,
KR> +                           CMPI_RC_ERR_FAILED,
KR> +                           "Unable to define guest on target system");
KR> +        }
KR> +
KR>          cu_statusf(_BROKER, &s,
KR>                     CMPI_RC_OK,
KR>                     "");

It seems to me that a migration of any sort has now become like an
offline migration, but with a little extra goo in the middle.  So, I
wonder if we can't change the switch above to something like this:

  xml = virDomainGetXMLDesc(local_dom);
  
  switch (type) {
  case CIM_MIGRATE_OTHER: /* Offline */
      break;
  
  case CIM_MIGRATE_LIVE:
      s = handle_migration(dconn, dom)
      break;

  default:
      /* ERROR */
  }
  
  if (s.rc == CMPI_RC_OK)
      define_guest_on_target(dconn, xml);

Thus, offline migration is mostly just a fall-through, but the other
migration types have actual work in the middle.

Thoughts?  Since Kaitlin is out, I'll pick this up and re-swizzle.

-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms at us.ibm.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 188 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvirt-cim/attachments/20080317/3bfa3cf8/attachment.sig>