9:37 a.m.
Dan Smith wrote:
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?
I'm no Heidi, but it seems like we should be prefixing this. We prefix
every other class we create, and if somebody doesn't want to deal with
the Xen/KVM distinction they are welcome to subscribe to/filter
on/whatever they want the CIM super class and get both, right?
As to where the prefix comes from, I think Kaitlin was on to something
with the ref that comes in from migrate_do, although I think it can be
even easier than she said. We don't actually need to attach the prefix
to the migration_job structure; we're already attaching the classname,
so all we need is a call to class_prefix_name and we've got a prefix.
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);
As was already brought to light by my first run at cu_dup_instance, I
get easily turned around when I start working with actual instances and
such, so I'd like to make sure I follow this. Let's say I fix up
cu_dup_instance so that it creates a new instance using the existing
instances ObjectPath, then copies all the properties correctly. Then I
wouldn't have to call get_typed_instance in the first place, because
everything that it does is already taken care of, right?
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?
Sure that's fine. I didn't think too much about code structure when I
was in "wait is that gonna work?" mode; should have cleaned up a little
more before I sent it out.
--
-Jay