# HG changeset patch # User Jay Gagnon <grendel@linux.vnet.ibm.com> # Date 1211297079 14400 # Node ID f00dd77c664c258551c5f884a0bee5d842241e4e # Parent c949b1845113e11e01329abb5aa81ca6ade8e3ec (#4) ProcessorRASD, the class that won't go away Around and round we go on the merry-go-round of indecision, hopefully for our last ride. The definitely probably absolutely tentative final plan is that the default representation of processors as virt_device structs will be one per domain, with a quantity. Virt_Devices will just need to be told how to handle that, and all the RASD stuff will be much happier for it. And then we will never speak of this again. So today being my last official day on the team, I kind of have to get this out in a not perfect state. From what I can see it does what it is supposed to do, but it is entirely possible (likely?) that there are a few things in here that aren't really great things to do. I wish I could polish it more but I think I've at least got it to the point where any work left to be done is generic "not a great idea in C" problems, as opposed to the much more annoying "what exactly are we supposed to do in this case" problems. Changes from 1 to 2: Close brace on if block. That one is really embarrassing; I have no idea what happened there. I mean that crap won't even compile. Removed unnecessary proc_found boolean. That was me being lazy after changing my mind on something. Cleaned up a couple leftover CU_DEBUG calls. Addition of vcpu_inst function, which is similar to vcpu_instances but always only works on one instance and takes a dev_id. This is to prevent all instances from being given a dev_id of zero. It is unfortunately similar to vcpu_instances in other regards, but I'm not sure if it's worth the overhead of trying to combine the similar parts. Fixed quantity v number problem in xml_parse_test that I appear to have been failing to address for about six years. Changes from 2 to 3: Actually include changes needed for associations. Make vcpu_instances use vcpu_inst. Changes from 3 to 4: Rename list_devs to something more meaningful Fix misplaced free. Signed-off-by: Jay Gagnon <grendel@linux.vnet.ibm.com> Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r c949b1845113 -r f00dd77c664c libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c Thu May 15 17:56:39 2008 -0700 +++ b/libxkutil/device_parsing.c Tue May 20 11:24:39 2008 -0400 @@ -337,7 +337,6 @@ 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) @@ -347,24 +346,15 @@ static int parse_vcpu_device(xmlNode *no free(count_str); - list = calloc(count, sizeof(*list)); + list = calloc(1, sizeof(*list)); if (list == NULL) goto err; - - 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; - } + + list->dev.vcpu.quantity = count; *vdevs = list; - return count; + return 1; err: free(list); @@ -620,7 +610,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.number = _dev->dev.vcpu.number; + dev->dev.vcpu.quantity = _dev->dev.vcpu.quantity; } else if (dev->type == CIM_RES_TYPE_EMU) { DUP_FIELD(dev, _dev, dev.emu.path); } else if (dev->type == CIM_RES_TYPE_GRAPHICS) { @@ -672,6 +662,32 @@ 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; @@ -683,6 +699,8 @@ 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 c949b1845113 -r f00dd77c664c libxkutil/device_parsing.h --- a/libxkutil/device_parsing.h Thu May 15 17:56:39 2008 -0700 +++ b/libxkutil/device_parsing.h Tue May 20 11:24:39 2008 -0400 @@ -50,7 +50,7 @@ struct mem_device { }; struct vcpu_device { - uint64_t number; + uint64_t quantity; }; struct emu_device { diff -r c949b1845113 -r f00dd77c664c libxkutil/xml_parse_test.c --- a/libxkutil/xml_parse_test.c Thu May 15 17:56:39 2008 -0700 +++ b/libxkutil/xml_parse_test.c Tue May 20 11:24:39 2008 -0400 @@ -96,7 +96,7 @@ static void print_dev_vcpu(struct virt_d static void print_dev_vcpu(struct virt_device *dev, FILE *d) { - print_u64(d, "Virtual CPU", dev->dev.vcpu.number); + print_u64(d, "Virtual CPU", dev->dev.vcpu.quantity); } static void print_dev_emu(struct virt_device *dev, diff -r c949b1845113 -r f00dd77c664c src/Virt_Device.c --- a/src/Virt_Device.c Thu May 15 17:56:39 2008 -0700 +++ b/src/Virt_Device.c Tue May 20 11:24:39 2008 -0400 @@ -174,23 +174,6 @@ static CMPIInstance *mem_instance(const return inst; } -static CMPIInstance *vcpu_instance(const CMPIBroker *broker, - struct vcpu_device *dev, - const virDomainPtr dom, - const char *ns) -{ - CMPIInstance *inst; - virConnectPtr conn; - - conn = virDomainGetConnect(dom); - inst = get_typed_instance(broker, - pfx_from_conn(conn), - "Processor", - ns); - - return inst; -} - static int device_set_devid(CMPIInstance *instance, struct virt_device *dev, const virDomainPtr dom) @@ -229,43 +212,127 @@ static int device_set_systemname(CMPIIns return 1; } -static CMPIInstance *device_instance(const CMPIBroker *broker, - struct virt_device *dev, - const virDomainPtr dom, - const char *ns) +static char *get_vcpu_inst_id(const virDomainPtr dom, + int proc_num) { - CMPIInstance *instance; + int rc; + char *id_num = NULL; + char *dev_id = NULL; - if (dev->type == CIM_RES_TYPE_NET) - instance = net_instance(broker, - &dev->dev.net, - dom, - ns); - else if (dev->type == CIM_RES_TYPE_DISK) - instance = disk_instance(broker, - &dev->dev.disk, - dom, - ns); - else if (dev->type == CIM_RES_TYPE_MEM) - instance = mem_instance(broker, - &dev->dev.mem, - dom, - ns); - else if (dev->type == CIM_RES_TYPE_PROC) - instance = vcpu_instance(broker, - &dev->dev.vcpu, - dom, - ns); - else - return NULL; + rc = asprintf(&id_num, "%d", proc_num); + if (rc == -1) { + free(dev_id); + dev_id = NULL; + goto out; + } + + dev_id = get_fq_devid((char *)virDomainGetName(dom), id_num); + free(id_num); - if (!instance) - return NULL; + out: + return dev_id; +} - device_set_devid(instance, dev, dom); - device_set_systemname(instance, dom); +static bool vcpu_inst(const CMPIBroker *broker, + const virDomainPtr dom, + const char *ns, + int dev_id_num, + struct inst_list *list) +{ + char *dev_id; + CMPIInstance *inst; + virConnectPtr conn = NULL; - return instance; + conn = virDomainGetConnect(dom); + inst = get_typed_instance(broker, + pfx_from_conn(conn), + "Processor", + ns); + if (inst == NULL) + return false; + + dev_id = get_vcpu_inst_id(dom, dev_id_num); + CMSetProperty(inst, "DeviceID", + (CMPIValue *)dev_id, CMPI_chars); + free(dev_id); + + device_set_systemname(inst, dom); + inst_list_add(list, inst); + + return true; +} + +static bool vcpu_instances(const CMPIBroker *broker, + const virDomainPtr dom, + const char *ns, + uint64_t proc_count, + struct inst_list *list) +{ + int i; + bool rc; + + for (i = 0; i < proc_count; i++) { + rc = vcpu_inst(broker, dom, ns, i, list); + if (!rc) + return false; + } + + return true; +} + +static bool device_instances(const CMPIBroker *broker, + struct virt_device *devs, + int count, + const virDomainPtr dom, + const char *ns, + struct inst_list *list) +{ + int i; + bool ret; + uint64_t proc_count = 0; + CMPIInstance *instance = NULL; + + for (i = 0; i < count; i++) { + struct virt_device *dev = &devs[i]; + + if (dev->type == CIM_RES_TYPE_NET) + instance = net_instance(broker, + &dev->dev.net, + dom, + ns); + else if (dev->type == CIM_RES_TYPE_DISK) + instance = disk_instance(broker, + &dev->dev.disk, + dom, + ns); + else if (dev->type == CIM_RES_TYPE_MEM) + instance = mem_instance(broker, + &dev->dev.mem, + dom, + ns); + else if (dev->type == CIM_RES_TYPE_PROC) { + proc_count = dev->dev.vcpu.quantity; + continue; + } else + return false; + + if (!instance) + return false; + + device_set_devid(instance, dev, dom); + device_set_systemname(instance, dom); + inst_list_add(list, instance); + } + + if (proc_count) { + ret = vcpu_instances(broker, + dom, + ns, + proc_count, + list); + } + + return true; } uint16_t res_type_from_device_classname(const char *classname) @@ -290,25 +357,27 @@ static CMPIStatus _get_devices(const CMP { CMPIStatus s = {CMPI_RC_OK, NULL}; int count; - int i; + bool rc; struct virt_device *devs = NULL; count = get_devices(dom, &devs, type); if (count <= 0) goto out; - for (i = 0; i < count; i++) { - CMPIInstance *dev = NULL; + rc = device_instances(broker, + devs, + count, + dom, + NAMESPACE(reference), + list); - dev = device_instance(broker, - &devs[i], - dom, - NAMESPACE(reference)); - if (dev) - inst_list_add(list, dev); + if (!rc) { + cu_statusf(broker, &s, + CMPI_RC_ERR_FAILED, + "Couldn't get device instances"); + } - cleanup_virt_device(&devs[i]); - } + cleanup_virt_devices(&devs, count); out: free(devs); @@ -427,6 +496,29 @@ static int parse_devid(const char *devid return 1; } +static int proc_dev_list(uint64_t quantity, + struct virt_device **list) +{ + int i; + int rc; + + + *list = (struct virt_device *)calloc(quantity, + sizeof(struct virt_device)); + + for (i = 0; i < quantity; i++) { + char *dev_num; + + rc = asprintf(&dev_num, "%d", i); + + (*list)[i].id = strdup(dev_num); + + free(dev_num); + } + + return quantity; +} + static struct virt_device *find_dom_dev(virDomainPtr dom, char *device, int type) @@ -439,6 +531,17 @@ static struct virt_device *find_dom_dev( count = get_devices(dom, &list, type); if (!count) goto out; + + if (type == CIM_RES_TYPE_PROC) { + struct virt_device *tmp_list; + int tmp_count; + + tmp_count = proc_dev_list(list[0].dev.vcpu.quantity, + &tmp_list); + cleanup_virt_devices(&list, count); + list = tmp_list; + count = tmp_count; + } for (i = 0; i < count; i++) { if (STREQC(device, list[i].id)) @@ -462,10 +565,13 @@ CMPIStatus get_device_by_name(const CMPI CMPIStatus s = {CMPI_RC_OK, NULL}; char *domain = NULL; char *device = NULL; - CMPIInstance *instance = NULL; virConnectPtr conn = NULL; virDomainPtr dom = NULL; struct virt_device *dev = NULL; + struct inst_list tmp_list; + bool rc; + + inst_list_init(&tmp_list); conn = connect_by_classname(broker, CLASSNAME(reference), &s); if (conn == NULL) { @@ -501,13 +607,30 @@ CMPIStatus get_device_by_name(const CMPI goto err; } - instance = device_instance(broker, - dev, - dom, - NAMESPACE(reference)); + if (type == CIM_RES_TYPE_PROC) { + int ret; + int dev_id_num; + + ret = sscanf(dev->id, "%d", &dev_id_num); + + rc = vcpu_inst(broker, + dom, + NAMESPACE(reference), + dev_id_num, + &tmp_list); + } else { + + rc = device_instances(broker, + dev, + 1, + dom, + NAMESPACE(reference), + &tmp_list); + } + cleanup_virt_device(dev); - *_inst = instance; + *_inst = tmp_list.list[0]; err: virDomainFree(dom); @@ -515,6 +638,7 @@ CMPIStatus get_device_by_name(const CMPI free(device); out: + inst_list_free(&tmp_list); virConnectClose(conn); return s; diff -r c949b1845113 -r f00dd77c664c src/Virt_RASD.c --- a/src/Virt_RASD.c Thu May 15 17:56:39 2008 -0700 +++ b/src/Virt_RASD.c Tue May 20 11:24:39 2008 -0400 @@ -31,7 +31,6 @@ #include <libcmpiutil/libcmpiutil.h> #include <libcmpiutil/std_instance.h> -#include "device_parsing.h" #include "misc_util.h" #include "cs_util.h" @@ -56,10 +55,10 @@ static struct virt_device *_find_dev(str return NULL; } -static int list_devs(virConnectPtr conn, - const uint16_t type, - const char *host, - struct virt_device **list) +int list_rasds(virConnectPtr conn, + const uint16_t type, + const char *host, + struct virt_device **list) { virDomainPtr dom; @@ -79,7 +78,7 @@ static struct virt_device *find_dev(virC struct virt_device *list = NULL; struct virt_device *dev = NULL; - count = list_devs(conn, type, host, &list); + count = list_rasds(conn, type, host, &list); if (count > 0) { dev = _find_dev(list, count, devid); cleanup_virt_devices(&list, count); @@ -173,10 +172,8 @@ 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. */ - uint64_t quantity = dev->dev.vcpu.number + 1; CMSetProperty(inst, "VirtualQuantity", - (CMPIValue *)&quantity, CMPI_uint64); + (CMPIValue *)&dev->dev.vcpu.quantity, CMPI_uint64); } /* FIXME: Put the HostResource in place */ diff -r c949b1845113 -r f00dd77c664c src/Virt_RASD.h --- a/src/Virt_RASD.h Thu May 15 17:56:39 2008 -0700 +++ b/src/Virt_RASD.h Tue May 20 11:24:39 2008 -0400 @@ -20,6 +20,8 @@ */ #ifndef __VIRT_RASD_H #define __VIRT_RASD_H + +#include "device_parsing.h" char *rasd_to_xml(CMPIInstance *rasd); @@ -55,6 +57,10 @@ CMPIStatus get_rasd_by_ref(const CMPIBro const char **properties, CMPIInstance **_inst); +int list_rasds(virConnectPtr conn, + const uint16_t type, + const char *host, + struct virt_device **list); #endif /* diff -r c949b1845113 -r f00dd77c664c src/Virt_SettingsDefineState.c --- a/src/Virt_SettingsDefineState.c Thu May 15 17:56:39 2008 -0700 +++ b/src/Virt_SettingsDefineState.c Tue May 20 11:24:39 2008 -0400 @@ -39,13 +39,95 @@ const static CMPIBroker *_BROKER; +static int get_proc_dev_count(const char *name, + const char *cn, + uint16_t type, + char *host, + CMPIStatus *s) +{ + virConnectPtr conn = NULL; + struct virt_device *list = NULL; + struct virt_device *dev = NULL; + int dev_count = -1; + int count = -1; + + conn = connect_by_classname(_BROKER, cn, s); + if (conn == NULL) { + cu_statusf(_BROKER, s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance (%s)", + name); + goto out; + } + + dev_count = list_rasds(conn, type, host, &list); + if (dev_count <= 0) { + cu_statusf(_BROKER, s, + CMPI_RC_ERR_FAILED, + "Unable to get list of processors"); + goto out; + } + + dev = &list[0]; + count = dev->dev.vcpu.quantity; + + cleanup_virt_devices(&list, dev_count); + + out: + virConnectClose(conn); + return count; +} + +static char *get_rasd_id(const CMPIInstance *inst, uint16_t type) +{ + char *id = NULL; + int ret; + const char *tmp; + + if (type == CIM_RES_TYPE_PROC) { + ret = cu_get_str_prop(inst, "SystemName", &tmp); + if (ret != CMPI_RC_OK) { + CU_DEBUG("No SystemName in device instance"); + goto out; + } + + ret = asprintf(&id, "%s/proc", tmp); + if (ret == -1) { + id = NULL; + goto out; + } + } else { + ret = cu_get_str_prop(inst, "DeviceID", &tmp); + if (ret != CMPI_RC_OK) { + CU_DEBUG("No DeviceID in device instance"); + id = NULL; + goto out; + } + + id = strdup(tmp); + } + + out: + return id; +} + +static char *get_dev_id(const char *host, int devid) +{ + char *id = NULL; + + if (asprintf(&id, "%s/%d", host, devid) == -1) + id = NULL; + + return id; +} + static CMPIStatus dev_to_rasd(const CMPIObjectPath *ref, struct std_assoc_info *info, struct inst_list *list) { CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIInstance *inst = NULL; - const char *name = NULL; + char *id = NULL; if (!match_hypervisor_prefix(ref, info)) return s; @@ -54,16 +136,18 @@ static CMPIStatus dev_to_rasd(const CMPI if (s.rc != CMPI_RC_OK) goto out; - if (cu_get_str_path(ref, "DeviceID", &name) != CMPI_RC_OK) { + id = get_rasd_id(inst, + res_type_from_device_classname(CLASSNAME(ref))); + if (id == NULL) { cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Missing DeviceID"); + CMPI_RC_ERR_NOT_FOUND, + "Unable to get RASD id from DeviceID"); goto out; - } + } s = get_rasd_by_name(_BROKER, ref, - name, + id, res_type_from_device_classname(CLASSNAME(ref)), NULL, &inst); @@ -73,6 +157,7 @@ static CMPIStatus dev_to_rasd(const CMPI inst_list_add(list, inst); out: + free(id); return s; } @@ -83,7 +168,12 @@ static CMPIStatus rasd_to_dev(const CMPI CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIInstance *inst = NULL; const char *name = NULL; + char *host = NULL; + char *devid = NULL; + char *id = NULL; uint16_t type; + int count = 1; + int i; if (!match_hypervisor_prefix(ref, info)) return s; @@ -92,6 +182,13 @@ static CMPIStatus rasd_to_dev(const CMPI if (s.rc != CMPI_RC_OK) goto out; + if (res_type_from_rasd_classname(CLASSNAME(ref), &type) != CMPI_RC_OK) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Missing ResourceType"); + goto out; + } + if (cu_get_str_path(ref, "InstanceID", &name) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, @@ -99,20 +196,48 @@ static CMPIStatus rasd_to_dev(const CMPI goto out; } - if (res_type_from_rasd_classname(CLASSNAME(ref), &type) != CMPI_RC_OK) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Missing ResourceType"); - goto out; + if (type == CIM_RES_TYPE_PROC) { + if (parse_fq_devid((char *)name, &host, &devid) != 1) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance (%s)", + name); + goto out; + } + + count = get_proc_dev_count(name, + CLASSNAME(ref), + type, + host, + &s); + if (count <= 0) + goto out; } - s = get_device_by_name(_BROKER, ref, name, type, &inst); - if (s.rc != CMPI_RC_OK) - goto out; + for (i = 0; i < count; i++) { + if (type == CIM_RES_TYPE_PROC) + id = get_dev_id(host, i); + else + id = strdup(name); - inst_list_add(list, inst); + if (id == NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + "Unable to get RASD id from DeviceID"); + goto out; + } + + s = get_device_by_name(_BROKER, ref, id, type, &inst); + if (s.rc != CMPI_RC_OK) + goto out; + + inst_list_add(list, inst); + free(id); + } out: + free(host); + free(devid); return s; }