[PATCH 0 of 2] [RFC] First steps in new processor RASD behavior
Just want to get this out before I start in on the resource add/mod/del stuff, to see if I'm on the right track. The idea here is that we are not supporting processor pinning, will support scheduling (weight/limit attributes), and are making Processor RASDs one-per-domain. Does this look sane?
# HG changeset patch # User Jay Gagnon <grendel@linux.vnet.ibm.com> # Date 1207593066 14400 # Node ID ade926a6ad096be7c1bebab3eb7ac4080aac9088 # Parent 707113ae82b8cff34acaa1808c0ddb89237e3d31 Remove processor pinning RASD code We have decided to support domain scheduling but not CPU pinning, since it's a nightmare CIM-wise, and not exactly fun elsewise. This means we need to take the pinnning representation support out of the tree. Signed-off-by: Jay Gagnon <grendel@linux.vnet.ibm.com> diff -r 707113ae82b8 -r ade926a6ad09 src/Virt_RASD.c --- a/src/Virt_RASD.c Fri Apr 04 08:58:29 2008 -0700 +++ b/src/Virt_RASD.c Mon Apr 07 14:31:06 2008 -0400 @@ -23,7 +23,6 @@ #include <string.h> #include <inttypes.h> #include <sys/stat.h> -#include <unistd.h> #include <cmpidt.h> #include <cmpift.h> @@ -93,191 +92,6 @@ char *rasd_to_xml(CMPIInstance *rasd) { /* FIXME: Remove this */ return NULL; -} - -static bool proc_get_physical_ref(const CMPIBroker *broker, - uint32_t physnum, - struct inst_list *list) -{ - CMPIObjectPath *op = NULL; - CMPIStatus s; - char hostname[255]; - char *devid = NULL; - CMPIInstance *inst; - bool result = false; - - if (asprintf(&devid, "%i", physnum) == -1) { - CU_DEBUG("Failed to create DeviceID string"); - goto out; - } - - if (gethostname(hostname, sizeof(hostname)) == -1) { - CU_DEBUG("Hostname overflow"); - goto out; - } - - op = CMNewObjectPath(broker, "root/cimv2", "Linux_Processor", &s); - if ((op == NULL) || (s.rc != CMPI_RC_OK)) { - CU_DEBUG("Failed to get ObjectPath for processor"); - goto out; - } - - inst = CMNewInstance(broker, op, &s); - if ((inst == NULL) || (s.rc != CMPI_RC_OK)) { - CU_DEBUG("Failed to make instance"); - goto out; - } - - CMSetProperty(inst, "CreationClassName", - (CMPIValue *)"Linux_Processor", CMPI_chars); - CMSetProperty(inst, "SystemName", - (CMPIValue *)hostname, CMPI_chars); - CMSetProperty(inst, "SystemCreationClassName", - (CMPIValue *)"Linux_ComputerSystem", CMPI_chars); - CMSetProperty(inst, "DeviceID", - (CMPIValue *)devid, CMPI_chars); - - inst_list_add(list, inst); - - result = true; - out: - free(devid); - - return result; -} - -static uint32_t proc_set_cpu(const CMPIBroker *broker, - virNodeInfoPtr node, - virDomainPtr dom, - struct virt_device *dev, - struct inst_list *list) -{ - virVcpuInfoPtr vinfo = NULL; - virDomainInfo info; - uint8_t *cpumaps = NULL; - int ret; - int i; - int vcpu = dev->dev.vcpu.number; - int maplen = VIR_CPU_MAPLEN(VIR_NODEINFO_MAXCPUS(*node)); - - ret = virDomainGetInfo(dom, &info); - if (ret == -1) { - CU_DEBUG("Failed to get info for domain `%s'", - virDomainGetName(dom)); - goto out; - } - - if (dev->dev.vcpu.number >= info.nrVirtCpu) { - CU_DEBUG("VCPU %i higher than max of %i for %s", - dev->dev.vcpu.number, - info.nrVirtCpu, - virDomainGetName(dom)); - goto out; - } - - vinfo = calloc(info.nrVirtCpu, sizeof(*vinfo)); - if (vinfo == NULL) { - CU_DEBUG("Failed to allocate memory for %i virVcpuInfo", - info.nrVirtCpu); - goto out; - } - - cpumaps = calloc(info.nrVirtCpu, maplen); - if (cpumaps == NULL) { - CU_DEBUG("Failed to allocate memory for %ix%i maps", - info.nrVirtCpu, maplen); - goto out; - } - - ret = virDomainGetVcpus(dom, vinfo, info.nrVirtCpu, cpumaps, maplen); - if (ret < info.nrVirtCpu) { - CU_DEBUG("Failed to get VCPU info for %s", - virDomainGetName(dom)); - goto out; - } - - for (i = 0; i < VIR_NODEINFO_MAXCPUS(*node); i++) { - if (VIR_CPU_USABLE(cpumaps, maplen, vcpu, i)) { - CU_DEBUG("VCPU %i pinned to physical %i", - vcpu, i); - proc_get_physical_ref(broker, i, list); - } else { - CU_DEBUG("VCPU %i not pinned to physical %i", - vcpu, i); - } - } - out: - free(vinfo); - free(cpumaps); - - return 0; -} - -static CMPIStatus proc_rasd_from_vdev(const CMPIBroker *broker, - struct virt_device *dev, - const char *host, - const CMPIObjectPath *ref, - CMPIInstance *inst) -{ - virConnectPtr conn = NULL; - virDomainPtr dom = NULL; - virNodeInfo node; - CMPIStatus s; - CMPIArray *array; - struct inst_list list; - - inst_list_init(&list); - - conn = connect_by_classname(broker, CLASSNAME(ref), &s); - if (conn == NULL) { - cu_statusf(broker, &s, - CMPI_RC_ERR_FAILED, - "Failed to connect for ProcRASD (%s)", - CLASSNAME(ref)); - goto out; - } - - dom = virDomainLookupByName(conn, host); - if (dom == NULL) { - cu_statusf(broker, &s, - CMPI_RC_ERR_NOT_FOUND, - "Unable to get domain for ProcRASD: %s", host); - goto out; - } - - if (virNodeGetInfo(virDomainGetConnect(dom), &node) == -1) { - cu_statusf(broker, &s, - CMPI_RC_ERR_FAILED, - "Unable to get node info"); - goto out; - } - - proc_set_cpu(broker, &node, dom, dev, &list); - - if (list.cur > 0) { - int i; - - array = CMNewArray(broker, - list.cur, - CMPI_instance, - &s); - for (i = 0; i < list.cur; i++) { - CMSetArrayElementAt(array, - i, - (CMPIValue *)&list.list[i], - CMPI_instance); - } - - CMSetProperty(inst, "HostResource", - (CMPIValue *)&array, CMPI_instanceA); - } - - out: - inst_list_free(&list); - virDomainFree(dom); - virConnectClose(conn); - - return s; } static CMPIInstance *rasd_from_vdev(const CMPIBroker *broker, @@ -354,8 +168,6 @@ static CMPIInstance *rasd_from_vdev(cons (CMPIValue *)&dev->dev.mem.size, CMPI_uint64); CMSetProperty(inst, "Limit", (CMPIValue *)&dev->dev.mem.maxsize, CMPI_uint64); - } else if (dev->type == CIM_RES_TYPE_PROC) { - proc_rasd_from_vdev(broker, dev, host, ref, inst); } /* FIXME: Put the HostResource in place */
# HG changeset patch # User Jay Gagnon <grendel@linux.vnet.ibm.com> # Date 1207668726 14400 # Node ID ddbd91b1f796acc59db18997ded9faf8ebf14006 # Parent ade926a6ad096be7c1bebab3eb7ac4080aac9088 Change how processors are represented using RASD. We currently do one RASD per processor per domain, but it looks like that isn't going to work very well with how we do scheduling, so this switches the "read" representation over to one RASD per domain, with VirtualQuantity set appropriately. Signed-off-by: Jay Gagnon <grendel@linux.vnet.ibm.com> diff -r ade926a6ad09 -r ddbd91b1f796 libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c Mon Apr 07 14:31:06 2008 -0400 +++ b/libxkutil/device_parsing.c Tue Apr 08 11:32:06 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 ade926a6ad09 -r ddbd91b1f796 libxkutil/device_parsing.h --- a/libxkutil/device_parsing.h Mon Apr 07 14:31:06 2008 -0400 +++ b/libxkutil/device_parsing.h Tue Apr 08 11:32:06 2008 -0400 @@ -50,7 +50,7 @@ struct mem_device { }; struct vcpu_device { - uint32_t number; + uint32_t quantity; }; struct emu_device { diff -r ade926a6ad09 -r ddbd91b1f796 src/Virt_RASD.c --- a/src/Virt_RASD.c Mon Apr 07 14:31:06 2008 -0400 +++ b/src/Virt_RASD.c Tue Apr 08 11:32:06 2008 -0400 @@ -168,6 +168,9 @@ static CMPIInstance *rasd_from_vdev(cons (CMPIValue *)&dev->dev.mem.size, CMPI_uint64); CMSetProperty(inst, "Limit", (CMPIValue *)&dev->dev.mem.maxsize, CMPI_uint64); + } else if (dev->type == CIM_RES_TYPE_PROC) { + CMSetProperty(inst, "VirtualQuantity", + (CMPIValue *)&dev->dev.vcpu.quantity, CMPI_uint32); } /* FIXME: Put the HostResource in place */
JG> Just want to get this out before I start in on the resource JG> add/mod/del stuff, to see if I'm on the right track. The idea JG> here is that we are not supporting processor pinning, will support JG> scheduling (weight/limit attributes), and are making Processor JG> RASDs one-per-domain. Does this look sane? I haven't tested it yet (I can tomorrow), but this looks sane to me. Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Jay Gagnon wrote:
Just want to get this out before I start in on the resource add/mod/del stuff, to see if I'm on the right track. The idea here is that we are not supporting processor pinning, will support scheduling (weight/limit attributes), and are making Processor RASDs one-per-domain. Does this look sane?
This looks good to me. +1 Some of the Processor and EAFP tests fail due to these changes, but this is because the tests are expecting the InstanceID for the device to be <guest name>/<processor id> instead of <guest name>/proc. The commit log for the second patch is a bit misleading, since you're changing device_parsing.c which also impacts Virt_Device. But otherwise, this tests fine for me. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
participants (3)
-
Dan Smith -
Jay Gagnon -
Kaitlin Rupert