Re: [Libvirt-cim] [PATCH 2 of 2] Add new provider: RedirectionService

KR> +#define SERVICE_TYPE 3 KR> +#define SHARING_MODE 3 KR> + KR> +#define ENABLED_STATE 2 KR> +#define REQUESTED_STATE 12 Should these be prefixed by SVPC_ and put in svpc_types.h? KR> +static CMPIStatus get_rs(const CMPIObjectPath *reference, KR> + CMPIInstance **_inst, KR> + const CMPIBroker *broker, KR> + const CMPIContext *context, KR> + bool is_get_inst) KR> +{ KR> + CMPIStatus s = {CMPI_RC_OK, NULL}; KR> + virConnectPtr conn = NULL; KR> + CMPIInstance *inst = NULL; KR> + CMPIArray *array; KR> + const char *name = NULL; KR> + const char *ccname = NULL; KR> + uint16_t prop_val; KR> + KR> + *_inst = NULL; KR> + conn = connect_by_classname(broker, CLASSNAME(reference), &s); KR> + if (conn == NULL) { KR> + if (is_get_inst) KR> + cu_statusf(broker, &s, KR> + CMPI_RC_ERR_NOT_FOUND, KR> + "No such instance"); KR> + KR> + return s; KR> + } KR> + KR> + inst = get_typed_instance(broker, KR> + pfx_from_conn(conn), KR> + "RedirectionService", KR> + NAMESPACE(reference)); KR> + KR> + if (inst == NULL) { KR> + CU_DEBUG("Failed to get typed instance"); KR> + cu_statusf(broker, &s, KR> + CMPI_RC_ERR_FAILED, KR> + "Failed to create instance"); KR> + goto out; KR> + } KR> + KR> + s = get_host_system_properties(&name, KR> + &ccname, KR> + reference, KR> + broker, KR> + context); KR> + if (s.rc != CMPI_RC_OK) { KR> + cu_statusf(broker, &s, KR> + CMPI_RC_ERR_FAILED, KR> + "Unable to get host attributes"); KR> + goto out; KR> + } KR> + KR> + CMSetProperty(inst, "Name", KR> + (CMPIValue *)"Redirection Service", CMPI_chars); KR> + KR> + CMSetProperty(inst, "SystemName", KR> + (CMPIValue *)name, CMPI_chars); KR> + KR> + CMSetProperty(inst, "SystemCreationClassName", KR> + (CMPIValue *)ccname, CMPI_chars); KR> + KR> + CMSetProperty(inst, "ElementName", KR> + (CMPIValue *)"Redirection Service", CMPI_chars); KR> + This function already has a lot packed into it. Should we break out the high-level pieces a bit? KR> + array = CMNewArray(broker, 1, CMPI_uint16, &s); KR> + if ((s.rc != CMPI_RC_OK) || CMIsNullObject(array)) KR> + goto out; KR> + KR> + prop_val = (uint16_t)SERVICE_TYPE; KR> + CMSetArrayElementAt(array, 0, &prop_val, CMPI_uint16); KR> + KR> + CMSetProperty(inst, "RedirectionServiceType", KR> + (CMPIValue *)&array, CMPI_uint16A); KR> + KR> + prop_val = (uint16_t) virConnectNumOfDomains(conn) + KR> + (uint16_t) virConnectNumOfDefinedDomains(conn); If one or more of these fail, you'll get an incorrect count, and potentially a negative value. Since your prop_val is unsigned, this could result in exposing 65535 (or so) "MaxCurrentEnabledSAPs", which isn't quite right. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms(a)us.ibm.com