[PATCH] Changed the output of AllocationCapability association when using DiskPool as input
# HG changeset patch # User Richard Maciel <rmaciel@linux.vnet.ibm.com> # Date 1236119796 10800 # Node ID d2d4d7e48f871111f655398e6900bc534231a55d # Parent 1aff0d0e9bf49e738827b7157c0df407b814ae7d Changed the output of AllocationCapability association when using DiskPool as input This patch fix the results of a 'association instances' query when passing a disk pool AllocationCapabilities reference as input. Before this patch, this query returned RASD templates for the disk pools, but now it returns the RASD templates for each of the volumes which composes the disk pool passed as input. To test: - Create a domain containing a diskpool and some volumes - Execute query: wbemcli ein 'http://localhost:5988/root/virt:CIM_AllocationCapabilities' - Select the reference to a diskpool and use in an association query like the one below: wbemcli ai -nl -ac KVM_SettingsDefineCapabilities 'http://@localhost:5988/root/virt:KVM_AllocationCapabilities.InstanceID="DiskPool/default"' - There must be four templates for each volume in the diskpool (MINIMUM, MAXIMUM, DEFAULT, INCREMENT) and their address must correspond to the address of the volume Signed-off-by: Richard Maciel <rmaciel@linux.vnet.ibm.com> diff -r 1aff0d0e9bf4 -r d2d4d7e48f87 src/Virt_SettingsDefineCapabilities.c --- a/src/Virt_SettingsDefineCapabilities.c Wed Mar 04 15:25:33 2009 -0800 +++ b/src/Virt_SettingsDefineCapabilities.c Tue Mar 03 19:36:36 2009 -0300 @@ -562,71 +562,23 @@ return s; } -static int get_disk_freespace(const CMPIObjectPath *ref, - CMPIStatus *s, - uint64_t *free_space) -{ - bool ret = false; - const char *inst_id; - CMPIrc prop_ret; - virConnectPtr conn; - CMPIInstance *pool_inst; - - if (cu_get_str_path(ref, "InstanceID", &inst_id) != CMPI_RC_OK) { - cu_statusf(_BROKER, s, - CMPI_RC_ERR_FAILED, - "Could not get InstanceID"); - goto out; - } - - conn = connect_by_classname(_BROKER, CLASSNAME(ref), s); - if (s->rc != CMPI_RC_OK) { - cu_statusf(_BROKER, s, - CMPI_RC_ERR_FAILED, - "Could not get connection"); - goto out; - } - - /* Getting the relevant resource pool directly finds the free space - * for us. It is in the Capacity field. */ - *s = get_pool_by_name(_BROKER, ref, inst_id, &pool_inst); - if (s->rc != CMPI_RC_OK) - goto out; - - prop_ret = cu_get_u64_prop(pool_inst, "Capacity", free_space); - if (prop_ret != CMPI_RC_OK) { - cu_statusf(_BROKER, s, - CMPI_RC_ERR_FAILED, - "Could not get capacity from instance"); - goto out; - } - - CU_DEBUG("Got capacity from pool_inst: %lld", *free_space); - ret = true; - - out: - return ret; -} - static CMPIStatus set_disk_props(int type, const CMPIObjectPath *ref, const char *id, + const char *disk_path, uint64_t disk_size, uint16_t emu_type, struct inst_list *list) { - const char *addr; const char *dev; CMPIInstance *inst; CMPIStatus s = {CMPI_RC_OK, NULL}; if (type == DOMAIN_LXC) { - addr = "/tmp"; dev = "/lxc_mnt/tmp"; } else { dev = "hda"; - addr = "/dev/null"; } inst = sdc_rasd_inst(&s, ref, CIM_RES_TYPE_DISK); @@ -638,7 +590,7 @@ (CMPIValue *)"MegaBytes", CMPI_chars); CMSetProperty(inst, "VirtualQuantity", (CMPIValue *)&disk_size, CMPI_uint64); - CMSetProperty(inst, "Address", (CMPIValue *)addr, CMPI_chars); + CMSetProperty(inst, "Address", (CMPIValue *)disk_path, CMPI_chars); if (type == DOMAIN_LXC) CMSetProperty(inst, "MountPoint", (CMPIValue *)dev, CMPI_chars); @@ -664,34 +616,28 @@ return s; } -static CMPIStatus disk_template(const CMPIObjectPath *ref, - int template_type, - struct inst_list *list) +static CMPIStatus cdrom_template(const CMPIObjectPath *ref, + int template_type, + struct inst_list *list) { - bool ret; char *pfx; const char *id; - uint64_t disk_size; - uint16_t emu_type = 0; + char *vol_path; + uint64_t vol_size = 0; CMPIStatus s = {CMPI_RC_OK, NULL}; + uint16_t emu_type = 1; switch(template_type) { case SDC_RASD_MIN: - disk_size = SDC_DISK_MIN; id = "Minimum"; break; case SDC_RASD_MAX: - ret = get_disk_freespace(ref, &s, &disk_size); - if (!ret) - goto out; id = "Maximum"; break; case SDC_RASD_INC: - disk_size = SDC_DISK_INC; id = "Increment"; break; case SDC_RASD_DEF: - disk_size = SDC_DISK_DEF; id = "Default"; break; default: @@ -701,6 +647,8 @@ goto out; } + vol_path = "/dev/null"; + pfx = class_prefix_name(CLASSNAME(ref)); if (STREQ(pfx, "Xen")) { @@ -708,48 +656,120 @@ int i = 0; for (; i < 2; i++) { - emu_type = 0; s = set_disk_props(xen_type[i], ref, - id, - disk_size, + id, + vol_path, + vol_size, emu_type, list); - if (s.rc != CMPI_RC_OK) - goto out; - - emu_type = 1; - s = set_disk_props(xen_type[i], - ref, - id, - disk_size, - emu_type, - list); - if (s.rc != CMPI_RC_OK) - goto out; } } else if (STREQ(pfx, "KVM")) { s = set_disk_props(DOMAIN_KVM, ref, - id, - disk_size, + id, + vol_path, + vol_size, emu_type, list); - if (s.rc != CMPI_RC_OK) - goto out; - emu_type = 1; + } else if (!STREQ(pfx, "LXC")){ + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unsupported virtualization type"); + } + + out: + return s; + +} + + +static CMPIStatus volume_template(const CMPIObjectPath *ref, + int template_type, + virStorageVolPtr volume_ptr, + struct inst_list *list) +{ + char *pfx; + const char *id; + char *vol_path; + uint64_t vol_size; + virStorageVolInfo vol_info; + CMPIStatus s = {CMPI_RC_OK, NULL}; + int retval; + uint16_t emu_type = 0; + + retval = virStorageVolGetInfo(volume_ptr, &vol_info); + if (retval == -1) { + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + virStorageVolGetConnect(volume_ptr), + "Unable to get volume information"); + goto out; + } + + switch(template_type) { + case SDC_RASD_MIN: + vol_size = SDC_DISK_MIN; + id = "Minimum"; + break; + case SDC_RASD_MAX: + vol_size = (uint64_t)vol_info.capacity; + id = "Maximum"; + break; + case SDC_RASD_INC: + vol_size = SDC_DISK_INC; + id = "Increment"; + break; + case SDC_RASD_DEF: + vol_size = (uint64_t)vol_info.allocation; + id = "Default"; + break; + default: + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unsupported sdc_rasd type"); + goto out; + } + + vol_path = virStorageVolGetPath(volume_ptr); + if (vol_path == NULL) { + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + virStorageVolGetConnect(volume_ptr), + "Unable to get volume path"); + goto out; + } + + pfx = class_prefix_name(CLASSNAME(ref)); + + if (STREQ(pfx, "Xen")) { + int xen_type[2] = {DOMAIN_XENFV, DOMAIN_XENPV}; + int i = 0; + + for (; i < 2; i++) { + s = set_disk_props(xen_type[i], + ref, + id, + vol_path, + vol_size, + emu_type, + list); + } + } else if (STREQ(pfx, "KVM")) { s = set_disk_props(DOMAIN_KVM, ref, - id, - disk_size, + id, + vol_path, + vol_size, emu_type, list); } else if (STREQ(pfx, "LXC")) { s = set_disk_props(DOMAIN_LXC, ref, id, - disk_size, + vol_path, + vol_size, emu_type, list); } else { @@ -762,6 +782,109 @@ return s; } +static CMPIStatus disk_template(const CMPIObjectPath *ref, + int template_type, + struct inst_list *list) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + virConnectPtr conn = NULL; + virStoragePoolPtr poolptr = NULL; + virStorageVolPtr volptr = NULL; + const char *instid = NULL; + char *host = NULL; + const char *poolname = NULL; + char **volnames = NULL; + int numvols, numvolsret = 0; + int i; + + + conn = connect_by_classname(_BROKER, CLASSNAME(ref), &s); + + if (cu_get_str_path(ref, "InstanceID", &instid) != CMPI_RC_OK) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to get InstanceID for disk device"); + goto out; + } + + if (parse_fq_devid(instid, &host, (char **)&poolname) != 1) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to get pool device id"); + goto out; + } + + if ((poolptr = virStoragePoolLookupByName(conn, poolname)) == NULL) { + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + conn, + "Storage pool `%s' not found", + poolname); + goto out; + } + + if ((numvols = virStoragePoolNumOfVolumes(poolptr)) == -1) { + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn, + "Unable to get the number of volumes \ + of storage pool `%s'", + poolname); + goto out; + } + + + volnames = (char **)malloc(sizeof(char *) * numvols); + + numvolsret = virStoragePoolListVolumes(poolptr, + volnames, + numvols); + + if (numvolsret == -1) { + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn, + "Unable to get a pointer to volumes \ + of storage pool `%s'", + poolname); + goto out; + } + + for (i = 0; i < numvolsret; i++) { + + volptr = virStorageVolLookupByName(poolptr, volnames[i]); + if (volptr == NULL) { + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + conn, + "Storage Volume `%s' not found", + volnames[i]); + goto out; + } + + s = volume_template(ref, + template_type, + volptr, + list); + if (s.rc != CMPI_RC_OK) + goto out; + } + + s = cdrom_template(ref, + template_type, + list); + + out: + if(volnames != NULL) + free(volnames); + + virStorageVolFree(volptr); + virStoragePoolFree(poolptr); + virConnectClose(conn); + + return s; +} + static CMPIStatus graphics_template(const CMPIObjectPath *ref, int template_type, struct inst_list *list)
@@ -638,7 +590,7 @@ (CMPIValue *)"MegaBytes", CMPI_chars); CMSetProperty(inst, "VirtualQuantity", (CMPIValue *)&disk_size, CMPI_uint64);
Instead of setting the vol_size as 0, just don't set the VirtualQuantity attribute for CDROM instances.
- CMSetProperty(inst, "Address", (CMPIValue *)addr, CMPI_chars); + CMSetProperty(inst, "Address", (CMPIValue *)disk_path, CMPI_chars);
if (type == DOMAIN_LXC) CMSetProperty(inst, "MountPoint", (CMPIValue *)dev, CMPI_chars); @@ -664,34 +616,28 @@ return s; }
-static CMPIStatus disk_template(const CMPIObjectPath *ref, - int template_type, - struct inst_list *list) +static CMPIStatus cdrom_template(const CMPIObjectPath *ref, + int template_type, + struct inst_list *list) { - bool ret; char *pfx; const char *id; - uint64_t disk_size; - uint16_t emu_type = 0; + char *vol_path; + uint64_t vol_size = 0; CMPIStatus s = {CMPI_RC_OK, NULL}; + uint16_t emu_type = 1;
switch(template_type) { case SDC_RASD_MIN: - disk_size = SDC_DISK_MIN; id = "Minimum";
To distinguish these instances from other instances, can you use something like "Minimum CDROM" or something similar?
+ +static CMPIStatus volume_template(const CMPIObjectPath *ref, + int template_type, + virStorageVolPtr volume_ptr, + struct inst_list *list) +{ + char *pfx; + const char *id; + char *vol_path; + uint64_t vol_size; + virStorageVolInfo vol_info; + CMPIStatus s = {CMPI_RC_OK, NULL}; + int retval; + uint16_t emu_type = 0; + + retval = virStorageVolGetInfo(volume_ptr, &vol_info);
Most of the rest of the code uses "ret" for this.
+ if (retval == -1) { + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + virStorageVolGetConnect(volume_ptr), + "Unable to get volume information"); + goto out; + } + + switch(template_type) { + case SDC_RASD_MIN: + vol_size = SDC_DISK_MIN;
If SDC_DISK_MIN is greater than vol_info.capacity, vol_info.capacity should be used instead.
+ } else if (STREQ(pfx, "KVM")) { s = set_disk_props(DOMAIN_KVM, ref, - id, - disk_size, + id, + vol_path, + vol_size, emu_type, list); } else if (STREQ(pfx, "LXC")) { s = set_disk_props(DOMAIN_LXC, ref, id, - disk_size, + vol_path,
Containers guests don't have a disk associated with them. Instead, the Address attribute points to a directory that is used as the root directory for the guest. So you don't want to generate an LXC guest for each volume in the diskpool. And you'll want to use a dummy directory (like "/tmp") for the Address.
+ vol_size, emu_type, list); } else { @@ -762,6 +782,109 @@ return s; }
+static CMPIStatus disk_template(const CMPIObjectPath *ref, + int template_type, + struct inst_list *list) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + virConnectPtr conn = NULL; + virStoragePoolPtr poolptr = NULL; + virStorageVolPtr volptr = NULL; + const char *instid = NULL; + char *host = NULL; + const char *poolname = NULL; + char **volnames = NULL; + int numvols, numvolsret = 0;
Declare these on separate lines.
+ int i; + + + conn = connect_by_classname(_BROKER, CLASSNAME(ref), &s); + + if (cu_get_str_path(ref, "InstanceID", &instid) != CMPI_RC_OK) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to get InstanceID for disk device"); + goto out; + } + + if (parse_fq_devid(instid, &host, (char **)&poolname) != 1) {
Passing a variable in this manner is pretty ugly. Instead, declare poolname as a char (and be sure to free it when you're done). The other providers do this as well, so it's good to be consistent when possible.
+ cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to get pool device id"); + goto out; + } + + if ((poolptr = virStoragePoolLookupByName(conn, poolname)) == NULL) {
Since virStoragePoolLookupByName(), use another variable and strdup() poolname. I know it's a few extra lines of code, but it's cleaner in my opinion.
+ + + volnames = (char **)malloc(sizeof(char *) * numvols);
Verify volnames was allocated appropriately. Return an error if it isn't.
+ + numvolsret = virStoragePoolListVolumes(poolptr, + volnames, + numvols);
You can fit all the params on one line if you'd like.
+ + if (numvolsret == -1) { + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_FAILED, + conn, + "Unable to get a pointer to volumes \ + of storage pool `%s'", + poolname); + goto out; + } + + for (i = 0; i < numvolsret; i++) { +
No space is needed here.
+ volptr = virStorageVolLookupByName(poolptr, volnames[i]); + if (volptr == NULL) { + virt_set_status(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + conn, + "Storage Volume `%s' not found", + volnames[i]); + goto out; + } + + s = volume_template(ref, + template_type, + volptr, + list);
Same here, all the params can fit on one line. We usually only break up a line if it spans 80 characters.
+ if (s.rc != CMPI_RC_OK) + goto out; + } + + s = cdrom_template(ref, + template_type, + list); + + out: + if(volnames != NULL)
No need for the if here. free() will check to see if the variable is NULL. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
participants (2)
-
Kaitlin Rupert -
Richard Maciel