[PATCH] (#2) Fix device_parsing of processors

# HG changeset patch # User Jay Gagnon <grendel@linux.vnet.ibm.com> # Date 1208274605 14400 # Node ID f0153bd85ec1bdffabaf99266bc65472e1c080ad # Parent 01b0a24fc16f9daff19f3733552f223ddf6d7a18 (#2) Fix device_parsing of processors. Kaitlin discovered that my changes to device_parsing.c make it so we only get one Processor as well, not just one Proc RASD. This moves the flattening step up a little higher so that enumerating on Processor works correctly. It's not the prettiest approach to things, but I think given Processor's unique Device-RASD relationship it's a good compromise. I would of course be open to suggestions for making it a bit more elegant. Changes from 1 to 2: Fix xml_parse_test.c's print_dev_vcpu, and by "fix" I mean "fix, because once again I made a change that breaks it." Signed-off-by: Jay Gagnon <grendel@linux.vnet.ibm.com> diff -r 01b0a24fc16f -r f0153bd85ec1 libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c Mon Apr 14 15:17:08 2008 -0700 +++ b/libxkutil/device_parsing.c Tue Apr 15 11:50:05 2008 -0400 @@ -337,6 +337,7 @@ static int parse_vcpu_device(xmlNode *no struct virt_device *list = NULL; char *count_str; int count; + int i; count_str = get_node_content(node); if (count_str == NULL) @@ -346,15 +347,24 @@ static int parse_vcpu_device(xmlNode *no free(count_str); - list = calloc(1, sizeof(*list)); + list = calloc(count, sizeof(*list)); if (list == NULL) goto err; - - list->dev.vcpu.quantity = count; + + for (i = 0; i < count; i++) { + struct virt_device *vdev = &list[i]; + struct vcpu_device *cdev = &vdev->dev.vcpu; + + cdev->number = i; + + vdev->type = CIM_RES_TYPE_PROC; + if (asprintf(&vdev->id, "%i", i) == -1) + vdev->id = NULL; + } *vdevs = list; - return 1; + return count; err: free(list); @@ -610,7 +620,7 @@ struct virt_device *virt_device_dup(stru dev->dev.mem.size = _dev->dev.mem.size; dev->dev.mem.maxsize = _dev->dev.mem.maxsize; } else if (dev->type == CIM_RES_TYPE_PROC) { - dev->dev.vcpu.quantity = _dev->dev.vcpu.quantity; + dev->dev.vcpu.number = _dev->dev.vcpu.number; } else if (dev->type == CIM_RES_TYPE_EMU) { DUP_FIELD(dev, _dev, dev.emu.path); } else if (dev->type == CIM_RES_TYPE_GRAPHICS) { @@ -662,32 +672,6 @@ static int _get_mem_device(const char *x return 1; } -static int _get_proc_device(const char *xml, struct virt_device **list) -{ - struct virt_device *proc_devs = NULL; - struct virt_device *proc_dev = NULL; - int ret; - - ret = parse_devices(xml, &proc_devs, CIM_RES_TYPE_PROC); - if (ret <= 0) - return ret; - - proc_dev = malloc(sizeof(*proc_dev)); - if (proc_dev == NULL) - return 0; - - memset(proc_dev, 0, sizeof(*proc_dev)); - - proc_dev->type = CIM_RES_TYPE_PROC; - proc_dev->id = strdup("proc"); - proc_dev->dev.vcpu.quantity = proc_devs[0].dev.vcpu.quantity; - *list = proc_dev; - - cleanup_virt_devices(&proc_devs, ret); - - return 1; -}; - int get_devices(virDomainPtr dom, struct virt_device **list, int type) { char *xml; @@ -699,8 +683,6 @@ int get_devices(virDomainPtr dom, struct if (type == CIM_RES_TYPE_MEM) ret = _get_mem_device(xml, list); - else if (type == CIM_RES_TYPE_PROC) - ret = _get_proc_device(xml, list); else ret = parse_devices(xml, list, type); diff -r 01b0a24fc16f -r f0153bd85ec1 libxkutil/device_parsing.h --- a/libxkutil/device_parsing.h Mon Apr 14 15:17:08 2008 -0700 +++ b/libxkutil/device_parsing.h Tue Apr 15 11:50:05 2008 -0400 @@ -50,7 +50,7 @@ struct mem_device { }; struct vcpu_device { - uint32_t quantity; + uint32_t number; }; struct emu_device { diff -r 01b0a24fc16f -r f0153bd85ec1 libxkutil/xml_parse_test.c --- a/libxkutil/xml_parse_test.c Mon Apr 14 15:17:08 2008 -0700 +++ b/libxkutil/xml_parse_test.c Tue Apr 15 11:50:05 2008 -0400 @@ -94,7 +94,7 @@ static void print_dev_vcpu(struct virt_d static void print_dev_vcpu(struct virt_device *dev, FILE *d) { - print_u32(d, "Virtual CPUs", dev->dev.vcpu.quantity); + print_u32(d, "Virtual CPU", dev->dev.vcpu.number); } static void print_dev_emu(struct virt_device *dev, diff -r 01b0a24fc16f -r f0153bd85ec1 src/Virt_RASD.c --- a/src/Virt_RASD.c Mon Apr 14 15:17:08 2008 -0700 +++ b/src/Virt_RASD.c Tue Apr 15 11:50:05 2008 -0400 @@ -169,8 +169,10 @@ static CMPIInstance *rasd_from_vdev(cons CMSetProperty(inst, "Limit", (CMPIValue *)&dev->dev.mem.maxsize, CMPI_uint64); } else if (dev->type == CIM_RES_TYPE_PROC) { + /* This would be the place to set the virtualquantity. */ + int quantity = dev->dev.vcpu.number + 1; CMSetProperty(inst, "VirtualQuantity", - (CMPIValue *)&dev->dev.vcpu.quantity, CMPI_uint32); + (CMPIValue *)&quantity, CMPI_uint32); } /* FIXME: Put the HostResource in place */ @@ -343,6 +345,22 @@ static CMPIStatus _get_rasds(const CMPIB if (count <= 0) goto out; + /* Bit hackish, but for proc we need to cut list down to one. */ + if (type == CIM_RES_TYPE_PROC) { + struct virt_device *tmp_dev = NULL; + tmp_dev = calloc(1, sizeof(*tmp_dev)); + tmp_dev = virt_device_dup(&devs[count - 1]); + + tmp_dev->id = strdup("proc"); + + for (i = 0; i < count; i++) + cleanup_virt_device(&devs[i]); + + free(devs); + devs = tmp_dev; + count = 1; + } + for (i = 0; i < count; i++) { CMPIInstance *dev = NULL; const char *host = NULL;

@@ -346,15 +347,24 @@ static int parse_vcpu_device(xmlNode *no
free(count_str);
- list = calloc(1, sizeof(*list)); + list = calloc(count, sizeof(*list)); if (list == NULL) goto err; - - list->dev.vcpu.quantity = count; + + for (i = 0; i < count; i++) { + struct virt_device *vdev = &list[i]; + struct vcpu_device *cdev = &vdev->dev.vcpu; + + cdev->number = i; + + vdev->type = CIM_RES_TYPE_PROC; + if (asprintf(&vdev->id, "%i", i) == -1) + vdev->id = NULL; + }
*vdevs = list;
- return 1; + return count;
You could do return count + 1 here and then use the quantity instead of number. This would save you a step in the rasd_from_vdev() call. But either way is fine by me.
static void print_dev_emu(struct virt_device *dev, diff -r 01b0a24fc16f -r f0153bd85ec1 src/Virt_RASD.c --- a/src/Virt_RASD.c Mon Apr 14 15:17:08 2008 -0700 +++ b/src/Virt_RASD.c Tue Apr 15 11:50:05 2008 -0400 @@ -169,8 +169,10 @@ static CMPIInstance *rasd_from_vdev(cons CMSetProperty(inst, "Limit", (CMPIValue *)&dev->dev.mem.maxsize, CMPI_uint64); } else if (dev->type == CIM_RES_TYPE_PROC) { + /* This would be the place to set the virtualquantity. */ + int quantity = dev->dev.vcpu.number + 1;
Probably doesn't matter for this case, but the type here should match the type you set in CMSetProperty.
CMSetProperty(inst, "VirtualQuantity", - (CMPIValue *)&dev->dev.vcpu.quantity, CMPI_uint32); + (CMPIValue *)&quantity, CMPI_uint32);
This needs to be CMPI_uint64 (per the mof) - otherwise, the attribute might not set correctly in all cases. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com

Kaitlin Rupert wrote:
- return 1; + return count;
You could do return count + 1 here and then use the quantity instead of number. This would save you a step in the rasd_from_vdev() call.
But either way is fine by me. I think returning count + 1 here would cause problems for non-proc RASDs. I basically went with the assumption that this time around I should make as few changes as possible to code that wasn't explicitly only going to run for proc RASDs.
Probably doesn't matter for this case, but the type here should match the type you set in CMSetProperty.
This needs to be CMPI_uint64 (per the mof) - otherwise, the attribute might not set correctly in all cases.
Okay, I can fix those things easily enough. -- -Jay
participants (2)
-
Jay Gagnon
-
Kaitlin Rupert