[Libvirt-cim] [PATCH 3 of 3] Migration Job Indication

[Jay Gagnon]

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