[PATCH 0 of 2] RPCC: ignores supported hypervisor type and wrong object path
This patch set fixes RPCC's handling of the supported hypervisor and enables checking the client given object path on gi calls.
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1201865505 -3600 # Node ID d4b86a39d55d2208c5f56a6666ccc212ab60a240 # Parent 6b1bf8231d82f51f25ac3d954b63962bfc7484c9 RPCC: returns instances of Xen on a KVM only system This breaks the approach that is used all over libvirt-cim providers, where only instances of the currently supported hypervisor type are returned. Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r 6b1bf8231d82 -r d4b86a39d55d src/Virt_ResourcePoolConfigurationCapabilities.c --- a/src/Virt_ResourcePoolConfigurationCapabilities.c Fri Feb 01 12:20:46 2008 +0100 +++ b/src/Virt_ResourcePoolConfigurationCapabilities.c Fri Feb 01 12:31:45 2008 +0100 @@ -51,28 +51,39 @@ static CMPIStatus get_rpc_cap(const CMPI const CMPIObjectPath *reference, CMPIInstance **_inst) { - CMPIInstance *inst; + CMPIStatus s = {CMPI_RC_OK, NULL}; + CMPIInstance *inst = NULL; + virConnectPtr conn = NULL; + + conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s); + if (conn == NULL) + goto out; inst = get_typed_instance(broker, - CLASSNAME(reference), + pfx_from_conn(conn), "ResourcePoolConfigurationCapabilities", NAMESPACE(reference)); if (inst == NULL) - return (CMPIStatus){CMPI_RC_ERR_FAILED, NULL}; + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Can't create ResourcePoolConfigurationCapabilities instance"); CMSetProperty(inst, "InstanceID", (CMPIValue *)"RPCC", CMPI_chars); /* No method currently supported */ + out: *_inst = inst; - - return (CMPIStatus){CMPI_RC_OK, NULL}; + virConnectClose(conn); + + return s; } static CMPIStatus return_rpc_cap(const CMPIObjectPath *reference, const CMPIResult *results, - bool names_only) + bool names_only, + bool getInstance) { CMPIStatus s; CMPIInstance *inst; @@ -81,11 +92,19 @@ static CMPIStatus return_rpc_cap(const C if (s.rc != CMPI_RC_OK) goto out; + if (inst == NULL) { + if (getInstance) + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance"); + goto out; + } + if (names_only) cu_return_instance_name(results, inst); else CMReturnInstance(results, inst); - + out: return s; } @@ -95,7 +114,7 @@ static CMPIStatus EnumInstanceNames(CMPI const CMPIResult *results, const CMPIObjectPath *reference) { - return return_rpc_cap(reference, results, 1); + return return_rpc_cap(reference, results, true, false); } static CMPIStatus EnumInstances(CMPIInstanceMI *self, @@ -105,7 +124,7 @@ static CMPIStatus EnumInstances(CMPIInst const char **properties) { - return return_rpc_cap(reference, results, 0); + return return_rpc_cap(reference, results, false, false); } static CMPIStatus GetInstance(CMPIInstanceMI *self, @@ -114,7 +133,7 @@ static CMPIStatus GetInstance(CMPIInstan const CMPIObjectPath *reference, const char **properties) { - return return_rpc_cap(reference, results, 0); + return return_rpc_cap(reference, results, false, true); }
Heidi Eckhart wrote:
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1201865505 -3600 # Node ID d4b86a39d55d2208c5f56a6666ccc212ab60a240 # Parent 6b1bf8231d82f51f25ac3d954b63962bfc7484c9 RPCC: returns instances of Xen on a KVM only system
This breaks the approach that is used all over libvirt-cim providers, where only instances of the currently supported hypervisor type are returned.
Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com>
diff -r 6b1bf8231d82 -r d4b86a39d55d src/Virt_ResourcePoolConfigurationCapabilities.c --- a/src/Virt_ResourcePoolConfigurationCapabilities.c Fri Feb 01 12:20:46 2008 +0100 +++ b/src/Virt_ResourcePoolConfigurationCapabilities.c Fri Feb 01 12:31:45 2008 +0100 @@ -51,28 +51,39 @@ static CMPIStatus get_rpc_cap(const CMPI const CMPIObjectPath *reference, CMPIInstance **_inst) { - CMPIInstance *inst; + CMPIStatus s = {CMPI_RC_OK, NULL}; + CMPIInstance *inst = NULL; + virConnectPtr conn = NULL; + + conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s); + if (conn == NULL) + goto out;
Not sure if everyone will agree, but would it be a good idea to set an error status here? Currently, only Virt_ResourcePoolConfigurationCapabilities is calling this function, but I think that's a potential bug. ElementCapabilities doesn't support this Capabilities class, but I think it should based on the ResourceAllocation profile. Of course, that's a subject of a different patch. However, if we set the status here, then ElementCapabilities won't have to worry about checking whether the instance is NULL. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
Kaitlin Rupert wrote:
Heidi Eckhart wrote:
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1201865505 -3600 # Node ID d4b86a39d55d2208c5f56a6666ccc212ab60a240 # Parent 6b1bf8231d82f51f25ac3d954b63962bfc7484c9 RPCC: returns instances of Xen on a KVM only system
This breaks the approach that is used all over libvirt-cim providers, where only instances of the currently supported hypervisor type are returned.
Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com>
diff -r 6b1bf8231d82 -r d4b86a39d55d src/Virt_ResourcePoolConfigurationCapabilities.c --- a/src/Virt_ResourcePoolConfigurationCapabilities.c Fri Feb 01 12:20:46 2008 +0100 +++ b/src/Virt_ResourcePoolConfigurationCapabilities.c Fri Feb 01 12:31:45 2008 +0100 @@ -51,28 +51,39 @@ static CMPIStatus get_rpc_cap(const CMPI const CMPIObjectPath *reference, CMPIInstance **_inst) { - CMPIInstance *inst; + CMPIStatus s = {CMPI_RC_OK, NULL}; + CMPIInstance *inst = NULL; + virConnectPtr conn = NULL; + + conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s); + if (conn == NULL) + goto out;
Not sure if everyone will agree, but would it be a good idea to set an error status here?
Currently, only Virt_ResourcePoolConfigurationCapabilities is calling this function, but I think that's a potential bug. ElementCapabilities doesn't support this Capabilities class, but I think it should based on the ResourceAllocation profile.
Of course, that's a subject of a different patch. However, if we set the status here, then ElementCapabilities won't have to worry about checking whether the instance is NULL. I think this might be one of those places where setting a negative status could cause an error code to filter up to the CIMOM when we don't want it to. Something like the distinction between "CMPI_RC_ERR" and "CMPI_RC_NOT_FOUND" or whatever that was.
-- -Jay
KR> Not sure if everyone will agree, but would it be a good idea to KR> set an error status here? Yeah, I think you're right. If the calling function of this currently needs to return a CMPI_RC_OK instead of an error, then it probably needs to detect it and return appropriately. If this function makes more sense (in the general case) to return an error, then we should do that. -- 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 1201868641 -3600 # Node ID 21fecbdb0f84910eb9a889319db6486882bd7194 # Parent d4b86a39d55d2208c5f56a6666ccc212ab60a240 RPCC: getInstance does not take care of client given object path wbemgi 'http://localhost:5988/root/virt:KVM_ResourcePoolConfigurationCapabilities.InstanceID="wrong"' returns the instance instead of NOT_FOUND. Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r d4b86a39d55d -r 21fecbdb0f84 src/Virt_ResourcePoolConfigurationCapabilities.c --- a/src/Virt_ResourcePoolConfigurationCapabilities.c Fri Feb 01 12:31:45 2008 +0100 +++ b/src/Virt_ResourcePoolConfigurationCapabilities.c Fri Feb 01 13:24:01 2008 +0100 @@ -99,6 +99,12 @@ static CMPIStatus return_rpc_cap(const C "No such instance"); goto out; } + + if (getInstance) { + s = cu_validate_ref(_BROKER, reference, inst); + if (s.rc != CMPI_RC_OK) + goto out; + } if (names_only) cu_return_instance_name(results, inst);
participants (4)
-
Dan Smith -
Heidi Eckhart -
Jay Gagnon -
Kaitlin Rupert