[PATCH 0 of 4] #3 - Enable VSSD to validate client given object path and Adopt changed to VSSD interface to ESD & SDS & VSSDC
VSSD does now take care of the client given object path. The resulting interface changes have been adopted to - ESD by the second patch - SDS by the third patch - VSSDC by the fourth patch (diff to patch set 1) diff to patch set 2: - updated VSSD to compare a returned integer result explicitely
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1203672857 -3600 # Node ID 644476f8ee52477f05fc3080baa517f697497eee # Parent d72742466a8892154997d2f210cb9ceeff035962 VSSD: validate client given object path wbemgi 'http://localhost:5988/root/virt:KVM_VirtualSystemSettingData.InstanceID="Xen:kvm1-f8"' on a KVM only system is returning an instance, but should not. wbemgi 'http://localhost:5988/root/virt:Xen_VirtualSystemSettingData.InstanceID="KVM:kvm1-f8"' on a KVM only system is seg faulting, but should not ;). Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r d72742466a88 -r 644476f8ee52 src/Virt_VSSD.c --- a/src/Virt_VSSD.c Tue Feb 19 15:29:52 2008 -0800 +++ b/src/Virt_VSSD.c Fri Feb 22 10:34:17 2008 +0100 @@ -135,42 +135,46 @@ static int instance_from_dom(virDomainPt return ret; } -CMPIInstance *get_vssd_instance(virDomainPtr dom, - const CMPIBroker *broker, - const CMPIObjectPath *ref) -{ - CMPIInstance *inst; - +static CMPIInstance *_get_vssd(const CMPIBroker *broker, + const CMPIObjectPath *reference, + virConnectPtr conn, + virDomainPtr dom, + CMPIStatus *s) +{ + CMPIInstance *inst = NULL; + inst = get_typed_instance(broker, - CLASSNAME(ref), + pfx_from_conn(conn), "VirtualSystemSettingData", - NAMESPACE(ref)); - - if (inst == NULL) - return NULL; - - if (instance_from_dom(dom, inst)) - return inst; - else - return NULL; -} - -static CMPIStatus enum_vssd(const CMPIObjectPath *reference, - const CMPIResult *results, - int names_only) + NAMESPACE(reference)); + + if (inst == NULL) { + cu_statusf(broker, s, + CMPI_RC_ERR_FAILED, + "Unable to init VirtualSystemSettingData instance"); + goto out; + } + + if (instance_from_dom(dom, inst) != 1) { + cu_statusf(broker, s, + CMPI_RC_ERR_FAILED, + "Unable to get VSSD instance from Domain"); + } + + out: + return inst; +} + +static CMPIStatus return_enum_vssd(const CMPIObjectPath *reference, + const CMPIResult *results, + bool names_only) { virConnectPtr conn; virDomainPtr *list; int count; int i; - CMPIStatus s; - const char *ns; - - if (!provider_is_responsible(_BROKER, reference, &s)) - return s; - - ns = NAMESPACE(reference); - + CMPIStatus s = {CMPI_RC_OK, NULL}; + conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s); if (conn == NULL) return s; @@ -181,60 +185,90 @@ static CMPIStatus enum_vssd(const CMPIOb CMPI_RC_ERR_FAILED, "Failed to enumerate domains"); goto out; - } else if (count == 0) { - cu_statusf(_BROKER, &s, - CMPI_RC_OK, - ""); - goto out; - } + } else if (count == 0) + goto out; for (i = 0; i < count; i++) { - CMPIInstance *inst; - - inst = get_vssd_instance(list[i], _BROKER, reference); + CMPIInstance *inst = NULL; + + inst = _get_vssd(_BROKER, reference, conn, list[i], &s); + virDomainFree(list[i]); if (inst == NULL) continue; - + if (names_only) cu_return_instance_name(results, inst); else CMReturnInstance(results, inst); } - cu_statusf(_BROKER, &s, - CMPI_RC_OK, - ""); out: free(list); return s; - -} - -static CMPIInstance *get_vssd_for_name(const CMPIObjectPath *reference, - char *name) +} + +CMPIStatus get_vssd_by_name(const CMPIBroker *broker, + const CMPIObjectPath *reference, + const char *name, + CMPIInstance **_inst) { virConnectPtr conn; virDomainPtr dom; - CMPIStatus s; - CMPIInstance *inst = NULL; - - conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s); - if (conn == NULL) - return NULL; - + CMPIStatus s = {CMPI_RC_OK, NULL}; + + conn = connect_by_classname(broker, CLASSNAME(reference), &s); + if (conn == NULL) { + cu_statusf(broker, &s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance"); + goto out; + } + dom = virDomainLookupByName(conn, name); - if (dom == NULL) - goto out; - - inst = get_vssd_instance(dom, _BROKER, reference); - + if (dom == NULL) { + cu_statusf(broker, &s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance (%s)", + name); + goto out; + } + + *_inst = _get_vssd(broker, reference, conn, dom, &s); + + virDomainFree(dom); + out: virConnectClose(conn); - virDomainFree(dom); - - return inst; + + return s; +} + +CMPIStatus get_vssd_by_ref(const CMPIBroker *broker, + const CMPIObjectPath *reference, + CMPIInstance **_inst) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + char *name = NULL; + + if (!parse_instanceid(reference, NULL, &name)) { + cu_statusf(broker, &s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance (InstanceID)"); + goto out; + } + + s = get_vssd_by_name(broker, reference, name, _inst); + if (s.rc != CMPI_RC_OK) + goto out; + + s = cu_validate_ref(broker, reference, *_inst); + + free(name); + + out: + return s; } static CMPIStatus EnumInstanceNames(CMPIInstanceMI *self, @@ -242,7 +276,7 @@ static CMPIStatus EnumInstanceNames(CMPI const CMPIResult *results, const CMPIObjectPath *reference) { - return enum_vssd(reference, results, 1); + return return_enum_vssd(reference, results, true); } static CMPIStatus EnumInstances(CMPIInstanceMI *self, @@ -251,7 +285,7 @@ static CMPIStatus EnumInstances(CMPIInst const CMPIObjectPath *reference, const char **properties) { - return enum_vssd(reference, results, 0); + return return_enum_vssd(reference, results, false); } static CMPIStatus GetInstance(CMPIInstanceMI *self, @@ -261,26 +295,15 @@ static CMPIStatus GetInstance(CMPIInstan const char **properties) { CMPIStatus s; - CMPIInstance *inst; - char *locid; - - if (!parse_instanceid(reference, NULL, &locid)) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Invalid InstanceID specified"); - return s; - } - - inst = get_vssd_for_name(reference, locid); - if (inst) - CMReturnInstance(results, inst); - - cu_statusf(_BROKER, &s, - CMPI_RC_OK, - ""); - - free(locid); - + CMPIInstance *inst = NULL; + + s = get_vssd_by_ref(_BROKER, reference, &inst); + if (s.rc != CMPI_RC_OK) + goto out; + + CMReturnInstance(results, inst); + + out: return s; } diff -r d72742466a88 -r 644476f8ee52 src/Virt_VSSD.h --- a/src/Virt_VSSD.h Tue Feb 19 15:29:52 2008 -0800 +++ b/src/Virt_VSSD.h Fri Feb 22 10:34:17 2008 +0100 @@ -21,8 +21,14 @@ #ifndef __VIRT_VSSD_H #define __VIRT_VSSD_H -CMPIInstance *get_vssd_instance(virDomainPtr dom, - const CMPIBroker *broker, - const CMPIObjectPath *ref); +CMPIStatus get_vssd_by_ref(const CMPIBroker *broker, + const CMPIObjectPath *reference, + CMPIInstance **_inst); + +CMPIStatus get_vssd_by_name(const CMPIBroker *broker, + const CMPIObjectPath *reference, + const char *name, + CMPIInstance **_inst); + #endif
HE> dom = virDomainLookupByName(conn, name); HE> - if (dom == NULL) HE> - goto out; HE> - HE> - inst = get_vssd_instance(dom, _BROKER, reference); HE> - HE> + if (dom == NULL) { HE> + cu_statusf(broker, &s, HE> + CMPI_RC_ERR_NOT_FOUND, HE> + "No such instance (%s)", HE> + name); HE> + goto out; I think you leak dom here, since you moved the virDomainFree before the out target: HE> + } HE> + HE> + *_inst = _get_vssd(broker, reference, conn, dom, &s); HE> + HE> + virDomainFree(dom); HE> + HE> out: HE> virConnectClose(conn); HE> - virDomainFree(dom); HE> - HE> - return inst; HE> + HE> + return s; HE> +} ... HE> +CMPIStatus get_vssd_by_ref(const CMPIBroker *broker, HE> + const CMPIObjectPath *reference, HE> + CMPIInstance **_inst) HE> +{ HE> + CMPIStatus s = {CMPI_RC_OK, NULL}; HE> + char *name = NULL; HE> + HE> + if (!parse_instanceid(reference, NULL, &name)) { HE> + cu_statusf(broker, &s, HE> + CMPI_RC_ERR_NOT_FOUND, HE> + "No such instance (InstanceID)"); HE> + goto out; HE> + } HE> + HE> + s = get_vssd_by_name(broker, reference, name, _inst); HE> + if (s.rc != CMPI_RC_OK) HE> + goto out; You leak name here because the free is before the out: HE> + HE> + s = cu_validate_ref(broker, reference, *_inst); HE> + HE> + free(name); HE> + HE> + out: HE> + return s; HE> } -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
HE> + if (dom == NULL) { HE> + cu_statusf(broker, &s, HE> + CMPI_RC_ERR_NOT_FOUND, HE> + "No such instance (%s)", HE> + name); HE> + goto out;
I think you leak dom here, since you moved the virDomainFree before the out target:
Mhh, I think not. If dom==NULL we "goto out". If dom is there, then _get_vssd() is called and virDomainFree right after. So there is no "goto out" jump in between these steps.
HE> + } HE> + HE> + *_inst = _get_vssd(broker, reference, conn, dom, &s); HE> + HE> + virDomainFree(dom); HE> + HE> out: HE> virConnectClose(conn); HE> + HE> + return s; HE> +}
...
HE> +CMPIStatus get_vssd_by_ref(const CMPIBroker *broker, HE> + const CMPIObjectPath *reference, HE> + CMPIInstance **_inst) HE> +{ HE> + CMPIStatus s = {CMPI_RC_OK, NULL}; HE> + char *name = NULL; HE> + HE> + if (!parse_instanceid(reference, NULL, &name)) { HE> + cu_statusf(broker, &s, HE> + CMPI_RC_ERR_NOT_FOUND, HE> + "No such instance (InstanceID)"); HE> + goto out; HE> + } HE> + HE> + s = get_vssd_by_name(broker, reference, name, _inst); HE> + if (s.rc != CMPI_RC_OK) HE> + goto out;
You leak name here because the free is before the out:
Good catch ... will fix this. Thanks :). -- Regards Heidi Eckhart Software Engineer IBM Linux Technology Center - Open Hypervisor
HE> Mhh, I think not. If dom==NULL we "goto out". If dom is there, HE> then _get_vssd() is called and virDomainFree right after. So there HE> is no "goto out" jump in between these steps. Right, sorry, I was reading the diff incorrectly :) -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1203672860 -3600 # Node ID 5713725abc8c466a8d621d90fe00454b7dfb3801 # Parent 644476f8ee52477f05fc3080baa517f697497eee ESD: adopt interface changes in VSSD Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r 644476f8ee52 -r 5713725abc8c src/Virt_ElementSettingData.c --- a/src/Virt_ElementSettingData.c Fri Feb 22 10:34:17 2008 +0100 +++ b/src/Virt_ElementSettingData.c Fri Feb 22 10:34:20 2008 +0100 @@ -41,47 +41,20 @@ static CMPIStatus vssd_to_vssd(const CMP { CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIInstance *inst; - virConnectPtr conn = NULL; - virDomainPtr dom = NULL; - char *host = NULL; - + if (!match_hypervisor_prefix(ref, info)) return s; - if (!parse_instanceid(ref, NULL, &host)) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unable to get system name"); - goto out; - } - - conn = connect_by_classname(_BROKER, CLASSNAME(ref), &s); - if (conn == NULL) - goto out; - - dom = virDomainLookupByName(conn, host); - if (dom == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "No such system `%s'", host); - goto out; - } - - inst = get_vssd_instance(dom, _BROKER, ref); - if (inst == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Error getting VSSD for `%s'", host); - goto out; - } - + /* Special association case: + * VSSD instance is pointing to itself + */ + s = get_vssd_by_ref(_BROKER, ref, &inst); + if (s.rc != CMPI_RC_OK) + goto out; + inst_list_add(list, inst); - + out: - virDomainFree(dom); - virConnectClose(conn); - free(host); - return s; }
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1203672860 -3600 # Node ID 5b9393cbce63ab45b6d5be07e1c6d5a866248ca5 # Parent 5713725abc8c466a8d621d90fe00454b7dfb3801 SDS: adopt interface changes in VSSD Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r 5713725abc8c -r 5b9393cbce63 src/Virt_SettingsDefineState.c --- a/src/Virt_SettingsDefineState.c Fri Feb 22 10:34:20 2008 +0100 +++ b/src/Virt_SettingsDefineState.c Fri Feb 22 10:34:20 2008 +0100 @@ -196,18 +196,16 @@ static CMPIStatus vs_to_vssd(const CMPIO struct std_assoc_info *info, struct inst_list *list) { - virConnectPtr conn = NULL; - virDomainPtr dom = NULL; + CMPIStatus s = {CMPI_RC_OK, NULL}; + CMPIInstance *inst; const char *name; - CMPIInstance *vssd; - CMPIStatus s = {CMPI_RC_OK, NULL}; - + if (!match_hypervisor_prefix(ref, info)) return s; - - conn = connect_by_classname(_BROKER, CLASSNAME(ref), &s); - if (conn == NULL) - return s; + + s = get_domain(_BROKER, ref, &inst); + if (s.rc != CMPI_RC_OK) + goto out; if (cu_get_str_path(ref, "Name", &name) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, @@ -215,28 +213,15 @@ static CMPIStatus vs_to_vssd(const CMPIO "Missing Name property"); goto out; } - - dom = virDomainLookupByName(conn, name); - if (dom == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "No such domain `%s'", name); - goto out; - } - - vssd = get_vssd_instance(dom, _BROKER, ref); - if (vssd != NULL) - inst_list_add(list, vssd); - - cu_statusf(_BROKER, &s, - CMPI_RC_OK, - ""); - out: - virDomainFree(dom); - virConnectClose(conn); - + + s = get_vssd_by_name(_BROKER, ref, name, &inst); + if (s.rc != CMPI_RC_OK) + goto out; + + inst_list_add(list, inst); + + out: return s; - } static CMPIStatus vssd_to_vs(const CMPIObjectPath *ref, @@ -250,9 +235,14 @@ static CMPIStatus vssd_to_vs(const CMPIO virConnectPtr conn = NULL; CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIInstance *cs; + CMPIInstance *inst; if (!match_hypervisor_prefix(ref, info)) return s; + + s = get_vssd_by_ref(_BROKER, ref, &inst); + if (s.rc != CMPI_RC_OK) + goto out; if (cu_get_str_path(ref, "InstanceID", &id) != CMPI_RC_OK) { cu_statusf(_BROKER, &s,
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1203672861 -3600 # Node ID b9cbac659a845feda6f170fe23663ea7f187bf21 # Parent 5b9393cbce63ab45b6d5be07e1c6d5a866248ca5 VSSDC: adopt interface changes in VSSD Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r 5b9393cbce63 -r b9cbac659a84 src/Virt_VSSDComponent.c --- a/src/Virt_VSSDComponent.c Fri Feb 22 10:34:20 2008 +0100 +++ b/src/Virt_VSSDComponent.c Fri Feb 22 10:34:21 2008 +0100 @@ -41,6 +41,7 @@ static CMPIStatus vssd_to_rasd(const CMP struct inst_list *list) { CMPIStatus s = {CMPI_RC_OK, NULL}; + CMPIInstance *inst = NULL; char *name = NULL; int i = 0; int types[] = { @@ -52,7 +53,11 @@ static CMPIStatus vssd_to_rasd(const CMP }; if (!match_hypervisor_prefix(ref, info)) - return s; + goto out; + + s = get_vssd_by_ref(_BROKER, ref, &inst); + if (s.rc != CMPI_RC_OK) + goto out; if (!parse_instanceid(ref, NULL, &name)) { cu_statusf(_BROKER, &s, @@ -69,48 +74,9 @@ static CMPIStatus vssd_to_rasd(const CMP list); } - cu_statusf(_BROKER, &s, - CMPI_RC_OK, - ""); - out: free(name); - return s; -} - -static CMPIStatus vssd_for_name(const char *host, - const CMPIObjectPath *ref, - CMPIInstance **inst) -{ - virConnectPtr conn = NULL; - virDomainPtr dom = NULL; - CMPIStatus s; - - conn = connect_by_classname(_BROKER, CLASSNAME(ref), &s); - if (conn == NULL) - goto out; - - dom = virDomainLookupByName(conn, host); - if (dom == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "No such system `%s'", host); - goto out; - } - - *inst = get_vssd_instance(dom, _BROKER, ref); - if (*inst == NULL) - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Error getting VSSD for `%s'", host); - else - cu_statusf(_BROKER, &s, - CMPI_RC_OK, - ""); out: - virDomainFree(dom); - virConnectClose(conn); - return s; } @@ -143,9 +109,11 @@ static CMPIStatus rasd_to_vssd(const CMP goto out; } - s = vssd_for_name(host, ref, &vssd); - if (vssd) - inst_list_add(list, vssd); + s = get_vssd_by_name(_BROKER, ref, host, &vssd); + if (s.rc != CMPI_RC_OK) + goto out; + + inst_list_add(list, vssd); out: free(host);
Heidi Eckhart wrote:
VSSD does now take care of the client given object path. The resulting interface changes have been adopted to - ESD by the second patch - SDS by the third patch - VSSDC by the fourth patch (diff to patch set 1)
diff to patch set 2: - updated VSSD to compare a returned integer result explicitely
This set tested out fine on my system. +1 -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
participants (3)
-
Dan Smith -
Heidi Eckhart -
Kaitlin Rupert