[Libvirt-cim] [PATCH 1 of 2] Add calls into VSMigrationService to utilize new MigrationIndication provider

[Dan Smith]

JG> +static bool raise_indication(const CMPIContext *context,
JG> +                             const char *base_type,
JG> +                             const char *ns,
JG> +                             const CMPIInstance *ind)
JG> +{
JG> +        char *type;
JG> +        CMPIStatus s;
JG> +
JG> +        /* Seems like this shouldn't be hardcoded. */
JG> +        type = get_typed_class("Xen", base_type);
JG> +
JG> +        s = stdi_raise_indication(_BROKER, context, type, ns, ind);
JG> +
JG> +        free(type);
JG> +
JG> +        return s.rc == CMPI_RC_OK;
JG> +}

So, this actually brings up an interesting point which I think we
glazed over when we put the migration provider in the first time.

I left the migration job classname as Virt_MigrationJob, thus not
having a specific platform prefix on it.  I did that because it seemed
to me that clients could potentially use the job as an entry point,
needing to see what guests were leaving a host at any point, and then
following the ref's to platform-specific details.

However, as Jay points out, I think that the indication should be
typed per-platform as usual.  So, should we type the job to match,
since we'll need to persist the information for typing the indication
as well?

Heidi, maybe you have some input here?


JG> @@ -297,12 +319,50 @@ static void migrate_job_set_state(struct
JG>          CMSetProperty(inst, "Status",
JG>                        (CMPIValue *)status, CMPI_chars);

JG> +        CU_DEBUG("Creating indication.");
JG> +        /* Prefix needs to be dynamic */
JG> +        ind = get_typed_instance(_BROKER,
JG> +                                 "Xen",
JG> +                                 "ComputerSystemMigrationIndication",
JG> +                                 job->ref_ns);
JG> +        /* Prefix needs to be dynamic */
JG> +        if (ind == NULL) {
JG> +                CU_DEBUG("Failed to create ind, type '%s:%s_%s'", 
JG> +                         job->ref_ns,
JG> +                         "Xen",
JG> +                         "ComputerSystemMigrationIndication");
JG> +        }
JG> +
JG> +        /* Need to copy job inst before attaching as PreviousInstance because 
JG> +           otherwise the changes we are about to make to job inst are made 
JG> +           to PreviousInstance as well. */
JG> +        s = cu_dup_instance(_BROKER, inst, &prev_inst);

I didn't quite understand from the other patch, but I don't think this
is what you want to do.  You create an instance with a set of
properties set (like CCN) with the get_typed_instance() call, but then
just overwrite the ObjectPath of it here.

I think the semantics of the cu_dup_instance() call should be that it
creates you a new instance.  Something like this maybe:

  new_inst = cu_dup_instance(_BROKER, src_inst, &s);

JG> +        if (s.rc != CMPI_RC_OK || prev_inst == NULL) {
JG> +                CU_DEBUG("dup_instance failed (%i:%s)", s.rc, s.msg);
JG> +                return;
JG> +        }
JG> +
JG> +        CU_DEBUG("Setting PreviousInstance");
JG> +        CMSetProperty(ind, "PreviousInstance", 
JG> +                      (CMPIValue *)&prev_inst, CMPI_instance);
JG> +
JG>          CU_DEBUG("Modifying job %s (%i:%s)", job->uuid, state, status);

JG>          s = CBModifyInstance(_BROKER, job->context, op, inst, NULL);
JG>          if (s.rc != CMPI_RC_OK)
JG>                  CU_DEBUG("Failed to update job instance: %s",
JG>                           CMGetCharPtr(s.msg));
JG> +
JG> +        CU_DEBUG("Setting SourceInstance");
JG> +        CMSetProperty(ind, "SourceInstance",
JG> +                      (CMPIValue *)&inst, CMPI_instance);
JG> +
JG> +        rc = raise_indication(job->context,
JG> +                              "ComputerSystemMigrationIndication",
JG> +                              job->ref_ns,
JG> +                              ind);
JG> +        if (!rc)
JG> +                CU_DEBUG("Failed to raise indication");
JG>  }

These changes make the migrate_job_set_state() function quite a bit
longer.  Can we break out these out into a separate function?  If it
just returns an indication instance, then maybe something like this
could work:

  ind = prepare_indication(job_inst);

  /* Existing job_inst update code */

  raise_indication(ind, job_inst);

The prepare call would dup the old instance and create the indication
instance, and the raise call would add the updated version and do the
actual raise.  What do you think?

-- 
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/20080201/670d48fc/attachment.sig>