[PATCH 0 of 3] MigrationJob Indications
This set breaks CSMI into three indications that track the status of the migration job through its entire lifecycle. There might be a few rough spots in the main patch here, as a seemingly innocuous typo turned into two days of sfcb debugging. As a result, I'm a bit sick of looking at this thing, so if you see something that's not finished product quality, please point it out. I think the overall approach is fairly sound, since it's a pretty straightforward problem.
# HG changeset patch # User Jay Gagnon <grendel@linux.vnet.ibm.com> # Date 1204900469 18000 # Node ID 1990fcb31b29f8e5247bb42ae67e38131060d486 # Parent 604bd97baadf924a706cdfc99ad8bc4eb2e2a6db MigrationJob schema changes Instead of just ComputerSystemMigrationIndication, we need three ComputerSystemMigrationJob<foo>Indication classes, for Created, Modified, and Deleted. Signed-off-by: Jay Gagnon <grendel@linux.vnet.ibm.com> diff -r 604bd97baadf -r 1990fcb31b29 schema/ComputerSystemMigrationIndication.mof --- a/schema/ComputerSystemMigrationIndication.mof Wed Mar 05 07:02:25 2008 -0800 +++ b/schema/ComputerSystemMigrationIndication.mof Fri Mar 07 09:34:29 2008 -0500 @@ -3,7 +3,23 @@ [Description ("Xen_ComputerSystem migration status"), Provider("cmpi::Virt_ComputerSystemMigrationIndication") ] -class Xen_ComputerSystemMigrationIndication : CIM_InstModification +class Xen_ComputerSystemMigrationJobCreatedIndication : CIM_InstCreation +{ + uint32 RaiseIndication([IN] CIM_InstCreation REF TheIndication); +}; + +[Description ("KVM_ComputerSystem migration status"), + Provider("cmpi::Virt_ComputerSystemMigrationIndication") +] +class KVM_ComputerSystemMigrationJobCreatedIndication : CIM_InstCreation +{ + uint32 RaiseIndication([IN] CIM_InstCreation REF TheIndication); +}; + +[Description ("Xen_ComputerSystem migration status"), + Provider("cmpi::Virt_ComputerSystemMigrationIndication") +] +class Xen_ComputerSystemMigrationJobModifiedIndication : CIM_InstModification { uint32 RaiseIndication([IN] CIM_InstModification REF TheIndication); }; @@ -11,7 +27,23 @@ class Xen_ComputerSystemMigrationIndicat [Description ("KVM_ComputerSystem migration status"), Provider("cmpi::Virt_ComputerSystemMigrationIndication") ] -class KVM_ComputerSystemMigrationIndication : CIM_InstModification +class KVM_ComputerSystemMigrationJobModifiedIndication : CIM_InstModification { uint32 RaiseIndication([IN] CIM_InstModification REF TheIndication); }; + +[Description ("Xen_ComputerSystem migration status"), + Provider("cmpi::Virt_ComputerSystemMigrationIndication") +] +class Xen_ComputerSystemMigrationJobDeletedIndication : CIM_InstDeletion +{ + uint32 RaiseIndication([IN] CIM_InstDeletion REF TheIndication); +}; + +[Description ("KVM_ComputerSystem migration status"), + Provider("cmpi::Virt_ComputerSystemMigrationIndication") +] +class KVM_ComputerSystemMigrationJobDeletedIndication : CIM_InstDeletion +{ + uint32 RaiseIndication([IN] CIM_InstDeletion REF TheIndication); +}; diff -r 604bd97baadf -r 1990fcb31b29 schema/ComputerSystemMigrationIndication.registration --- a/schema/ComputerSystemMigrationIndication.registration Wed Mar 05 07:02:25 2008 -0800 +++ b/schema/ComputerSystemMigrationIndication.registration Fri Mar 07 09:34:29 2008 -0500 @@ -1,4 +1,8 @@ # Copyright IBM Corp. 2007 # Classname Namespace ProviderName ProviderModule ProviderTypes -Xen_ComputerSystemMigrationIndication root/virt Virt_ComputerSystemMigrationIndicationProvider Virt_ComputerSystemMigrationIndication indication method -KVM_ComputerSystemMigrationIndication root/virt Virt_ComputerSystemMigrationIndicationProvider Virt_ComputerSystemMigrationIndication indication method +Xen_ComputerSystemMigrationJobCreatedIndication root/virt Virt_ComputerSystemMigrationIndicationProvider Virt_ComputerSystemMigrationIndication indication method +Xen_ComputerSystemMigrationJobModifiedIndication root/virt Virt_ComputerSystemMigrationIndicationProvider Virt_ComputerSystemMigrationIndication indication method +Xen_ComputerSystemMigrationJobDeletedIndication root/virt Virt_ComputerSystemMigrationIndicationProvider Virt_ComputerSystemMigrationIndication indication method +KVM_ComputerSystemMigrationJobCreatedIndication root/virt Virt_ComputerSystemMigrationIndicationProvider Virt_ComputerSystemMigrationIndication indication method +KVM_ComputerSystemMigrationJobModifiedIndication root/virt Virt_ComputerSystemMigrationIndicationProvider Virt_ComputerSystemMigrationIndication indication method +KVM_ComputerSystemMigrationJobDeletedIndication root/virt Virt_ComputerSystemMigrationIndicationProvider Virt_ComputerSystemMigrationIndication indication method
# HG changeset patch # User Jay Gagnon <grendel@linux.vnet.ibm.com> # Date 1204900472 18000 # Node ID 38cd28b072e3416714053bd049c91a55fca9c0c3 # Parent 1990fcb31b29f8e5247bb42ae67e38131060d486 Convert CSMI for MigrationJob Nothing fancy here, just some filter updates. Signed-off-by: Jay Gagnon <grendel@linux.vnet.ibm.com> diff -r 1990fcb31b29 -r 38cd28b072e3 src/Virt_ComputerSystemMigrationIndication.c --- a/src/Virt_ComputerSystemMigrationIndication.c Fri Mar 07 09:34:29 2008 -0500 +++ b/src/Virt_ComputerSystemMigrationIndication.c Fri Mar 07 09:34:32 2008 -0500 @@ -42,10 +42,14 @@ static const CMPIBroker *_BROKER; -DECLARE_FILTER(xen_migrate, "Xen_ComputerSystemMigrationIndication"); +DECLARE_FILTER(xen_created, "Xen_ComputerSystemMigrationJobCreatedIndication"); +DECLARE_FILTER(xen_mod, "Xen_ComputerSystemMigrationJobModifiedIndication"); +DECLARE_FILTER(xen_deleted, "Xen_ComputerSystemMigrationJobDeletedIndication"); static struct std_ind_filter *filters[] = { - &xen_migrate, + &xen_created, + &xen_mod, + &xen_deleted, NULL, };
# HG changeset patch # User Jay Gagnon <grendel@linux.vnet.ibm.com> # Date 1204901289 18000 # Node ID 1ccab6e9470c08a4dd6f411078325d3574cd9abf # Parent 38cd28b072e3416714053bd049c91a55fca9c0c3 Migration Job Indication We need to add some more indication to VS_MigrationService. ComputerSystemMigrationIndication is getting broken up into ComputerSystemMigrationJob[Created|Modified|Deleted]Indication, so that we can give clients that did not initiate the migration a better idea of what's going on. Each migration will fire a couple additional indications now. Changes required to make this happen: Enumeration of supported indication types. A new function ind_type_to_name. Converts int from the enumeration to an indication name. A new function ref_from_job (migration_job struct -> objectpath). This is mainly existing functionality but is now needed in two places. A new function raise_deleted_ind. The work required to raise the Deleted indication was cluttering up migrate_vs so I broke it out. Changes to prepare_indication and raise_indication to accomodate the new indication types. Then of course a few lines to actually use the new stuff. Signed-off-by: Jay Gagnon <grendel@linux.vnet.ibm.com> diff -r 38cd28b072e3 -r 1ccab6e9470c src/Virt_VSMigrationService.c --- a/src/Virt_VSMigrationService.c Fri Mar 07 09:34:32 2008 -0500 +++ b/src/Virt_VSMigrationService.c Fri Mar 07 09:48:09 2008 -0500 @@ -51,6 +51,12 @@ const static CMPIBroker *_BROKER; +enum { + MIG_CREATED, + MIG_MODIFIED, + MIG_DELETED, +}; + struct migration_job { CMPIContext *context; char *domain; @@ -271,23 +277,49 @@ static CMPIStatus vs_migratable_system(C return vs_migratable(ref, name, dname, results, argsout); } +static char *ind_type_to_name(int ind_type) +{ + char *ind_name = NULL; + + switch (ind_type) { + case MIG_CREATED: + ind_name = "ComputerSystemMigrationJobCreatedIndication"; + break; + case MIG_DELETED: + ind_name = "ComputerSystemMigrationJobDeletedIndication"; + break; + case MIG_MODIFIED: + ind_name = "ComputerSystemMigrationJobModifiedIndication"; + break; + } + + return ind_name; +} + static bool raise_indication(const CMPIContext *context, - const char *base_type, + int ind_type, const char *ns, CMPIInstance *inst, CMPIInstance *ind) { char *type; CMPIStatus s; + char *ind_name = NULL; + + ind_name = ind_type_to_name(ind_type); CU_DEBUG("Setting SourceInstance"); CMSetProperty(ind, "SourceInstance", (CMPIValue *)&inst, CMPI_instance); /* Seems like this shouldn't be hardcoded. */ - type = get_typed_class("Xen", base_type); - + type = get_typed_class("Xen", ind_name); + + CU_DEBUG("stdi_raise"); + CU_DEBUG("broker %p, context %p, type %s, ns %s, ind %p", + _BROKER, context, type, ns, ind); s = stdi_raise_indication(_BROKER, context, type, ns, ind); + CU_DEBUG("raise done"); free(type); @@ -297,41 +329,95 @@ static CMPIInstance *prepare_indication( static CMPIInstance *prepare_indication(const CMPIBroker *broker, CMPIInstance *inst, char *ns, + int ind_type, CMPIStatus *s) { + char *ind_name = NULL; CMPIInstance *ind = NULL; CMPIInstance *prev_inst = NULL; - CU_DEBUG("Creating indication."); + ind_name = ind_type_to_name(ind_type); + + CU_DEBUG("Creating indication: '%s:%s_%s'", + ns, "Xen", ind_name); /* Prefix needs to be dynamic */ - ind = get_typed_instance(broker, - "Xen", - "ComputerSystemMigrationIndication", - ns); + ind = get_typed_instance(broker, "Xen", ind_name, ns); /* Prefix needs to be dynamic */ if (ind == NULL) { CU_DEBUG("Failed to create ind, type '%s:%s_%s'", - ns, - "Xen", - "ComputerSystemMigrationIndication"); - } - - /* Need to copy job inst before attaching as PreviousInstance because - otherwise the changes we are about to make to job inst are made - to PreviousInstance as well. */ - prev_inst = cu_dup_instance(_BROKER, inst, s); - if (s->rc != CMPI_RC_OK || prev_inst == NULL) { - CU_DEBUG("dup_instance failed (%i:%s)", s->rc, s->msg); - ind = NULL; - goto out; - } - - CU_DEBUG("Setting PreviousInstance"); - CMSetProperty(ind, "PreviousInstance", - (CMPIValue *)&prev_inst, CMPI_instance); + ns, "Xen", ind_name); + } + + if (ind_type == MIG_MODIFIED) { + /* Need to copy job inst before attaching as PreviousInstance because + otherwise the changes we are about to make to job inst are made + to PreviousInstance as well. */ + prev_inst = cu_dup_instance(_BROKER, inst, s); + if (s->rc != CMPI_RC_OK || prev_inst == NULL) { + CU_DEBUG("dup_instance failed (%i:%s)", s->rc, s->msg); + ind = NULL; + goto out; + } + CU_DEBUG("Setting PreviousInstance"); + CMSetProperty(ind, "PreviousInstance", + (CMPIValue *)&prev_inst, CMPI_instance); + } out: return ind; +} + +static CMPIObjectPath *ref_from_job(struct migration_job *job, + CMPIStatus *s) +{ + CMPIObjectPath *ref = NULL; + + ref = CMNewObjectPath(_BROKER, + job->ref_ns, + "Virt_MigrationJob", + s); + if (s->rc != CMPI_RC_OK) { + CU_DEBUG("Failed to create job ref for update"); + goto out; + } + + CMSetNameSpace(ref, job->ref_ns); + CMAddKey(ref, "InstanceID", (CMPIValue *)job->uuid, CMPI_chars); + + CU_DEBUG("Getting job instance %s", job->uuid); + CU_DEBUG(" REF: %s", CMGetCharPtr(CMObjectPathToString(ref, NULL))); + + out: + return ref; +} + +static void raise_deleted_ind(struct migration_job *job) +{ + CMPIInstance *ind = NULL; + CMPIInstance *inst = NULL; + CMPIObjectPath *ref = NULL; + CMPIStatus s = {CMPI_RC_OK, NULL}; + bool rc; + + ref = ref_from_job(job, &s); + if ((ref == NULL) || (s.rc != CMPI_RC_OK)) { + CU_DEBUG("Failed to get job ref for delete"); + return; + } + inst = CBGetInstance(_BROKER, job->context, ref, NULL, &s); + if ((inst == NULL) || (s.rc != CMPI_RC_OK)) { + CU_DEBUG("Failed to get job instance for delete (%i)", s.rc); + return; + } + + ind = prepare_indication(_BROKER, inst, job->ref_ns, MIG_DELETED, &s); + + rc = raise_indication(job->context, MIG_MODIFIED, job->ref_ns, + inst, ind); + if (!rc) + CU_DEBUG("Failed to raise indication"); + + return; } static void migrate_job_set_state(struct migration_job *job, @@ -344,27 +430,18 @@ static void migrate_job_set_state(struct CMPIStatus s; CMPIObjectPath *op; - op = CMNewObjectPath(_BROKER, - job->ref_ns, - "Virt_MigrationJob", - &s); - if (s.rc != CMPI_RC_OK) { - CU_DEBUG("Failed to create job path for update"); + op = ref_from_job(job, &s); + if ((op == NULL) || (s.rc != CMPI_RC_OK)) { + CU_DEBUG("Failed to get job ref for update"); return; } - - CMSetNameSpace(op, job->ref_ns); - CMAddKey(op, "InstanceID", (CMPIValue *)job->uuid, CMPI_chars); - - CU_DEBUG("Getting job instance %s", job->uuid); - CU_DEBUG(" OP: %s", CMGetCharPtr(CMObjectPathToString(op, NULL))); inst = CBGetInstance(_BROKER, job->context, op, NULL, &s); if ((inst == NULL) || (s.rc != CMPI_RC_OK)) { CU_DEBUG("Failed to get job instance for update (%i)", s.rc); return; } - ind = prepare_indication(_BROKER, inst, job->ref_ns, &s); + ind = prepare_indication(_BROKER, inst, job->ref_ns, MIG_MODIFIED, &s); CMSetProperty(inst, "JobState", (CMPIValue *)&state, CMPI_uint16); @@ -378,11 +455,8 @@ static void migrate_job_set_state(struct CU_DEBUG("Failed to update job instance: %s", CMGetCharPtr(s.msg)); - rc = raise_indication(job->context, - "ComputerSystemMigrationIndication", - job->ref_ns, - inst, - ind); + rc = raise_indication(job->context, MIG_MODIFIED, job->ref_ns, + inst, ind); if (!rc) CU_DEBUG("Failed to raise indication"); } @@ -549,6 +623,8 @@ static CMPIStatus migrate_vs(struct migr ""); out: + raise_deleted_ind(job); + free(uri); free(xml); virDomainFree(dom); @@ -714,6 +790,9 @@ static CMPIStatus migrate_do(const CMPIO struct migration_job *job; CMPI_THREAD_TYPE thread; uint32_t retcode = 1; + CMPIInstance *ind = NULL; + CMPIInstance *inst = NULL; + bool rc; job = migrate_job_prepare(context, ref, domain, host, type); if (job == NULL) { @@ -728,8 +807,20 @@ static CMPIStatus migrate_do(const CMPIO s = migrate_create_job_instance(context, job, &job_op); if (s.rc != CMPI_RC_OK) goto out; - + CMAddArg(argsout, "Job", (CMPIValue *)&job_op, CMPI_ref); + + inst = CBGetInstance(_BROKER, job->context, job_op, NULL, &s); + if ((inst == NULL) || (s.rc != CMPI_RC_OK)) { + CU_DEBUG("Failed to get job instance for create ind", s.rc); + goto out; + } + + ind = prepare_indication(_BROKER, inst, job->ref_ns, MIG_CREATED, &s); + rc = raise_indication(job->context, MIG_CREATED, job->ref_ns, + inst, ind); + if (!rc) + CU_DEBUG("Failed to raise indication"); thread = _BROKER->xft->newThread((void*)migration_thread, job, 0);
JG> +static char *ind_type_to_name(int ind_type) const. JG> +{ JG> + char *ind_name = NULL; const. JG> + switch (ind_type) { JG> + case MIG_CREATED: JG> + ind_name = "ComputerSystemMigrationJobCreatedIndication"; JG> + break; JG> + case MIG_DELETED: JG> + ind_name = "ComputerSystemMigrationJobDeletedIndication"; JG> + break; JG> + case MIG_MODIFIED: JG> + ind_name = "ComputerSystemMigrationJobModifiedIndication"; JG> + break; JG> + } JG> + JG> + return ind_name; JG> +} JG> + JG> static bool raise_indication(const CMPIContext *context, JG> - const char *base_type, JG> + int ind_type, JG> const char *ns, JG> CMPIInstance *inst, JG> CMPIInstance *ind) JG> { JG> char *type; JG> CMPIStatus s; JG> + char *ind_name = NULL; const. JG> + JG> + ind_name = ind_type_to_name(ind_type); JG> CU_DEBUG("Setting SourceInstance"); JG> CMSetProperty(ind, "SourceInstance", JG> (CMPIValue *)&inst, CMPI_instance); JG> /* Seems like this shouldn't be hardcoded. */ JG> - type = get_typed_class("Xen", base_type); JG> - JG> + type = get_typed_class("Xen", ind_name); JG> + JG> + CU_DEBUG("stdi_raise"); JG> + CU_DEBUG("broker %p, context %p, type %s, ns %s, ind %p", JG> + _BROKER, context, type, ns, ind); JG> s = stdi_raise_indication(_BROKER, context, type, ns, ind); JG> + CU_DEBUG("raise done"); This debug looks a little overkill, likely related to your fight with sfcb. JG> +static CMPIObjectPath *ref_from_job(struct migration_job *job, JG> + CMPIStatus *s) JG> +{ JG> + CMPIObjectPath *ref = NULL; JG> + JG> + ref = CMNewObjectPath(_BROKER, JG> + job->ref_ns, JG> + "Virt_MigrationJob", JG> + s); JG> + if (s->rc != CMPI_RC_OK) { JG> + CU_DEBUG("Failed to create job ref for update"); JG> + goto out; JG> + } JG> + JG> + CMSetNameSpace(ref, job->ref_ns); JG> + CMAddKey(ref, "InstanceID", (CMPIValue *)job->uuid, CMPI_chars); JG> + JG> + CU_DEBUG("Getting job instance %s", job->uuid); JG> + CU_DEBUG(" REF: %s", CMGetCharPtr(CMObjectPathToString(ref, NULL))); This debug is a little misleading, isn't it? Doesn't look like you're about to do anything like "Getting job instance" right after this. JG> - rc = raise_indication(job->context, JG> - "ComputerSystemMigrationIndication", JG> - job->ref_ns, JG> - inst, JG> - ind); JG> + rc = raise_indication(job->context, MIG_MODIFIED, job->ref_ns, JG> + inst, ind); Bzzzt. I just called Heidi on this, so I'm not going to let you slide either... :) -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
JG> +static char *ind_type_to_name(int ind_type)
const.
JG> +{ JG> + char *ind_name = NULL;
const.
JG> + switch (ind_type) { JG> + case MIG_CREATED: JG> + ind_name = "ComputerSystemMigrationJobCreatedIndication"; JG> + break; JG> + case MIG_DELETED: JG> + ind_name = "ComputerSystemMigrationJobDeletedIndication"; JG> + break; JG> + case MIG_MODIFIED: JG> + ind_name = "ComputerSystemMigrationJobModifiedIndication"; JG> + break; JG> + } JG> + JG> + return ind_name; JG> +} JG> + JG> static bool raise_indication(const CMPIContext *context, JG> - const char *base_type, JG> + int ind_type, JG> const char *ns, JG> CMPIInstance *inst, JG> CMPIInstance *ind) JG> { JG> char *type; JG> CMPIStatus s; JG> + char *ind_name = NULL;
const.
I'm never gonna figure that out, am I?
JG> + JG> + ind_name = ind_type_to_name(ind_type);
JG> CU_DEBUG("Setting SourceInstance"); JG> CMSetProperty(ind, "SourceInstance", JG> (CMPIValue *)&inst, CMPI_instance);
JG> /* Seems like this shouldn't be hardcoded. */ JG> - type = get_typed_class("Xen", base_type); JG> - JG> + type = get_typed_class("Xen", ind_name); JG> + JG> + CU_DEBUG("stdi_raise"); JG> + CU_DEBUG("broker %p, context %p, type %s, ns %s, ind %p", JG> + _BROKER, context, type, ns, ind); JG> s = stdi_raise_indication(_BROKER, context, type, ns, ind); JG> + CU_DEBUG("raise done");
This debug looks a little overkill, likely related to your fight with sfcb.
Yea, that's a leftover. It can go.
JG> +static CMPIObjectPath *ref_from_job(struct migration_job *job, JG> + CMPIStatus *s) JG> +{ JG> + CMPIObjectPath *ref = NULL; JG> + JG> + ref = CMNewObjectPath(_BROKER, JG> + job->ref_ns, JG> + "Virt_MigrationJob", JG> + s); JG> + if (s->rc != CMPI_RC_OK) { JG> + CU_DEBUG("Failed to create job ref for update"); JG> + goto out; JG> + } JG> + JG> + CMSetNameSpace(ref, job->ref_ns); JG> + CMAddKey(ref, "InstanceID", (CMPIValue *)job->uuid, CMPI_chars); JG> + JG> + CU_DEBUG("Getting job instance %s", job->uuid); JG> + CU_DEBUG(" REF: %s", CMGetCharPtr(CMObjectPathToString(ref, NULL)));
This debug is a little misleading, isn't it? Doesn't look like you're about to do anything like "Getting job instance" right after this.
I'm the victim of a cut-paste there. That debug made sense when it was actually right before the code that got the instance, but when I separated the ref code from the inst code I totally missed it. I still like having the ref output there, though. It's a convenient double-check.
JG> - rc = raise_indication(job->context, JG> - "ComputerSystemMigrationIndication", JG> - job->ref_ns, JG> - inst, JG> - ind); JG> + rc = raise_indication(job->context, MIG_MODIFIED, job->ref_ns, JG> + inst, ind);
Bzzzt. I just called Heidi on this, so I'm not going to let you slide either... :)
Grumble, grumble... -- -Jay
JG> I'm the victim of a cut-paste there. That debug made sense when JG> it was actually right before the code that got the instance, but JG> when I separated the ref code from the inst code I totally missed JG> it. I still like having the ref output there, though. It's a JG> convenient double-check. Indeed, just make the message a little more accurate, like "Here is my ref:" or something :) -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
participants (2)
-
Dan Smith -
Jay Gagnon