[PATCH] Fix VSMS instance code to refuse to return an instance if ref is invalid

# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1193689589 25200 # Node ID a9008606ede19590b358580f3bc2732f9c92c23c # Parent bf5ad6924b99903d68d019a7c5faaa2f5a5d1ef9 Fix VSMS instance code to refuse to return an instance if ref is invalid Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r bf5ad6924b99 -r a9008606ede1 src/Makefile.am --- a/src/Makefile.am Mon Oct 29 15:39:52 2007 -0400 +++ b/src/Makefile.am Mon Oct 29 13:26:29 2007 -0700 @@ -56,7 +56,7 @@ libVirt_ComputerSystemIndication_la_LIBA libVirt_ComputerSystemIndication_la_LIBADD = -lVirt_ComputerSystem -lpthread -lrt libVirt_VirtualSystemManagementService_la_SOURCES = Virt_VirtualSystemManagementService.c -libVirt_VirtualSystemManagementService_la_LIBADD = -lVirt_ComputerSystem -lVirt_ComputerSystemIndication -lVirt_RASD +libVirt_VirtualSystemManagementService_la_LIBADD = -lVirt_ComputerSystem -lVirt_ComputerSystemIndication -lVirt_RASD -lVirt_HostSystem libVirt_VirtualSystemManagementCapabilities_la_SOURCES = Virt_VirtualSystemManagementCapabilities.c diff -r bf5ad6924b99 -r a9008606ede1 src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c Mon Oct 29 15:39:52 2007 -0400 +++ b/src/Virt_VirtualSystemManagementService.c Mon Oct 29 13:26:29 2007 -0700 @@ -44,6 +44,7 @@ #include "Virt_ComputerSystem.h" #include "Virt_ComputerSystemIndication.h" #include "Virt_RASD.h" +#include "Virt_HostSystem.h" #include "svpc_types.h" const static CMPIBroker *_BROKER; @@ -896,18 +897,24 @@ STDIM_MethodMIStub(, Virt_VirtualSystemM STDIM_MethodMIStub(, Virt_VirtualSystemManagementService, _BROKER, CMNoHook, my_handlers); - -static CMPIStatus return_vsms(const CMPIObjectPath *reference, - const CMPIResult *results, - int name_only) +static CMPIStatus _get_vsms(const CMPIObjectPath *reference, + CMPIInstance **_inst, + int name_only) { CMPIStatus s; CMPIInstance *inst; + CMPIInstance *host; + char *val = NULL; + + s = get_host_cs(_BROKER, reference, &host); + if (s.rc != CMPI_RC_OK) + goto out; inst = get_typed_instance(_BROKER, "VirtualSystemManagementService", NAMESPACE(reference)); if (inst == NULL) { + CU_DEBUG("Failed to get typed instance"); cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Failed to create instance"); @@ -916,6 +923,46 @@ static CMPIStatus return_vsms(const CMPI CMSetProperty(inst, "Name", (CMPIValue *)"Management Service", CMPI_chars); + + if (cu_get_str_prop(host, "Name", &val) != CMPI_RC_OK) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to get name of System"); + goto out; + } + + CMSetProperty(inst, "SystemName", + (CMPIValue *)val, CMPI_chars); + free(val); + + if (cu_get_str_prop(host, "CreationClassName", &val) != CMPI_RC_OK) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to get creation class of system"); + goto out; + } + + CMSetProperty(inst, "SystemCreationClassName", + (CMPIValue *)val, CMPI_chars); + free(val); + + CMSetStatus(&s, CMPI_RC_OK); + + *_inst = inst; + out: + return s; +} + +static CMPIStatus return_vsms(const CMPIObjectPath *reference, + const CMPIResult *results, + int name_only) +{ + CMPIInstance *inst; + CMPIStatus s; + + s = _get_vsms(reference, &inst, name_only); + if (s.rc != CMPI_RC_OK) + goto out; if (name_only) cu_return_instance_name(results, inst); @@ -923,7 +970,6 @@ static CMPIStatus return_vsms(const CMPI CMReturnInstance(results, inst); CMSetStatus(&s, CMPI_RC_OK); - out: return s; } @@ -946,13 +992,58 @@ static CMPIStatus EnumInstances(CMPIInst return return_vsms(reference, results, 0); } +static int compare_prop(const CMPIObjectPath *ref, + const CMPIInstance *inst, + const char *name, + int mandatory) +{ + char *prop = NULL; + char *key = NULL; + int rc = 0; + + key = cu_get_str_path(ref, name); + if (key == NULL) { + rc = !mandatory; + goto out; + } + + if (cu_get_str_prop(inst, name, &prop) != CMPI_RC_OK) + goto out; + + rc = STREQ(key, prop); + out: + free(prop); + free(key); + + return rc; +} + static CMPIStatus GetInstance(CMPIInstanceMI *self, const CMPIContext *context, const CMPIResult *results, - const CMPIObjectPath *reference, + const CMPIObjectPath *ref, const char **properties) { - return return_vsms(reference, results, 0); + CMPIInstance *inst; + CMPIStatus s; + + s = _get_vsms(ref, &inst, 0); + if (s.rc != CMPI_RC_OK) + return s; + + if (!compare_prop(ref, inst, "CreationClassName", 0) || + !compare_prop(ref, inst, "SystemName", 0) || + !compare_prop(ref, inst, "Name", 1) || + !compare_prop(ref, inst, "SystemCreationClassName", 0)) + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "No such instance"); + else { + CMSetStatus(&s, CMPI_RC_OK); + CMReturnInstance(results, inst); + } + + return s; } DEFAULT_CI();

DS> +static int compare_prop(const CMPIObjectPath *ref, DS> + const CMPIInstance *inst, DS> + const char *name, DS> + int mandatory) DS> +{ DS> + char *prop = NULL; DS> + char *key = NULL; DS> + int rc = 0; DS> + DS> + key = cu_get_str_path(ref, name); DS> + if (key == NULL) { DS> + rc = !mandatory; DS> + goto out; DS> + } DS> + DS> + if (cu_get_str_prop(inst, name, &prop) != CMPI_RC_OK) DS> + goto out; DS> + DS> + rc = STREQ(key, prop); DS> + out: DS> + free(prop); DS> + free(key); DS> + DS> + return rc; DS> +} I think that some variation of the above function probably deserves to be in libcmpiutil. The act of comparing an instance with a reference and make sure that all keys are either correct or unspecified seems like a common thing. There are probably other places where we need such a filter anyway. Thoughts? -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com

Dan Smith wrote:
DS> +static int compare_prop(const CMPIObjectPath *ref, DS> + const CMPIInstance *inst, DS> + const char *name, DS> + int mandatory) DS> +{ DS> + char *prop = NULL; DS> + char *key = NULL; DS> + int rc = 0; DS> + DS> + key = cu_get_str_path(ref, name); DS> + if (key == NULL) { DS> + rc = !mandatory; DS> + goto out; DS> + } DS> + DS> + if (cu_get_str_prop(inst, name, &prop) != CMPI_RC_OK) DS> + goto out; DS> + DS> + rc = STREQ(key, prop); DS> + out: DS> + free(prop); DS> + free(key); DS> + DS> + return rc; DS> +}
I think that some variation of the above function probably deserves to be in libcmpiutil. The act of comparing an instance with a reference and make sure that all keys are either correct or unspecified seems like a common thing. There are probably other places where we need such a filter anyway. Thoughts?
I absolutely agree. Such a function should become part of libcmpiutil.
------------------------------------------------------------------------
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- 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

Dan Smith wrote:
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1193689589 25200 # Node ID a9008606ede19590b358580f3bc2732f9c92c23c # Parent bf5ad6924b99903d68d019a7c5faaa2f5a5d1ef9 Fix VSMS instance code to refuse to return an instance if ref is invalid
Signed-off-by: Dan Smith <danms@us.ibm.com>
diff -r bf5ad6924b99 -r a9008606ede1 src/Makefile.am --- a/src/Makefile.am Mon Oct 29 15:39:52 2007 -0400 +++ b/src/Makefile.am Mon Oct 29 13:26:29 2007 -0700 @@ -56,7 +56,7 @@ libVirt_ComputerSystemIndication_la_LIBA libVirt_ComputerSystemIndication_la_LIBADD = -lVirt_ComputerSystem -lpthread -lrt
libVirt_VirtualSystemManagementService_la_SOURCES = Virt_VirtualSystemManagementService.c -libVirt_VirtualSystemManagementService_la_LIBADD = -lVirt_ComputerSystem -lVirt_ComputerSystemIndication -lVirt_RASD +libVirt_VirtualSystemManagementService_la_LIBADD = -lVirt_ComputerSystem -lVirt_ComputerSystemIndication -lVirt_RASD -lVirt_HostSystem
libVirt_VirtualSystemManagementCapabilities_la_SOURCES = Virt_VirtualSystemManagementCapabilities.c
diff -r bf5ad6924b99 -r a9008606ede1 src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c Mon Oct 29 15:39:52 2007 -0400 +++ b/src/Virt_VirtualSystemManagementService.c Mon Oct 29 13:26:29 2007 -0700 @@ -44,6 +44,7 @@ #include "Virt_ComputerSystem.h" #include "Virt_ComputerSystemIndication.h" #include "Virt_RASD.h" +#include "Virt_HostSystem.h" #include "svpc_types.h"
const static CMPIBroker *_BROKER; @@ -896,18 +897,24 @@ STDIM_MethodMIStub(, Virt_VirtualSystemM STDIM_MethodMIStub(, Virt_VirtualSystemManagementService, _BROKER, CMNoHook, my_handlers);
- -static CMPIStatus return_vsms(const CMPIObjectPath *reference, - const CMPIResult *results, - int name_only) +static CMPIStatus _get_vsms(const CMPIObjectPath *reference, + CMPIInstance **_inst, + int name_only) { CMPIStatus s; CMPIInstance *inst; + CMPIInstance *host; + char *val = NULL; + + s = get_host_cs(_BROKER, reference, &host); + if (s.rc != CMPI_RC_OK) + goto out;
inst = get_typed_instance(_BROKER, "VirtualSystemManagementService", NAMESPACE(reference)); if (inst == NULL) { + CU_DEBUG("Failed to get typed instance"); cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Failed to create instance"); @@ -916,6 +923,46 @@ static CMPIStatus return_vsms(const CMPI
CMSetProperty(inst, "Name", (CMPIValue *)"Management Service", CMPI_chars); + + if (cu_get_str_prop(host, "Name", &val) != CMPI_RC_OK) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to get name of System"); + goto out; + } + + CMSetProperty(inst, "SystemName", + (CMPIValue *)val, CMPI_chars); + free(val); + + if (cu_get_str_prop(host, "CreationClassName", &val) != CMPI_RC_OK) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to get creation class of system"); + goto out; + } + + CMSetProperty(inst, "SystemCreationClassName", + (CMPIValue *)val, CMPI_chars); + free(val); + + CMSetStatus(&s, CMPI_RC_OK); + + *_inst = inst; + out: + return s; +} + +static CMPIStatus return_vsms(const CMPIObjectPath *reference, + const CMPIResult *results, + int name_only) +{ + CMPIInstance *inst; + CMPIStatus s; + + s = _get_vsms(reference, &inst, name_only); + if (s.rc != CMPI_RC_OK) + goto out;
if (name_only) cu_return_instance_name(results, inst); @@ -923,7 +970,6 @@ static CMPIStatus return_vsms(const CMPI CMReturnInstance(results, inst);
CMSetStatus(&s, CMPI_RC_OK); - out: return s; } @@ -946,13 +992,58 @@ static CMPIStatus EnumInstances(CMPIInst return return_vsms(reference, results, 0); }
+static int compare_prop(const CMPIObjectPath *ref, + const CMPIInstance *inst, + const char *name, + int mandatory) +{ + char *prop = NULL; + char *key = NULL; + int rc = 0; + + key = cu_get_str_path(ref, name); + if (key == NULL) { + rc = !mandatory; + goto out; + } + + if (cu_get_str_prop(inst, name, &prop) != CMPI_RC_OK) + goto out; + + rc = STREQ(key, prop); + out: + free(prop); + free(key); + + return rc; +} + static CMPIStatus GetInstance(CMPIInstanceMI *self, const CMPIContext *context, const CMPIResult *results, - const CMPIObjectPath *reference, + const CMPIObjectPath *ref, const char **properties) { - return return_vsms(reference, results, 0); + CMPIInstance *inst; + CMPIStatus s; + + s = _get_vsms(ref, &inst, 0); + if (s.rc != CMPI_RC_OK) + return s; + + if (!compare_prop(ref, inst, "CreationClassName", 0) || + !compare_prop(ref, inst, "SystemName", 0) || + !compare_prop(ref, inst, "Name", 1) || + !compare_prop(ref, inst, "SystemCreationClassName", 0)) + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED,
In regard to DMTF's DSP200 the return code should be CMPI_RC_ERR_NOT_FOUND, as "the CIM Class does exist, but the requested CIM Instance does not exist in the specified namespace".
+ "No such instance"); + else { + CMSetStatus(&s, CMPI_RC_OK); + CMReturnInstance(results, inst); + } + + return s; }
DEFAULT_CI();
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
nothing else to comment ... +1 :) -- 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> In regard to DMTF's DSP200 the return code should be HE> CMPI_RC_ERR_NOT_FOUND, as "the CIM Class does exist, but the requested HE> CIM Instance does not exist in the specified namespace". Yep, as far as I can tell, almost everything in the tree right now returns CMPI_RC_ERR_FAILED as a generic failure code. We'll have to spend some time going back to tweak all of those at some point. However, since you've identified this one, I'll tweak and re-send my patch. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com

Dan Smith wrote:
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1193689589 25200 # Node ID a9008606ede19590b358580f3bc2732f9c92c23c # Parent bf5ad6924b99903d68d019a7c5faaa2f5a5d1ef9 Fix VSMS instance code to refuse to return an instance if ref is invalid
Signed-off-by: Dan Smith <danms@us.ibm.com>
diff -r bf5ad6924b99 -r a9008606ede1 src/Makefile.am --- a/src/Makefile.am Mon Oct 29 15:39:52 2007 -0400 +++ b/src/Makefile.am Mon Oct 29 13:26:29 2007 -0700 @@ -56,7 +56,7 @@ libVirt_ComputerSystemIndication_la_LIBA libVirt_ComputerSystemIndication_la_LIBADD = -lVirt_ComputerSystem -lpthread -lrt
libVirt_VirtualSystemManagementService_la_SOURCES = Virt_VirtualSystemManagementService.c -libVirt_VirtualSystemManagementService_la_LIBADD = -lVirt_ComputerSystem -lVirt_ComputerSystemIndication -lVirt_RASD +libVirt_VirtualSystemManagementService_la_LIBADD = -lVirt_ComputerSystem -lVirt_ComputerSystemIndication -lVirt_RASD -lVirt_HostSystem
libVirt_VirtualSystemManagementCapabilities_la_SOURCES = Virt_VirtualSystemManagementCapabilities.c
diff -r bf5ad6924b99 -r a9008606ede1 src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c Mon Oct 29 15:39:52 2007 -0400 +++ b/src/Virt_VirtualSystemManagementService.c Mon Oct 29 13:26:29 2007 -0700 @@ -44,6 +44,7 @@ #include "Virt_ComputerSystem.h" #include "Virt_ComputerSystemIndication.h" #include "Virt_RASD.h" +#include "Virt_HostSystem.h" #include "svpc_types.h"
const static CMPIBroker *_BROKER; @@ -896,18 +897,24 @@ STDIM_MethodMIStub(, Virt_VirtualSystemM STDIM_MethodMIStub(, Virt_VirtualSystemManagementService, _BROKER, CMNoHook, my_handlers);
- -static CMPIStatus return_vsms(const CMPIObjectPath *reference, - const CMPIResult *results, - int name_only) +static CMPIStatus _get_vsms(const CMPIObjectPath *reference, + CMPIInstance **_inst, + int name_only) { CMPIStatus s; CMPIInstance *inst; + CMPIInstance *host; + char *val = NULL; + + s = get_host_cs(_BROKER, reference, &host); + if (s.rc != CMPI_RC_OK) + goto out;
inst = get_typed_instance(_BROKER, "VirtualSystemManagementService", NAMESPACE(reference)); if (inst == NULL) { + CU_DEBUG("Failed to get typed instance"); cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Failed to create instance"); @@ -916,6 +923,46 @@ static CMPIStatus return_vsms(const CMPI
CMSetProperty(inst, "Name", (CMPIValue *)"Management Service", CMPI_chars); + + if (cu_get_str_prop(host, "Name", &val) != CMPI_RC_OK) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to get name of System"); + goto out; + } + + CMSetProperty(inst, "SystemName", + (CMPIValue *)val, CMPI_chars); + free(val); + + if (cu_get_str_prop(host, "CreationClassName", &val) != CMPI_RC_OK) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to get creation class of system"); + goto out; + } + + CMSetProperty(inst, "SystemCreationClassName", + (CMPIValue *)val, CMPI_chars); + free(val); + + CMSetStatus(&s, CMPI_RC_OK); + + *_inst = inst; + out: + return s; +} + +static CMPIStatus return_vsms(const CMPIObjectPath *reference, + const CMPIResult *results, + int name_only) +{ + CMPIInstance *inst; + CMPIStatus s; + + s = _get_vsms(reference, &inst, name_only); + if (s.rc != CMPI_RC_OK) + goto out;
if (name_only) cu_return_instance_name(results, inst); @@ -923,7 +970,6 @@ static CMPIStatus return_vsms(const CMPI CMReturnInstance(results, inst);
CMSetStatus(&s, CMPI_RC_OK); - out: return s; } @@ -946,13 +992,58 @@ static CMPIStatus EnumInstances(CMPIInst return return_vsms(reference, results, 0); }
+static int compare_prop(const CMPIObjectPath *ref, + const CMPIInstance *inst, + const char *name, + int mandatory) +{ + char *prop = NULL; + char *key = NULL; + int rc = 0; + + key = cu_get_str_path(ref, name); + if (key == NULL) { + rc = !mandatory; + goto out; + } + + if (cu_get_str_prop(inst, name, &prop) != CMPI_RC_OK) + goto out; + + rc = STREQ(key, prop); + out: + free(prop); + free(key); + + return rc; +} + static CMPIStatus GetInstance(CMPIInstanceMI *self, const CMPIContext *context, const CMPIResult *results, - const CMPIObjectPath *reference, + const CMPIObjectPath *ref, const char **properties) { - return return_vsms(reference, results, 0); + CMPIInstance *inst; + CMPIStatus s; + + s = _get_vsms(ref, &inst, 0); + if (s.rc != CMPI_RC_OK) + return s; + + if (!compare_prop(ref, inst, "CreationClassName", 0) || + !compare_prop(ref, inst, "SystemName", 0) || + !compare_prop(ref, inst, "Name", 1) || + !compare_prop(ref, inst, "SystemCreationClassName", 0)) + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED,
In regard to DMTF's DSP200 the return code should be CMPI_RC_ERR_NOT_FOUND, as "the CIM Class does exist, but the requested CIM Instance does not exist in the specified namespace".
+ "No such instance"); + else { + CMSetStatus(&s, CMPI_RC_OK); + CMReturnInstance(results, inst); + } + + return s; }
DEFAULT_CI();
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
nothing else to comment ... +1 :) -- 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
participants (2)
-
Dan Smith
-
Heidi Eckhart