Heidi Eckhart wrote:
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1196686834 -3600 # Node ID bea3e027d42ff41fc452935427981739bab76573 # Parent a1582d092f517919470b9ce7ff034b89e4b2bade [RFC] make_ref() of associations
While thinking about the implementation of make_ref() and about my proposal to base the asscociation's instance creation on a connect_by_classname, I came to the conclusion that opening a connection to libvirt for not using it, is overkill for this method. Once make_ref() gets called by the std_association logic, the provider can rely on that the prefix of the given reference was checked for the right hypervisor prefix. So I suggest to update all make_ref() functions to use get_typed_instance(). ... and refix some of my fixes :0. Besides that I suggest to rename some of the variables in make_ref() to make the relations clearer. Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com>
diff -r a1582d092f51 -r bea3e027d42f src/Virt_ElementAllocatedFromPool.c --- a/src/Virt_ElementAllocatedFromPool.c Mon Dec 03 13:49:01 2007 +0100 +++ b/src/Virt_ElementAllocatedFromPool.c Mon Dec 03 14:00:34 2007 +0100 @@ -241,27 +241,27 @@ static CMPIStatus pool_to_vdev(const CMP return s; }
-static CMPIInstance *make_ref(const CMPIObjectPath *ref, - const CMPIInstance *inst, +static CMPIInstance *make_ref(const CMPIObjectPath *source_op, + const CMPIInstance *target_inst, struct std_assoc_info *info, struct std_assoc *assoc) { - CMPIInstance *refinst = NULL; - - refinst = get_typed_instance(_BROKER, - CLASSNAME(ref), - "ElementAllocatedFromPool", - NAMESPACE(ref)); - - if (refinst != NULL) { - CMPIObjectPath *instop; - - instop = CMGetObjectPath(inst, NULL); - - set_reference(assoc, refinst, ref, instop); - } - - return refinst; + CMPIInstance *assoc_inst = NULL; + + assoc_inst = get_typed_instance(_BROKER, + CLASSNAME(source_op), + "ElementAllocatedFromPool", + NAMESPACE(source_op)); + + if (assoc_inst != NULL) { + CMPIObjectPath *target_op; + + target_op = CMGetObjectPath(target_inst, NULL); + + set_reference(assoc, assoc_inst, source_op, target_op); + } + + return assoc_inst; }
char* antecedent[] = {
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
Couple of things here. First thing, I'm okay with renaming things to make it more clear, but I'm not really a fan of using "op" for ObjectPath. I realize it's a more logical abbreviation than "ref", but we use "ref" in so many places that I think it's pretty firmly established, and I personally always see "op" and think "operand" or "operation." I would recommend a compromise of "source_ref" for that one. Second thing, I think I'm missing something in the first paragraph of your commit message. I'm not entirely sure I understand the explanation that leads up to "So I suggest to update all make_ref() functions to use get_typed_instance()," and it also doesn't seem to really relate to this patch, which only appears to do some renaming in make_ref. It seems like a good point of discussion, and clarification would be good, but maybe we should move it to a separate thread and limit this patch discussion to the variable naming. -- -Jay