Re: [Libvirt-cim] [PATCH 4 of 4] Add suport for CreateChildResourcePool

Kaitlin Rupert wrote: No need to do strdup here, since you strdup two lines after, no matter the result of cu_get_str_prop. Just do val = <string> Same here Same here Same here If you plan to keep the duplicated strdups, you should free the alloc'ed memory here. This will always return a NULL value. But you probably add this to be set when address validation fails, right? > + > +} > + > +static const char *rasd_to_vpool(CMPIInstance *inst, > + struct virt_pool *pool, > + uint16_t type, > + const char *ns) > +{ > + pool->type = type; > + > + if (type == CIM_RES_TYPE_NET) { > + return net_rasd_to_pool(inst, pool, ns); > + } > + > + pool->type = CIM_RES_TYPE_UNKNOWN; > + > + return "Resource type not supported on this platform"; > +} > + > +static const char *get_pool_properties(CMPIArray *settings, > + struct virt_pool *pool) > +{ > + CMPIObjectPath *op; > + CMPIData item; > + CMPIInstance *inst; > + const char *msg = NULL; > + uint16_t type; > + int count; > + > + count = CMGetArrayCount(settings, NULL); > + if (count < 1) > + return "No resources specified"; > + > + if (count > 1) > + CU_DEBUG("More than one RASD specified during pool creation"); > + > + item = CMGetArrayElementAt(settings, 0, NULL); > + if (CMIsNullObject(item.value.inst)) > + return "Internal array error"; > + > + inst = item.value.inst; > + > + op = CMGetObjectPath(inst, NULL); > + if (op == NULL) > + return "Unknown resource instance type"; > + > + if (res_type_from_rasd_classname(CLASSNAME(op), &type) != CMPI_RC_OK) > + return "Unable to determine resource type"; > + > + if (type != CIM_RES_TYPE_NET) > + return "Only network pools currently supported"; > + > + msg = rasd_to_vpool(inst, pool, type, NAMESPACE(op)); > +} > + > +static char *get_pool_id(int res_type, > + const char *name) > +{ > + char *id = NULL; > + const char *pool = NULL; > + > + if (res_type == CIM_RES_TYPE_NET) > + pool = "NetworkPool"; > + else if (res_type == CIM_RES_TYPE_DISK) > + pool = "DiskPool"; > + else if (res_type == CIM_RES_TYPE_MEM) > + pool = "MemoryPool"; > + else if (res_type == CIM_RES_TYPE_PROC) > + pool = "ProcessorPool"; > + else if (res_type == CIM_RES_TYPE_GRAPHICS) > + pool = "GraphicsPool"; > + else if (res_type == CIM_RES_TYPE_INPUT) > + pool = "InputPool"; > + else > + pool = "Unknown"; > + > + if (asprintf(&id, "%s/%s", pool, name) == -1) { > + return NULL; > + } > + > + return id; > +} > + > +static CMPIInstance *connect_and_create(char *xml, > + const CMPIObjectPath *ref, > + const char *id, > + int res_type, > + CMPIStatus *s) > +{ > + virConnectPtr conn; > + CMPIInstance *inst = NULL; > + > + conn = connect_by_classname(_BROKER, CLASSNAME(ref), s); > + if (conn == NULL) { > + CU_DEBUG("libvirt connection failed"); > + return NULL; > + } > + > + if (define_pool(conn, xml, res_type) == 0) { > + virt_set_status(_BROKER, s, > + CMPI_RC_ERR_FAILED, > + conn, > + "Unable to create resource pool"); > + goto out; > + } > + > + *s = get_pool_by_name(_BROKER, ref, id, &inst); > + if (s->rc != CMPI_RC_OK) { > + CU_DEBUG("Failed to get new pool instance: %s", id); > + cu_statusf(_BROKER, s, > + CMPI_RC_ERR_FAILED, > + "Failed to lookup resulting pool"); > + } > + > + out: > + virConnectClose(conn); > + > + return inst; > +} > + > +static CMPIStatus create_child_pool(CMPIMethodMI *self, > + const CMPIContext *context, > + const CMPIResult *results, > + const CMPIObjectPath *reference, > + const CMPIArgs *argsin, > + CMPIArgs *argsout) > +{ > + uint32_t rc = CIM_SVPC_RETURN_FAILED; > + CMPIStatus s = {CMPI_RC_OK, NULL}; > + CMPIInstance *inst = NULL; > + CMPIArray *set; > + CMPIArray *parent_pools; > + CMPIObjectPath *result; > + struct virt_pool *pool = NULL; > + const char *name = NULL; > + const char *msg = NULL; > + char *full_id = NULL; > + char *xml = NULL; > + > + CU_DEBUG("CreateResourcePool"); > + > + s = create_child_pool_parse_args(argsin, &name, &set, &parent_pools); > + if (s.rc != CMPI_RC_OK) > + goto out; > + > + pool = calloc(1, sizeof(*pool)); > + if (pool == NULL) { > + cu_statusf(_BROKER, &s, > + CMPI_RC_ERR_FAILED, > + "Failed to allocate pool struct"); > + goto out; > + } > + > + msg = get_pool_properties(set, pool); > + if (msg != NULL) { > + cu_statusf(_BROKER, &s, > + CMPI_RC_ERR_FAILED, > + "Settings Error: %s", msg); > + > + goto out; > + } > + > + full_id = get_pool_id(pool->type, name); > + if (full_id == NULL) { > + cu_statusf(_BROKER, &s, > + CMPI_RC_ERR_FAILED, > + "Unable to format resulting pool ID"); > + goto out; > + } > + > + s = get_pool_by_name(_BROKER, reference, full_id, &inst); > + if (s.rc == CMPI_RC_OK) { > + cu_statusf(_BROKER, &s, > + CMPI_RC_ERR_FAILED, > + "Pool with that name already exists"); > + goto out; > + } > + > + pool->id = strdup(name); You can use name directly, since the create_child_pool_parse_args already allocs memory for the name var. Or, if you decide to keep that way, free memory from the name var after this. Missing free(msg), free(pool) How about free(inst)? -- Richard Maciel, MSc IBM Linux Technology Center rmaciel(a)linux.vnet.ibm.com