[PATCH] [RFC] #2 - variable renaming in make_ref() of associations
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1196858495 -3600 # Node ID af64285fa953aca6c4183d0fd6845d110ddd1a1d # Parent 012fc8655c2b443511ba536ef2b425ac86f69f7b [RFC] #2 - variable renaming in make_ref() of associations Suggestion to rename some of the variables in make_ref() to make the relations clearer. Also moved the content of make_ref() to make_reference() in libxkutil to generalize it. This make_reference can then be used by each association's make_ref(). Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r 012fc8655c2b -r af64285fa953 libxkutil/misc_util.c --- a/libxkutil/misc_util.c Wed Dec 05 12:49:18 2007 +0100 +++ b/libxkutil/misc_util.c Wed Dec 05 13:41:35 2007 +0100 @@ -342,7 +342,30 @@ bool match_hypervisor_prefix(const CMPIO return rc; } - +CMPIInstance *make_reference(const CMPIBroker *broker, + const CMPIObjectPath *source_ref, + const CMPIInstance *target_inst, + struct std_assoc *assoc, + const char *assoc_classname) +{ + + CMPIInstance *ref_inst = NULL; + + ref_inst = get_typed_instance(broker, + CLASSNAME(source_ref), + assoc_classname, + NAMESPACE(source_ref)); + + if (ref_inst != NULL) { + CMPIObjectPath *target_ref; + + target_ref = CMGetObjectPath(target_inst, NULL); + + set_reference(assoc, ref_inst, source_ref, target_ref); + } + + return ref_inst; +} bool domain_online(virDomainPtr dom) { diff -r 012fc8655c2b -r af64285fa953 libxkutil/misc_util.h --- a/libxkutil/misc_util.h Wed Dec 05 12:49:18 2007 +0100 +++ b/libxkutil/misc_util.h Wed Dec 05 13:41:35 2007 +0100 @@ -109,6 +109,12 @@ bool match_hypervisor_prefix(const CMPIO bool match_hypervisor_prefix(const CMPIObjectPath *reference, struct std_assoc_info *info); +CMPIInstance *make_reference(const CMPIBroker *broker, + const CMPIObjectPath *source_ref, + const CMPIInstance *target_inst, + struct std_assoc *assoc, + const char *assoc_classname); + /* * Local Variables: * mode: C diff -r 012fc8655c2b -r af64285fa953 src/Virt_ElementAllocatedFromPool.c --- a/src/Virt_ElementAllocatedFromPool.c Wed Dec 05 12:49:18 2007 +0100 +++ b/src/Virt_ElementAllocatedFromPool.c Wed Dec 05 13:41:35 2007 +0100 @@ -247,27 +247,20 @@ 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_ref, + 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 *ref_inst = NULL; + + ref_inst = make_reference(_BROKER, + source_ref, + target_inst, + assoc, + "ElementAllocatedFromPool"); + + return ref_inst; } char* antecedent[] = {
Heidi Eckhart wrote:
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1196858495 -3600 # Node ID af64285fa953aca6c4183d0fd6845d110ddd1a1d # Parent 012fc8655c2b443511ba536ef2b425ac86f69f7b [RFC] #2 - variable renaming in make_ref() of associations
Suggestion to rename some of the variables in make_ref() to make the relations clearer. Also moved the content of make_ref() to make_reference() in libxkutil to generalize it. This make_reference can then be used by each association's make_ref(). Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com>
I like this. The rename is definitely more worthwhile if we generalize it, and I agree that it really can be generalized, as this patch shows. My only question is: would this be better suited to being part of std_association instead of libxkutil? Seems like anything that will use std_association will probably want to use make_reference(). -- -Jay
JG> My only question is: would this be better suited to being part of JG> std_association instead of libxkutil? Seems like anything that JG> will use std_association will probably want to use JG> make_reference(). No, the make_ref behavior is something we need to do in a special way that others won't. Putting the (Xen_|KVM_) switch into std_association would pollute libcmpiutil with libvirt-cim behavior. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
JG> My only question is: would this be better suited to being part of JG> std_association instead of libxkutil? Seems like anything that JG> will use std_association will probably want to use JG> make_reference().
Yes, that was my first try, until I had to realize that get_typed_instance() is that special to our providers as Dan describes below ;). So I had to move it to libxkutil.
No, the make_ref behavior is something we need to do in a special way that others won't. Putting the (Xen_|KVM_) switch into std_association would pollute libcmpiutil with libvirt-cim behavior.
-- Regards Heidi Eckhart Software Engineer Linux Technology Center - Open Hypervisor heidieck@linux.vnet.ibm.com ************************************************** IBM Deutschland Entwicklung GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschaeftsfuehrung: Herbert Kircher Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
HE> # HG changeset patch HE> # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> HE> # Date 1196858495 -3600 HE> # Node ID af64285fa953aca6c4183d0fd6845d110ddd1a1d HE> # Parent 012fc8655c2b443511ba536ef2b425ac86f69f7b HE> [RFC] #2 - variable renaming in make_ref() of associations HE> Suggestion to rename some of the variables in make_ref() HE> to make the relations clearer. HE> Also moved the content of make_ref() to make_reference() HE> in libxkutil to generalize it. This make_reference can HE> then be used by each association's make_ref(). Sounds good to me. HE> + CMPIInstance *ref_inst = NULL; HE> + HE> + ref_inst = make_reference(_BROKER, HE> + source_ref, HE> + target_inst, HE> + assoc, HE> + "ElementAllocatedFromPool"); HE> + HE> + return ref_inst; HE> } Why not just pass the make_reference function from libxkutil directly in the std_assoc block? The function should be able to infer the name of the reference from the assoc_class field of the std_assoc or info struct, right? That would let us cut out even more code from all the providers :) -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
HE> + CMPIInstance *ref_inst = NULL; HE> + HE> + ref_inst = make_reference(_BROKER, HE> + source_ref, HE> + target_inst, HE> + assoc, HE> + "ElementAllocatedFromPool"); HE> + HE> + return ref_inst; HE> }
Why not just pass the make_reference function from libxkutil directly in the std_assoc block? The function should be able to infer the name of the reference from the assoc_class field of the std_assoc or info struct, right?
That would let us cut out even more code from all the providers :) That's a ery interesting idea ! Sadly the current input parameter list of make_ref_t() in struct std_assoc does not include a broker pointer, which prevents me from reworking it in your suggested way. Maybe you have an idea how to solve this ? Thanks :).
-- Regards Heidi Eckhart Software Engineer Linux Technology Center - Open Hypervisor heidieck@linux.vnet.ibm.com ************************************************** IBM Deutschland Entwicklung GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschaeftsfuehrung: Herbert Kircher Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
HE> Sadly the current input parameter list of make_ref_t() in struct HE> std_assoc does not include a broker pointer, which prevents me HE> from reworking it in your suggested way. Ah, right :( HE> Maybe you have an idea how to solve this ? Perhaps we could have a macro that dispatches the make_reference() function with the appropriate broker? Maybe something like the following: #define LIBVIRT_CIM_DEFAULT_MAKEREF \ static CMPIInstance make_ref(const CMPIObjectPath *ref, \ const CMPIInstance *inst, \ struct std_assoc_info *info, \ struct std_assoc *assoc) \ { \ return default_make_ref(_BROKER, ref, inst, info, assoc);\ } What do you think? -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
HE> Sadly the current input parameter list of make_ref_t() in struct HE> std_assoc does not include a broker pointer, which prevents me HE> from reworking it in your suggested way.
Ah, right :(
HE> Maybe you have an idea how to solve this ?
Perhaps we could have a macro that dispatches the make_reference() function with the appropriate broker? Maybe something like the following:
#define LIBVIRT_CIM_DEFAULT_MAKEREF \ static CMPIInstance make_ref(const CMPIObjectPath *ref, \ const CMPIInstance *inst, \ struct std_assoc_info *info, \ struct std_assoc *assoc) \ { \ return default_make_ref(_BROKER, ref, inst, info, assoc);\ }
What do you think?
I think this is a neat idea. -- Kaitlin Rupert IBM Linux Technology Center karupert@us.ibm.com
participants (4)
-
Dan Smith -
Heidi Eckhart -
Jay Gagnon -
Kaitlin Rupert