[PATCH 0 of 4] #2 - RPCS and RPCC ignores supported hypervisor type and wrong object path, adopt changes to HS
This patch set is now the combination of the one patch against RPCS and the two patches against RPCC, I sent out on friday. The reason to combine these three patches in one patch set is, that both provider - RPCS and RPCC - look very similar now. This brought me to the idea, that it is possible to also define a common interface for instances providers - as this is already done for associations -, where the common functions could do a main part of the error handling (which is currently done again and again in each single provider itself). In addition patch 4 updates HS (thanks to Kaitlin) with the new interface of RPCS.
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1202131336 -3600 # Node ID f3ea51dea6f805fa354d173f38a9c5037e4785a3 # Parent 6c3c30bf8e026c9b3609195c55766c6c3f757b99 RPCS: enumerateInstance(Name)s and getInstance with wrong hypervisor segfaults wbemein http://localhost/root/virt:CIM_ResourcePoolConfigurationService on a KVM system with no Xen segfaults. The same happens to wbemgi 'http://localhost:5988/root/virt:Xen_ResourcePoolConfigurationService.SystemCreationClassName="Xen_HostSystem",SystemName="localhost.localdomain",CreationClassName="Xen_ResourcePoolConfigurationService",Name="RPCS"' This patch also makes RPCS aware of the supported hypervisor type. Now only instances of the machine's supported hypervisor are returned. This approach is used all over the libvirt-cim providers, but was broken here. Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r 6c3c30bf8e02 -r f3ea51dea6f8 src/Virt_ResourcePoolConfigurationService.c --- a/src/Virt_ResourcePoolConfigurationService.c Fri Feb 01 11:17:15 2008 +0100 +++ b/src/Virt_ResourcePoolConfigurationService.c Mon Feb 04 14:22:16 2008 +0100 @@ -94,21 +94,28 @@ DEFAULT_EQ(); DEFAULT_EQ(); DEFAULT_INST_CLEANUP(); -CMPIStatus rpcs_instance(const CMPIObjectPath *reference, - CMPIInstance **_inst, - const CMPIBroker *broker) +CMPIStatus get_rpcs(const CMPIObjectPath *reference, + CMPIInstance **_inst, + const CMPIBroker *broker, + bool getInstance) { CMPIInstance *inst; - CMPIInstance *host; - CMPIStatus s; - CMPIData prop; - - s = get_host_cs(broker, reference, &host); - if (s.rc != CMPI_RC_OK) - goto out; + CMPIStatus s = {CMPI_RC_OK, NULL}; + virConnectPtr conn = NULL; + const char *name = NULL; + const char *ccname = NULL; + + conn = connect_by_classname(broker, CLASSNAME(reference), &s); + if (conn == NULL) { + if (getInstance) + cu_statusf(broker, &s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance"); + goto out; + } inst = get_typed_instance(broker, - CLASSNAME(reference), + pfx_from_conn(conn), "ResourcePoolConfigurationService", NAMESPACE(reference)); if (inst == NULL) { @@ -119,32 +126,57 @@ CMPIStatus rpcs_instance(const CMPIObjec goto out; } - CMSetProperty(inst, "Name", - (CMPIValue *)"RPCS", CMPI_chars); - - prop = CMGetProperty(host, "CreationClassName", &s); + s = get_host_system_properties(&name, + &ccname, + reference, + broker); if (s.rc != CMPI_RC_OK) { cu_statusf(broker, &s, CMPI_RC_ERR_FAILED, - "Unable to get CreationClassName from HostSystem"); - goto out; - } + "Unable to get host attributes"); + goto out; + } + + CMSetProperty(inst, "Name", + (CMPIValue *)"RPCS", CMPI_chars); + + CMSetProperty(inst, "SystemName", + (CMPIValue *)name, CMPI_chars); CMSetProperty(inst, "SystemCreationClassName", - (CMPIValue *)&prop.value.string, CMPI_string); - - prop = CMGetProperty(host, "Name", NULL); - if (s.rc != CMPI_RC_OK) { - cu_statusf(broker, &s, - CMPI_RC_ERR_FAILED, - "Unable to get Name from HostSystem"); - goto out; - } - - CMSetProperty(inst, "SystemName", - (CMPIValue *)&prop.value.string, CMPI_string); + (CMPIValue *)ccname, CMPI_chars); + + if (getInstance) { + s = cu_validate_ref(broker, reference, inst); + if (s.rc != CMPI_RC_OK) + goto out; + } *_inst = inst; + + out: + virConnectClose(conn); + + return s; +} + +static CMPIStatus return_rpcs(const CMPIResult *results, + const CMPIObjectPath *reference, + bool names_only, + bool getInstance) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + CMPIInstance *inst = NULL; + + s = get_rpcs(reference, &inst, _BROKER, getInstance); + if (s.rc != CMPI_RC_OK || inst == NULL) + goto out; + + if (names_only) + cu_return_instance_name(results, inst); + else + CMReturnInstance(results, inst); + out: return s; } @@ -155,24 +187,7 @@ static CMPIStatus GetInstance(CMPIInstan const CMPIObjectPath *reference, const char **properties) { - CMPIInstance *inst; - CMPIStatus s; - const char *prop = NULL; - - s = rpcs_instance(reference, &inst, _BROKER); - if (s.rc != CMPI_RC_OK) - return s; - - prop = cu_compare_ref(reference, inst); - if (prop != NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_NOT_FOUND, - "No such instance (%s)", prop); - } else { - CMReturnInstance(results, inst); - } - - return s; + return return_rpcs(results, reference, false, true); } static CMPIStatus EnumInstanceNames(CMPIInstanceMI *self, @@ -180,14 +195,7 @@ static CMPIStatus EnumInstanceNames(CMPI const CMPIResult *results, const CMPIObjectPath *reference) { - CMPIInstance *inst; - CMPIStatus s; - - s = rpcs_instance(reference, &inst, _BROKER); - if (s.rc == CMPI_RC_OK) - cu_return_instance_name(results, inst); - - return s; + return return_rpcs(results, reference, true, false); } static CMPIStatus EnumInstances(CMPIInstanceMI *self, @@ -196,15 +204,7 @@ static CMPIStatus EnumInstances(CMPIInst const CMPIObjectPath *reference, const char **properties) { - - CMPIInstance *inst; - CMPIStatus s; - - s = rpcs_instance(reference, &inst, _BROKER); - if (s.rc == CMPI_RC_OK) - CMReturnInstance(results, inst); - - return s; + return return_rpcs(results, reference, false, false); } diff -r 6c3c30bf8e02 -r f3ea51dea6f8 src/Virt_ResourcePoolConfigurationService.h --- a/src/Virt_ResourcePoolConfigurationService.h Fri Feb 01 11:17:15 2008 +0100 +++ b/src/Virt_ResourcePoolConfigurationService.h Mon Feb 04 14:22:16 2008 +0100 @@ -19,6 +19,17 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -CMPIStatus rpcs_instance(const CMPIObjectPath *reference, - CMPIInstance **_inst, - const CMPIBroker *broker); +CMPIStatus get_rpcs(const CMPIObjectPath *reference, + CMPIInstance **_inst, + const CMPIBroker *broker, + bool getInstance); + +/* + * Local Variables: + * mode: C + * c-set-style: "K&R" + * tab-width: 8 + * c-basic-offset: 8 + * indent-tabs-mode: nil + * End: + */
HE> +CMPIStatus get_rpcs(const CMPIObjectPath *reference, HE> + CMPIInstance **_inst, HE> + const CMPIBroker *broker, HE> + bool getInstance) I think the functionality you're building in here is a good thing. I have a minor issue with the new parameter. The camelCase differs From the rest of our style. Perhaps something like "is_gi" would be more appropriate and also convey the message? -- 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 1202131344 -3600 # Node ID 4f877945eeba7e1c6a690150057b0221a882d002 # Parent f3ea51dea6f805fa354d173f38a9c5037e4785a3 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 f3ea51dea6f8 -r 4f877945eeba src/Virt_ResourcePoolConfigurationCapabilities.c --- a/src/Virt_ResourcePoolConfigurationCapabilities.c Mon Feb 04 14:22:16 2008 +0100 +++ b/src/Virt_ResourcePoolConfigurationCapabilities.c Mon Feb 04 14:22:24 2008 +0100 @@ -47,45 +47,63 @@ DEFAULT_EQ(); DEFAULT_EQ(); DEFAULT_INST_CLEANUP(); -static CMPIStatus get_rpc_cap(const CMPIBroker *broker, - const CMPIObjectPath *reference, - CMPIInstance **_inst) +static CMPIStatus get_rpc_cap(const CMPIObjectPath *reference, + CMPIInstance **_inst, + bool getInstance) { - CMPIInstance *inst; + CMPIStatus s = {CMPI_RC_OK, NULL}; + CMPIInstance *inst = NULL; + virConnectPtr conn = NULL; - inst = get_typed_instance(broker, - CLASSNAME(reference), + conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s); + if (conn == NULL) + goto out; + + inst = get_typed_instance(_BROKER, + 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 */ + if (getInstance) { + s = cu_validate_ref(_BROKER, reference, inst); + if (s.rc != CMPI_RC_OK) + goto out; + } + *_inst = inst; - return (CMPIStatus){CMPI_RC_OK, NULL}; + out: + 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; + CMPIInstance *inst = NULL; - s = get_rpc_cap(_BROKER, reference, &inst); - if (s.rc != CMPI_RC_OK) + s = get_rpc_cap(reference, &inst, getInstance); + if (s.rc != CMPI_RC_OK || inst == NULL) goto out; - + if (names_only) cu_return_instance_name(results, inst); else CMReturnInstance(results, inst); - + out: return s; } @@ -95,7 +113,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 +123,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 +132,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); }
HE> +static CMPIStatus get_rpc_cap(const CMPIObjectPath *reference, HE> + CMPIInstance **_inst, HE> + bool getInstance) This adds camelCase as well. Otherwise looks good. Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
HE> +static CMPIStatus get_rpc_cap(const CMPIObjectPath *reference, HE> + CMPIInstance **_inst, HE> + bool getInstance)
This adds camelCase as well.
Oopps ... I really don't know which horse was riding me here ... will fix this. Thanks. -- Regards Heidi Eckhart Software Engineer IBM Linux Technology Center - Open Hypervisor
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1202131345 -3600 # Node ID 971b870679bd5c11fb872c27b09150f612040eb9 # Parent 4f877945eeba7e1c6a690150057b0221a882d002 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 4f877945eeba -r 971b870679bd src/Virt_ResourcePoolConfigurationCapabilities.c --- a/src/Virt_ResourcePoolConfigurationCapabilities.c Mon Feb 04 14:22:24 2008 +0100 +++ b/src/Virt_ResourcePoolConfigurationCapabilities.c Mon Feb 04 14:22:25 2008 +0100 @@ -56,8 +56,13 @@ static CMPIStatus get_rpc_cap(const CMPI virConnectPtr conn = NULL; conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s); - if (conn == NULL) + if (conn == NULL) { + if (getInstance) + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance"); goto out; + } inst = get_typed_instance(_BROKER, pfx_from_conn(conn),
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1202131346 -3600 # Node ID 11ede99c6a8e94c930870f840af6d1a795bb6b63 # Parent 971b870679bd5c11fb872c27b09150f612040eb9 HS: adopt change in RPCS interface (get_rpcs_instance) Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r 971b870679bd -r 11ede99c6a8e src/Virt_HostedService.c --- a/src/Virt_HostedService.c Mon Feb 04 14:22:25 2008 +0100 +++ b/src/Virt_HostedService.c Mon Feb 04 14:22:26 2008 +0100 @@ -49,7 +49,7 @@ static CMPIStatus validate_service_ref(c if (STREQC(classname, "VirtualSystemManagementService")) { s = get_vsms(ref, &inst, _BROKER); } else if (STREQC(classname, "ResourcePoolConfigurationService")) { - s = rpcs_instance(ref, &inst, _BROKER); + s = get_rpcs(ref, &inst, _BROKER, true); } else if (STREQC(classname, "VirtualSystemMigrationService")) { s = get_migration_service(ref, &inst, _BROKER); } @@ -105,7 +105,7 @@ static CMPIStatus host_to_service(const if (s.rc != CMPI_RC_OK) return s; - s = rpcs_instance(ref, &inst, _BROKER); + s = get_rpcs(ref, &inst, _BROKER, false); if (s.rc != CMPI_RC_OK) return s; if (!CMIsNullObject(inst))
Heidi Eckhart wrote:
This patch set is now the combination of the one patch against RPCS and the two patches against RPCC, I sent out on friday. The reason to combine these three patches in one patch set is, that both provider - RPCS and RPCC - look very similar now. This brought me to the idea, that it is possible to also define a common interface for instances providers - as this is already done for associations -, where the common functions could do a main part of the error handling (which is currently done again and again in each single provider itself). In addition patch 4 updates HS (thanks to Kaitlin) with the new interface of RPCS.
I like the new interface of RPCS - good idea. =) +1 -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
participants (3)
-
Dan Smith -
Heidi Eckhart -
Kaitlin Rupert