[REWORK 0/7] Rework the controller patch series
I took the "liberty" of making some adjustments based on the base series. I have left the base code "as is" in patches 1, 3, and 6. If I simply apply 1 & 2 and more specifically - if I modify patch 1 to just add the "break;"'s in 'do_parse()' and 'device_to_xml()', then cimtest has "new" failures, such as: -------------------------------------------------------------------- ComputerSystem - 41_cs_to_settingdefinestate.py: FAIL ERROR - Exception details: KVM_SystemDevice returned 0 Logical Devices objects ERROR - Exception details is Failed to get SystemDevice information CIM_ERR_FAILED: Couldn't get device instances -------------------------------------------------------------------- Memory - 02_defgetmem.py: FAIL ERROR - No devices returned ERROR - Exception: 'int' object is not iterable CIM_ERR_FAILED: Couldn't get device instances -------------------------------------------------------------------- Processor - 02_definesys_get_procs.py: FAIL ERROR - No devices returned ERROR - Exception: 'int' object is not iterable CIM_ERR_FAILED: Couldn't get device instances -------------------------------------------------------------------- RASD - 03_rasd_errs.py: FAIL ERROR - Expected 6 RASDs, got 7 -------------------------------------------------------------------- ResourceAllocationFromPool - 01_forward.py: FAIL ERROR - 7 RASD insts != 6 pool insts -------------------------------------------------------------------- ResourceAllocationFromPool - 02_reverse.py: FAIL ERROR - 7 RASD insts != 6 pool insts -------------------------------------------------------------------- SettingsDefine - 01_forward.py: FAIL ERROR - 6 device insts != 7 RASD insts -------------------------------------------------------------------- SettingsDefine - 02_reverse.py: FAIL ERROR - Got 9 RASDs, expected 7 ERROR - Failed to verify RASDs -------------------------------------------------------------------- VirtualSystemManagementService - 16_removeresource.py: FAIL ERROR - No devices returned ERROR - UnboundLocalError : local variable 'ccn' referenced before assignment Traceback (most recent call last): File "/home/jferlan/git/cimtest.work/suites/libvirt-cim/lib/XenKvmLib/const.py", line 141, in do_try rc = f() File "16_removeresource.py", line 95, in main if ccn == input: UnboundLocalError: local variable 'ccn' referenced before assignment ERROR - None CIM_ERR_FAILED: Couldn't get device instances -------------------------------------------------------------------- VirtualSystemManagementService - 31_unset_netrasd.py: FAIL ERROR - CIMError : (1, u"CIM_ERR_FAILED: Couldn't get device instances") Traceback (most recent call last): ... assoc_names = cim.AssociatorNames(inst_name, AssocClass=a_class, ResultClass=r_class) ... CIMError: (1, u"CIM_ERR_FAILED: Couldn't get device instances") ERROR - None -------------------------------------------------------------------- VirtualSystemManagementService - 32_modify_cdrom_media.py: FAIL ERROR - CIMError : (1, u"CIM_ERR_FAILED: Couldn't get device instances") Traceback (most recent call last): ... assoc_names = cim.AssociatorNames(inst_name, AssocClass=a_class, ResultClass=r_class) ... CIMError: (1, u"CIM_ERR_FAILED: Couldn't get device instances") ERROR - None -------------------------------------------------------------------- I haven't dug in too deeply but there's some disconnect between the new 'controller' device and other parts. It took a bit of time to just get things where there are now - I can continue with this, but I wanted to "share" in hopes that someone else can also look and give me some hints. I'm hoping it's something simple and not that I have to have something else - although I'm beginning to think I need a logical device... I have not completed the 3 of 3 changes to manage all the new fields. I figured I could add those eventually, but I did want to make sure I was setting up the instance id correctly as described in the code reviews thus far. John Ferlan (4): Adjustments to patch 1/3 Rename set_rasd_device_address Changes to 2/3 to support more fields Changes to 3 of 3 from code review Xu Wang (3): libxutil: Controller Support RASD: Schema and Provider Support for Controller RASDs VSMS: Support for domains with controller devices libxkutil/device_parsing.c | 99 ++++++++++++++++++++++- libxkutil/device_parsing.h | 15 ++++ libxkutil/xmlgen.c | 47 +++++++++++ schema/ResourceAllocationSettingData.mof | 45 +++++++++++ schema/ResourceAllocationSettingData.registration | 1 + src/Virt_ElementSettingData.c | 1 + src/Virt_RASD.c | 81 +++++++++++++++++-- src/Virt_SettingsDefineState.c | 1 + src/Virt_VSSDComponent.c | 1 + src/Virt_VirtualSystemManagementService.c | 70 ++++++++++++++++ src/svpc_types.h | 4 +- 11 files changed, 355 insertions(+), 10 deletions(-) -- 1.8.5.3
From: Xu Wang <gesaint@linux.vnet.ibm.com> Signed-off-by: Xu Wang <gesaint@linux.vnet.ibm.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- libxkutil/device_parsing.c | 62 +++++++++++++++++++++++++++++++++++++++++++++- libxkutil/device_parsing.h | 9 +++++++ libxkutil/xmlgen.c | 28 +++++++++++++++++++++ src/svpc_types.h | 4 ++- 4 files changed, 101 insertions(+), 2 deletions(-) diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c index c9ae886..12c52dc 100644 --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -1,5 +1,5 @@ /* - * Copyright IBM Corp. 2007, 2013 + * Copyright IBM Corp. 2007, 2014 * * Authors: * Dan Smith <danms@us.ibm.com> @@ -49,6 +49,7 @@ #define GRAPHICS_XPATH (xmlChar *)"/domain/devices/graphics | "\ "/domain/devices/console" #define INPUT_XPATH (xmlChar *)"/domain/devices/input" +#define CONTROLLER_XPATH (xmlChar *)"/domain/devices/controller" #define DEFAULT_BRIDGE "xenbr0" #define DEFAULT_NETWORK "default" @@ -306,6 +307,15 @@ static void cleanup_input_device(struct input_device *dev) free(dev->bus); } +static void cleanup_controller_device(struct controller_device *dev) +{ + if (dev == NULL) + return; + + free(dev->type); + free(dev->model); +} + void cleanup_virt_device(struct virt_device *dev) { if (dev == NULL) @@ -323,6 +333,8 @@ void cleanup_virt_device(struct virt_device *dev) cleanup_input_device(&dev->dev.input); else if (dev->type == CIM_RES_TYPE_CONSOLE) cleanup_console_device(&dev->dev.console); + else if (dev->type == CIM_RES_TYPE_CONTROLLER) + cleanup_controller_device(&dev->dev.controller); free(dev->id); @@ -1101,6 +1113,42 @@ static int parse_input_device(xmlNode *node, struct virt_device **vdevs) return 0; } +static int parse_controller_device(xmlNode *node, struct virt_device **vdevs) +{ + struct virt_device *vdev = NULL; + struct controller_device *cdev = NULL; + int ret; + + vdev = calloc(1, sizeof(*vdev)); + if (vdev == NULL) + goto err; + + cdev = &(vdev->dev.controller); + + cdev->type = get_attr_value(node, "type"); + cdev->model = get_attr_value(node, "model"); + + if (cdev->type == NULL) + goto err; + + vdev->type = CIM_RES_TYPE_CONTROLLER; + + ret = asprintf(&vdev->id, "%s", cdev->type); + if (ret == -1) { + CU_DEBUG("Failed to create controller id string"); + goto err; + } + + *vdevs = vdev; + + return 1; + err: + cleanup_controller_device(cdev); + free(vdev); + + return 0; +} + static bool resize_devlist(struct virt_device **list, int newsize) { struct virt_device *_list; @@ -1224,6 +1272,10 @@ static int parse_devices(const char *xml, struct virt_device **_list, int type) func = &parse_input_device; break; + case CIM_RES_TYPE_CONTROLLER: + xpathstr = CONTROLLER_XPATH; + func = &parse_controller_device; + default: CU_DEBUG("Unrecognized device type. Returning."); goto err1; @@ -1343,7 +1395,11 @@ struct virt_device *virt_device_dup(struct virt_device *_dev) } else if (dev->type == CIM_RES_TYPE_CONSOLE) { console_device_dup(&dev->dev.console, &_dev->dev.console); + } else if (dev->type == CIM_RES_TYPE_CONTROLLER) { + DUP_FIELD(dev, _dev, dev.controller.type); + DUP_FIELD(dev, _dev, dev.controller.model); } + return dev; } @@ -1723,6 +1779,9 @@ int get_dominfo_from_xml(const char *xml, struct domain **dominfo) (*dominfo)->dev_vcpu_ct = parse_devices(xml, &(*dominfo)->dev_vcpu, CIM_RES_TYPE_PROC); + (*dominfo)->dev_controller_ct = parse_devices(xml, + &(*dominfo)->dev_controller, + CIM_RES_TYPE_CONTROLLER); return ret; @@ -1811,6 +1870,7 @@ void cleanup_dominfo(struct domain **dominfo) cleanup_virt_devices(&dom->dev_graphics, dom->dev_graphics_ct); cleanup_virt_devices(&dom->dev_input, dom->dev_input_ct); cleanup_virt_devices(&dom->dev_console, dom->dev_console_ct); + cleanup_virt_devices(&dom->dev_controller, dom->dev_controller_ct); free(dom); diff --git a/libxkutil/device_parsing.h b/libxkutil/device_parsing.h index 92427c1..556b9f2 100644 --- a/libxkutil/device_parsing.h +++ b/libxkutil/device_parsing.h @@ -161,6 +161,11 @@ struct input_device { char *bus; }; +struct controller_device { + char *type; + char *model; +}; + struct virt_device { uint16_t type; union { @@ -172,6 +177,7 @@ struct virt_device { struct graphics_device graphics; struct console_device console; struct input_device input; + struct controller_device controller; } dev; char *id; }; @@ -247,6 +253,9 @@ struct domain { struct virt_device *dev_vcpu; int dev_vcpu_ct; + + struct virt_device *dev_controller; + int dev_controller_ct; }; struct virt_device *virt_device_dup(struct virt_device *dev); diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c index 931f0c9..96e1c28 100644 --- a/libxkutil/xmlgen.c +++ b/libxkutil/xmlgen.c @@ -794,6 +794,29 @@ static const char *input_xml(xmlNodePtr root, struct domain *dominfo) return NULL; } +static const char *controller_xml(xmlNodePtr root, struct domain *dominfo) +{ + int i; + + for (i = 0; i < dominfo->dev_controller_ct; i++) { + xmlNodePtr tmp; + struct virt_device *_dev = &dominfo->dev_controller[i]; + if (_dev->type == CIM_RES_TYPE_UNKNOWN) + continue; + + struct controller_device *dev = &_dev->dev.controller; + + tmp = xmlNewChild(root, NULL, BAD_CAST "controller", NULL); + if (tmp == NULL) + return XML_ERROR; + + xmlNewProp(tmp, BAD_CAST "type", BAD_CAST dev->type); + xmlNewProp(tmp, BAD_CAST "model", BAD_CAST dev->model); + } + + return NULL; +} + static char *system_xml(xmlNodePtr root, struct domain *domain) { xmlNodePtr tmp; @@ -1125,6 +1148,10 @@ char *device_to_xml(struct virt_device *_dev) dominfo->dev_input_ct = 1; dominfo->dev_input = dev; break; + case CIM_RES_TYPE_CONTROLLER: + func = controller_xml; + dominfo->dev_controller_ct = 1; + dominfo->dev_controller = dev; default: cleanup_virt_devices(&dev, 1); goto out; @@ -1163,6 +1190,7 @@ char *system_to_xml(struct domain *dominfo) &console_xml, &graphics_xml, &emu_xml, + &controller_xml, NULL }; diff --git a/src/svpc_types.h b/src/svpc_types.h index 404e428..d76097c 100644 --- a/src/svpc_types.h +++ b/src/svpc_types.h @@ -32,12 +32,13 @@ #define CIM_RES_TYPE_DISK 17 #define CIM_RES_TYPE_GRAPHICS 24 #define CIM_RES_TYPE_INPUT 13 +#define CIM_RES_TYPE_CONTROLLER 33 #define CIM_RES_TYPE_UNKNOWN 1000 #define CIM_RES_TYPE_IMAGE 32768 #define CIM_RES_TYPE_CONSOLE 32769 #define CIM_RES_TYPE_EMU 32770 -#define CIM_RES_TYPE_COUNT 7 +#define CIM_RES_TYPE_COUNT 8 const static int cim_res_types[CIM_RES_TYPE_COUNT] = {CIM_RES_TYPE_NET, CIM_RES_TYPE_DISK, @@ -46,6 +47,7 @@ const static int cim_res_types[CIM_RES_TYPE_COUNT] = CIM_RES_TYPE_GRAPHICS, CIM_RES_TYPE_INPUT, CIM_RES_TYPE_CONSOLE, + CIM_RES_TYPE_CONTROLLER, }; #define CIM_VSSD_RECOVERY_NONE 2 -- 1.8.5.3
Based on review comments - I've made a few adjustments to the initial patch. This change would be merged with 1/3 once we ensure things work as expected. Differences to 1/3: 1. Add "break;" in appropriate spots as found by Coverity 2. Add controller fields for : index - Required property of <controller> ports - Optional property of <controller> vectors - Optional property of <controller> queues - Optional property of optional child <driver> of <controller> master - Optional child of <controller> with paired list of properties address - Optional child of <controller> with paired list of properties 3. Adjust the 'cdev->id' to be "controller:" + "type" + ":" + "index" where type, index is sourced from the <controller type='string' index='#'> 4. Change the CIM_RES_TYPE_CONTROLLER to 32771 Signed-off-by: John Ferlan <jferlan@redhat.com> --- libxkutil/device_parsing.c | 51 +++++++++++++++++++++++++++++++++++++++------- libxkutil/device_parsing.h | 6 ++++++ libxkutil/xmlgen.c | 27 ++++++++++++++++++++---- src/svpc_types.h | 2 +- 4 files changed, 74 insertions(+), 12 deletions(-) diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c index 12c52dc..41d75b8 100644 --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -313,7 +313,13 @@ static void cleanup_controller_device(struct controller_device *dev) return; free(dev->type); + free(dev->index); free(dev->model); + free(dev->queues); + free(dev->ports); + free(dev->vectors); + cleanup_device_address(&dev->address); + cleanup_device_address(&dev->master); } void cleanup_virt_device(struct virt_device *dev) @@ -1113,10 +1119,11 @@ static int parse_input_device(xmlNode *node, struct virt_device **vdevs) return 0; } -static int parse_controller_device(xmlNode *node, struct virt_device **vdevs) +static int parse_controller_device(xmlNode *cnode, struct virt_device **vdevs) { struct virt_device *vdev = NULL; struct controller_device *cdev = NULL; + xmlNode *child = NULL; int ret; vdev = calloc(1, sizeof(*vdev)); @@ -1125,15 +1132,36 @@ static int parse_controller_device(xmlNode *node, struct virt_device **vdevs) cdev = &(vdev->dev.controller); - cdev->type = get_attr_value(node, "type"); - cdev->model = get_attr_value(node, "model"); - - if (cdev->type == NULL) + cdev->type = get_attr_value(cnode, "type"); + if (cdev->type == NULL) { + CU_DEBUG("No type"); goto err; - + } + cdev->index = get_attr_value(cnode, "index"); + if (cdev->index == NULL) { + CU_DEBUG("No index"); + goto err; + } + cdev->model = get_attr_value(cnode, "model"); + cdev->ports = get_attr_value(cnode, "ports"); + cdev->vectors = get_attr_value(cnode, "vectors"); + + for (child = cnode->children; child != NULL; child = child->next) { + if (XSTREQ(child->name, "address")) { + parse_device_address(child, &cdev->address); + } else if (XSTREQ(child->name, "master")) { + /* Although technically not an address it is similar + * insomuch as it's a paired list of attributes that + * we're just going to save and write out later + */ + parse_device_address(child, &cdev->master); + } else if (XSTREQ(child->name, "driver")) { + cdev->queues = get_attr_value(child, "queues"); + } + } vdev->type = CIM_RES_TYPE_CONTROLLER; - ret = asprintf(&vdev->id, "%s", cdev->type); + ret = asprintf(&vdev->id, "controller:%s:%s", cdev->type, cdev->index); if (ret == -1) { CU_DEBUG("Failed to create controller id string"); goto err; @@ -1275,6 +1303,7 @@ static int parse_devices(const char *xml, struct virt_device **_list, int type) case CIM_RES_TYPE_CONTROLLER: xpathstr = CONTROLLER_XPATH; func = &parse_controller_device; + break; default: CU_DEBUG("Unrecognized device type. Returning."); @@ -1397,7 +1426,15 @@ struct virt_device *virt_device_dup(struct virt_device *_dev) &_dev->dev.console); } else if (dev->type == CIM_RES_TYPE_CONTROLLER) { DUP_FIELD(dev, _dev, dev.controller.type); + DUP_FIELD(dev, _dev, dev.controller.index); DUP_FIELD(dev, _dev, dev.controller.model); + DUP_FIELD(dev, _dev, dev.controller.ports); + DUP_FIELD(dev, _dev, dev.controller.vectors); + DUP_FIELD(dev, _dev, dev.controller.queues); + duplicate_device_address(&dev->dev.controller.master, + &_dev->dev.controller.master); + duplicate_device_address(&dev->dev.controller.address, + &_dev->dev.controller.address); } return dev; diff --git a/libxkutil/device_parsing.h b/libxkutil/device_parsing.h index 556b9f2..e3d5616 100644 --- a/libxkutil/device_parsing.h +++ b/libxkutil/device_parsing.h @@ -163,7 +163,13 @@ struct input_device { struct controller_device { char *type; + char *index; char *model; + char *ports; + char *vectors; + char *queues; + struct device_address address; + struct device_address master; }; struct virt_device { diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c index 96e1c28..2326b1f 100644 --- a/libxkutil/xmlgen.c +++ b/libxkutil/xmlgen.c @@ -799,6 +799,7 @@ static const char *controller_xml(xmlNodePtr root, struct domain *dominfo) int i; for (i = 0; i < dominfo->dev_controller_ct; i++) { + xmlNodePtr ctlr; xmlNodePtr tmp; struct virt_device *_dev = &dominfo->dev_controller[i]; if (_dev->type == CIM_RES_TYPE_UNKNOWN) @@ -806,12 +807,29 @@ static const char *controller_xml(xmlNodePtr root, struct domain *dominfo) struct controller_device *dev = &_dev->dev.controller; - tmp = xmlNewChild(root, NULL, BAD_CAST "controller", NULL); - if (tmp == NULL) + ctlr = xmlNewChild(root, NULL, BAD_CAST "controller", NULL); + if (ctlr == NULL) return XML_ERROR; - xmlNewProp(tmp, BAD_CAST "type", BAD_CAST dev->type); - xmlNewProp(tmp, BAD_CAST "model", BAD_CAST dev->model); + /* Required */ + xmlNewProp(ctlr, BAD_CAST "type", BAD_CAST dev->type); + xmlNewProp(ctlr, BAD_CAST "index", BAD_CAST dev->index); + + /* Optional */ + if (dev->model) + xmlNewProp(ctlr, BAD_CAST "model", BAD_CAST dev->model); + if (dev->ports) + xmlNewProp(ctlr, BAD_CAST "ports", BAD_CAST dev->ports); + if (dev->vectors) + xmlNewProp(ctlr, BAD_CAST "vectors", BAD_CAST dev->vectors); + if (dev->queues) { + tmp = xmlNewChild(ctlr, NULL, BAD_CAST "driver", NULL); + xmlNewProp(tmp, BAD_CAST "queueus", BAD_CAST dev->queues); + } + if (dev->master.ct > 0) + return device_address_xml(ctlr, &dev->master); + if (dev->address.ct > 0) + return device_address_xml(ctlr, &dev->address); } return NULL; @@ -1152,6 +1170,7 @@ char *device_to_xml(struct virt_device *_dev) func = controller_xml; dominfo->dev_controller_ct = 1; dominfo->dev_controller = dev; + break; default: cleanup_virt_devices(&dev, 1); goto out; diff --git a/src/svpc_types.h b/src/svpc_types.h index d76097c..7a2b653 100644 --- a/src/svpc_types.h +++ b/src/svpc_types.h @@ -32,11 +32,11 @@ #define CIM_RES_TYPE_DISK 17 #define CIM_RES_TYPE_GRAPHICS 24 #define CIM_RES_TYPE_INPUT 13 -#define CIM_RES_TYPE_CONTROLLER 33 #define CIM_RES_TYPE_UNKNOWN 1000 #define CIM_RES_TYPE_IMAGE 32768 #define CIM_RES_TYPE_CONSOLE 32769 #define CIM_RES_TYPE_EMU 32770 +#define CIM_RES_TYPE_CONTROLLER 32771 #define CIM_RES_TYPE_COUNT 8 const static int cim_res_types[CIM_RES_TYPE_COUNT] = -- 1.8.5.3
On 03/13/2014 11:27 PM, John Ferlan wrote:
Based on review comments - I've made a few adjustments to the initial patch. This change would be merged with 1/3 once we ensure things work as expected.
Differences to 1/3:
1. Add "break;" in appropriate spots as found by Coverity 2. Add controller fields for : index - Required property of <controller> ports - Optional property of <controller> vectors - Optional property of <controller> queues - Optional property of optional child <driver> of <controller> master - Optional child of <controller> with paired list of properties address - Optional child of <controller> with paired list of properties 3. Adjust the 'cdev->id' to be "controller:" + "type" + ":" + "index" where type, index is sourced from the <controller type='string' index='#'> 4. Change the CIM_RES_TYPE_CONTROLLER to 32771
Signed-off-by: John Ferlan <jferlan@redhat.com> --- libxkutil/device_parsing.c | 51 +++++++++++++++++++++++++++++++++++++++------- libxkutil/device_parsing.h | 6 ++++++ libxkutil/xmlgen.c | 27 ++++++++++++++++++++---- src/svpc_types.h | 2 +- 4 files changed, 74 insertions(+), 12 deletions(-)
diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c index 12c52dc..41d75b8 100644 --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -313,7 +313,13 @@ static void cleanup_controller_device(struct controller_device *dev) return;
free(dev->type); + free(dev->index); not needed if of type uint64_t free(dev->model); + free(dev->queues); + free(dev->ports); + free(dev->vectors); + cleanup_device_address(&dev->address); + cleanup_device_address(&dev->master); }
void cleanup_virt_device(struct virt_device *dev) @@ -1113,10 +1119,11 @@ static int parse_input_device(xmlNode *node, struct virt_device **vdevs) return 0; }
-static int parse_controller_device(xmlNode *node, struct virt_device **vdevs) +static int parse_controller_device(xmlNode *cnode, struct virt_device **vdevs) { struct virt_device *vdev = NULL; struct controller_device *cdev = NULL; + xmlNode *child = NULL; char *index = NULL;
int ret;
vdev = calloc(1, sizeof(*vdev)); @@ -1125,15 +1132,36 @@ static int parse_controller_device(xmlNode *node, struct virt_device **vdevs)
cdev = &(vdev->dev.controller);
- cdev->type = get_attr_value(node, "type"); - cdev->model = get_attr_value(node, "model"); - - if (cdev->type == NULL) + cdev->type = get_attr_value(cnode, "type"); + if (cdev->type == NULL) { + CU_DEBUG("No type"); goto err; - + } + cdev->index = get_attr_value(cnode, "index"); + if (cdev->index == NULL) { + CU_DEBUG("No index"); + goto err; + }
index = get_attr_value(cnode, "index"); if (index != NULL) { sscanf(index, "%" PRIu64, &cdev->index); free(index); } else { CU_DEBUG("No index"); goto err; }
+ cdev->model = get_attr_value(cnode, "model"); + cdev->ports = get_attr_value(cnode, "ports"); + cdev->vectors = get_attr_value(cnode, "vectors"); + + for (child = cnode->children; child != NULL; child = child->next) { + if (XSTREQ(child->name, "address")) { + parse_device_address(child, &cdev->address); + } else if (XSTREQ(child->name, "master")) { + /* Although technically not an address it is similar + * insomuch as it's a paired list of attributes that + * we're just going to save and write out later + */ + parse_device_address(child, &cdev->master); + } else if (XSTREQ(child->name, "driver")) { + cdev->queues = get_attr_value(child, "queues"); + } + } vdev->type = CIM_RES_TYPE_CONTROLLER;
- ret = asprintf(&vdev->id, "%s", cdev->type); + ret = asprintf(&vdev->id, "controller:%s:%s", cdev->type, cdev->index); if (ret == -1) { CU_DEBUG("Failed to create controller id string"); goto err; @@ -1275,6 +1303,7 @@ static int parse_devices(const char *xml, struct virt_device **_list, int type) case CIM_RES_TYPE_CONTROLLER: xpathstr = CONTROLLER_XPATH; func = &parse_controller_device; + break;
default: CU_DEBUG("Unrecognized device type. Returning."); @@ -1397,7 +1426,15 @@ struct virt_device *virt_device_dup(struct virt_device *_dev) &_dev->dev.console); } else if (dev->type == CIM_RES_TYPE_CONTROLLER) { DUP_FIELD(dev, _dev, dev.controller.type); + DUP_FIELD(dev, _dev, dev.controller.index); dev->dev.controller.index = _dev->dev.controller.index;
DUP_FIELD(dev, _dev, dev.controller.model); + DUP_FIELD(dev, _dev, dev.controller.ports); + DUP_FIELD(dev, _dev, dev.controller.vectors); + DUP_FIELD(dev, _dev, dev.controller.queues); + duplicate_device_address(&dev->dev.controller.master, + &_dev->dev.controller.master); + duplicate_device_address(&dev->dev.controller.address, + &_dev->dev.controller.address); }
return dev; diff --git a/libxkutil/device_parsing.h b/libxkutil/device_parsing.h index 556b9f2..e3d5616 100644 --- a/libxkutil/device_parsing.h +++ b/libxkutil/device_parsing.h @@ -163,7 +163,13 @@ struct input_device {
#define CONTROLLER_INDEX_NOT_SET -1
struct controller_device { char *type; + char *index;
this is an uint64 in the mof and I would suggest to at least for the index use uint64_t since it is good to have a number when things like hostdev are to be support in the future. The numbers for ports, vectors and queues are just handed from RASDs into xml and vice versa.
char *model; + char *ports; + char *vectors; + char *queues; + struct device_address address; + struct device_address master; };
struct virt_device { diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c index 96e1c28..2326b1f 100644 --- a/libxkutil/xmlgen.c +++ b/libxkutil/xmlgen.c @@ -799,6 +799,7 @@ static const char *controller_xml(xmlNodePtr root, struct domain *dominfo) int i;
for (i = 0; i < dominfo->dev_controller_ct; i++) { + xmlNodePtr ctlr; xmlNodePtr tmp; struct virt_device *_dev = &dominfo->dev_controller[i]; if (_dev->type == CIM_RES_TYPE_UNKNOWN) @@ -806,12 +807,29 @@ static const char *controller_xml(xmlNodePtr root, struct domain *dominfo)
struct controller_device *dev = &_dev->dev.controller;
- tmp = xmlNewChild(root, NULL, BAD_CAST "controller", NULL); - if (tmp == NULL) + ctlr = xmlNewChild(root, NULL, BAD_CAST "controller", NULL); + if (ctlr == NULL) return XML_ERROR;
- xmlNewProp(tmp, BAD_CAST "type", BAD_CAST dev->type); - xmlNewProp(tmp, BAD_CAST "model", BAD_CAST dev->model); + /* Required */ + xmlNewProp(ctlr, BAD_CAST "type", BAD_CAST dev->type); + xmlNewProp(ctlr, BAD_CAST "index", BAD_CAST dev->index);
if (cdev->index != CONTROLLER_INDEX_NOT_SET) { char *string = NULL; if (asprintf(&string, "%" PRIu64, cdev->index) == -1) return XML_ERROR; xmlNewProp(ctlr, BAD_CAST "index", BAD_CAST string); free(string); }
+ + /* Optional */ + if (dev->model) + xmlNewProp(ctlr, BAD_CAST "model", BAD_CAST dev->model); + if (dev->ports) + xmlNewProp(ctlr, BAD_CAST "ports", BAD_CAST dev->ports); + if (dev->vectors) + xmlNewProp(ctlr, BAD_CAST "vectors", BAD_CAST dev->vectors); + if (dev->queues) { + tmp = xmlNewChild(ctlr, NULL, BAD_CAST "driver", NULL); + xmlNewProp(tmp, BAD_CAST "queueus", BAD_CAST dev->queues); + } + if (dev->master.ct > 0) + return device_address_xml(ctlr, &dev->master); + if (dev->address.ct > 0) + return device_address_xml(ctlr, &dev->address); }
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
From: Xu Wang <gesaint@linux.vnet.ibm.com> Signed-off-by: Xu Wang <gesaint@linux.vnet.ibm.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- schema/ResourceAllocationSettingData.mof | 27 +++++++++++++++++++++++ schema/ResourceAllocationSettingData.registration | 3 +++ src/Virt_RASD.c | 24 ++++++++++++++++++++ 3 files changed, 54 insertions(+) diff --git a/schema/ResourceAllocationSettingData.mof b/schema/ResourceAllocationSettingData.mof index bf1fbb6..5c76a1c 100644 --- a/schema/ResourceAllocationSettingData.mof +++ b/schema/ResourceAllocationSettingData.mof @@ -322,6 +322,33 @@ class LXC_InputResourceAllocationSettingData : LXC_ResourceAllocationSettingData string BusType; }; +[Description ("Xen virtual controller device"), + Provider("cmpi::Virt_RASD") +] +class Xen_ControllerResourceAllocationSettingData : Xen_ResourceAllocationSettingData +{ + string Type; + string Model; +}; + +[Description ("KVM virtual controller device"), + Provider("cmpi::Virt_RASD") +] +class KVM_ControllerResourceAllocationSettingData : KVM_ResourceAllocationSettingData +{ + string Type; + string Model; +}; + +[Description ("LXC virtual controller device"), + Provider("cmpi::Virt_RASD") +] +class LXC_ControllerResourceAllocationSettingData : LXC_ResourceAllocationSettingData +{ + string Type; + string Model; +}; + [Description ("Xen virtual network pool settings"), Provider("cmpi::Virt_RASD") ] diff --git a/schema/ResourceAllocationSettingData.registration b/schema/ResourceAllocationSettingData.registration index b969bfe..1142376 100644 --- a/schema/ResourceAllocationSettingData.registration +++ b/schema/ResourceAllocationSettingData.registration @@ -7,6 +7,7 @@ Xen_MemResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance Xen_GraphicsResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance Xen_InputResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance Xen_ConsoleResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance +Xen_ControllerResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance KVM_DiskResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance KVM_NetResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance KVM_ProcResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance @@ -14,9 +15,11 @@ KVM_MemResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance KVM_GraphicsResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance KVM_InputResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance KVM_ConsoleResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance +KVM_ControllerResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance LXC_MemResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance LXC_DiskResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance LXC_ProcResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance LXC_GraphicsResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance LXC_InputResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance LXC_ConsoleResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance +LXC_ControllerResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance diff --git a/src/Virt_RASD.c b/src/Virt_RASD.c index df1e921..f75027e 100644 --- a/src/Virt_RASD.c +++ b/src/Virt_RASD.c @@ -903,6 +903,20 @@ static CMPIStatus set_input_rasd_params(const struct virt_device *dev, return s; } +static CMPIStatus set_controller_rasd_params(const struct virt_device *dev, + CMPIInstance *inst) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + + CMSetProperty(inst, "Type", + (CMPIValue *)dev->dev.controller.type, CMPI_chars); + + CMSetProperty(inst, "Model", + (CMPIValue *)dev->dev.controller.model, CMPI_chars); + + return s; +} + CMPIInstance *rasd_from_vdev(const CMPIBroker *broker, struct virt_device *dev, const char *host, @@ -937,6 +951,9 @@ CMPIInstance *rasd_from_vdev(const CMPIBroker *broker, } else if (dev->type == CIM_RES_TYPE_INPUT) { type = CIM_RES_TYPE_INPUT; base = "InputResourceAllocationSettingData"; + } else if (dev->type == CIM_RES_TYPE_CONTROLLER) { + type = CIM_RES_TYPE_CONTROLLER; + base = "ControllerResourceAllocationSettingData"; } else { return NULL; } @@ -992,6 +1009,8 @@ CMPIInstance *rasd_from_vdev(const CMPIBroker *broker, s = set_input_rasd_params(dev, inst); } else if (dev->type == CIM_RES_TYPE_CONSOLE) { s = set_console_rasd_params(dev, inst); + } else if (dev->type == CIM_RES_TYPE_CONTROLLER) { + s = set_controller_rasd_params(dev, inst); } /* FIXME: Put the HostResource in place */ @@ -1126,6 +1145,8 @@ CMPIrc res_type_from_rasd_classname(const char *cn, uint16_t *type) *type = CIM_RES_TYPE_IMAGE; else if (STREQ(base, "ConsoleResourceAllocationSettingData")) *type = CIM_RES_TYPE_CONSOLE; + else if (STREQ(base, "ControllerResourceAllocationSettingData")) + *type = CIM_RES_TYPE_CONTROLLER; else goto out; @@ -1163,6 +1184,9 @@ CMPIrc rasd_classname_from_type(uint16_t type, const char **classname) case CIM_RES_TYPE_INPUT: *classname = "InputResourceAllocationSettingData"; break; + case CIM_RES_TYPE_CONTROLLER: + *classname = "ControllerResourceAllocationSettingData"; + break; default: rc = CMPI_RC_ERR_FAILED; } -- 1.8.5.3
It's about to become multi-purpose and having AddressProperties and AddressValues static is problematic. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/Virt_RASD.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/Virt_RASD.c b/src/Virt_RASD.c index f75027e..249e6e5 100644 --- a/src/Virt_RASD.c +++ b/src/Virt_RASD.c @@ -289,10 +289,12 @@ static bool get_vol_size(const CMPIBroker *broker, } #endif -static CMPIStatus set_rasd_device_address(const CMPIBroker *broker, +static CMPIStatus set_rasd_property_value(const CMPIBroker *broker, const CMPIObjectPath *ref, const struct device_address *addr, - CMPIInstance *inst) + CMPIInstance *inst, + const char *property_name, + const char *property_value) { int i; CMPIArray *arr_key; @@ -338,11 +340,11 @@ static CMPIStatus set_rasd_device_address(const CMPIBroker *broker, CMPI_string); } - CMSetProperty(inst, "AddressProperties", + CMSetProperty(inst, property_name, (CMPIValue *)&arr_key, CMPI_stringA); - CMSetProperty(inst, "AddressValues", + CMSetProperty(inst, property_value, (CMPIValue *)&arr_value, CMPI_stringA); @@ -489,10 +491,12 @@ static CMPIStatus set_disk_rasd_params(const CMPIBroker *broker, CMPI_boolean); if (dev->dev.disk.address.ct > 0) - set_rasd_device_address(broker, + set_rasd_property_value(broker, ref, &dev->dev.disk.address, - inst); + inst, + "AddressProperties", + "AddressValues"); virStoragePoolFree(pool); virStorageVolFree(vol); @@ -658,10 +662,12 @@ static CMPIStatus set_net_rasd_params(const CMPIBroker *broker, CMPI_chars); if (dev->dev.net.address.ct > 0) - set_rasd_device_address(broker, + set_rasd_property_value(broker, ref, &dev->dev.net.address, - inst); + inst, + "AddressProperties", + "AddressValues"); #if LIBVIR_VERSION_NUMBER < 9000 out: -- 1.8.5.3
Signed-off-by: John Ferlan <jferlan@redhat.com> --- schema/ResourceAllocationSettingData.mof | 48 +++++++++++++++------- schema/ResourceAllocationSettingData.registration | 2 - src/Virt_ElementSettingData.c | 1 + src/Virt_RASD.c | 49 +++++++++++++++++++---- src/Virt_SettingsDefineState.c | 1 + src/Virt_VSSDComponent.c | 1 + 6 files changed, 78 insertions(+), 24 deletions(-) diff --git a/schema/ResourceAllocationSettingData.mof b/schema/ResourceAllocationSettingData.mof index 5c76a1c..bec49b8 100644 --- a/schema/ResourceAllocationSettingData.mof +++ b/schema/ResourceAllocationSettingData.mof @@ -322,31 +322,49 @@ class LXC_InputResourceAllocationSettingData : LXC_ResourceAllocationSettingData string BusType; }; -[Description ("Xen virtual controller device"), - Provider("cmpi::Virt_RASD") -] class Xen_ControllerResourceAllocationSettingData : Xen_ResourceAllocationSettingData { - string Type; - string Model; }; -[Description ("KVM virtual controller device"), +[Description ("KVM virtual controller device. It is identified by: " + "CIM_ResourceAllocationSettingData.ResourceType=1 ('Other'), " + "CIM_ResourceAllocationSettingData.OtherResourceType='controller'" + " and CIM_ResourceAllocationSettingData.ResourceSubType set to " + "one of 'ide', 'fdc', 'scsi', 'sata', 'ccid', 'virtio-serial', " + "or 'pci'."), Provider("cmpi::Virt_RASD") ] class KVM_ControllerResourceAllocationSettingData : KVM_ResourceAllocationSettingData { - string Type; - string Model; -}; + [Description ("Order in which the bus controller is encountered. " + "The order is controller type scoped.")] + uint64 Index; -[Description ("LXC virtual controller device"), - Provider("cmpi::Virt_RASD") -] -class LXC_ControllerResourceAllocationSettingData : LXC_ResourceAllocationSettingData -{ - string Type; + [Description ("Optional string providing a specific model " + "information based on the controller type.")] string Model; + + [Description ("The 'virtio-serial' controller uses the Ports and " + "Vectors to control how many devices can be connected " + "through the controller.")] + sint64 Ports; + sint64 Vectors; + + [Description ("")] + uint64 Queues; + + [Description ("Device master property names")] + string MasterProperties[]; + + [Description ("Device master property values")] + string MasterValues[]; + + [Description ("For controllers that are themselves devices on a " + "PCI or USB bus an optional element to specify the " + "exact relationship of the controller to its master " + "bus. Stored in the property and value arrays.")] + string AddressProperties[]; + string AddressValues[]; }; [Description ("Xen virtual network pool settings"), diff --git a/schema/ResourceAllocationSettingData.registration b/schema/ResourceAllocationSettingData.registration index 1142376..da0b7d9 100644 --- a/schema/ResourceAllocationSettingData.registration +++ b/schema/ResourceAllocationSettingData.registration @@ -7,7 +7,6 @@ Xen_MemResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance Xen_GraphicsResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance Xen_InputResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance Xen_ConsoleResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance -Xen_ControllerResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance KVM_DiskResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance KVM_NetResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance KVM_ProcResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance @@ -22,4 +21,3 @@ LXC_ProcResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance LXC_GraphicsResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance LXC_InputResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance LXC_ConsoleResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance -LXC_ControllerResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance diff --git a/src/Virt_ElementSettingData.c b/src/Virt_ElementSettingData.c index c088e49..1fa81f2 100644 --- a/src/Virt_ElementSettingData.c +++ b/src/Virt_ElementSettingData.c @@ -137,6 +137,7 @@ static char* resource_allocation_setting_data[] = { "KVM_GraphicsResourceAllocationSettingData", "KVM_ConsoleResourceAllocationSettingData", "KVM_InputResourceAllocationSettingData", + "KVM_ControllerResourceAllocationSettingData", "LXC_DiskResourceAllocationSettingData", "LXC_MemResourceAllocationSettingData", "LXC_NetResourceAllocationSettingData", diff --git a/src/Virt_RASD.c b/src/Virt_RASD.c index 249e6e5..d3922cf 100644 --- a/src/Virt_RASD.c +++ b/src/Virt_RASD.c @@ -909,16 +909,51 @@ static CMPIStatus set_input_rasd_params(const struct virt_device *dev, return s; } -static CMPIStatus set_controller_rasd_params(const struct virt_device *dev, +static CMPIStatus set_controller_rasd_params(const CMPIBroker *broker, + const CMPIObjectPath *ref, + const struct virt_device *dev, CMPIInstance *inst) { CMPIStatus s = {CMPI_RC_OK, NULL}; - CMSetProperty(inst, "Type", + CMSetProperty(inst, "OtherResourceType", "controller", CMPI_chars); + CMSetProperty(inst, "ResourceSubType", (CMPIValue *)dev->dev.controller.type, CMPI_chars); - - CMSetProperty(inst, "Model", - (CMPIValue *)dev->dev.controller.model, CMPI_chars); + CMSetProperty(inst, "Index", + (CMPIValue *)dev->dev.controller.index, CMPI_uint32); + + if (dev->dev.controller.model) + CMSetProperty(inst, "Model", + (CMPIValue *)dev->dev.controller.model, CMPI_chars); + + if (dev->dev.controller.ports) + CMSetProperty(inst, "Ports", + (CMPIValue *)dev->dev.controller.ports, CMPI_uint32); + + if (dev->dev.controller.vectors) + CMSetProperty(inst, "Vectors", + (CMPIValue *)dev->dev.controller.vectors, + CMPI_uint32); + + if (dev->dev.controller.queues) + CMSetProperty(inst, "Queues", + (CMPIValue *)dev->dev.controller.queues, CMPI_uint32); + + if (dev->dev.controller.master.ct > 0) + set_rasd_property_value(broker, + ref, + &dev->dev.controller.address, + inst, + "MasterProperties", + "MasterValues"); + + if (dev->dev.controller.address.ct > 0) + set_rasd_property_value(broker, + ref, + &dev->dev.controller.address, + inst, + "AddressProperties", + "AddressValues"); return s; } @@ -958,7 +993,7 @@ CMPIInstance *rasd_from_vdev(const CMPIBroker *broker, type = CIM_RES_TYPE_INPUT; base = "InputResourceAllocationSettingData"; } else if (dev->type == CIM_RES_TYPE_CONTROLLER) { - type = CIM_RES_TYPE_CONTROLLER; + type = CIM_RES_TYPE_OTHER; base = "ControllerResourceAllocationSettingData"; } else { return NULL; @@ -1016,7 +1051,7 @@ CMPIInstance *rasd_from_vdev(const CMPIBroker *broker, } else if (dev->type == CIM_RES_TYPE_CONSOLE) { s = set_console_rasd_params(dev, inst); } else if (dev->type == CIM_RES_TYPE_CONTROLLER) { - s = set_controller_rasd_params(dev, inst); + s = set_controller_rasd_params(broker, ref, dev, inst); } /* FIXME: Put the HostResource in place */ diff --git a/src/Virt_SettingsDefineState.c b/src/Virt_SettingsDefineState.c index c8cda97..d5c6726 100644 --- a/src/Virt_SettingsDefineState.c +++ b/src/Virt_SettingsDefineState.c @@ -361,6 +361,7 @@ static char* resource_allocation_setting_data[] = { "KVM_GraphicsResourceAllocationSettingData", "KVM_InputResourceAllocationSettingData", "KVM_ConsoleResourceAllocationSettingData", + "KVM_ControllerResourceAllocationSettingData", "LXC_DiskResourceAllocationSettingData", "LXC_MemResourceAllocationSettingData", "LXC_NetResourceAllocationSettingData", diff --git a/src/Virt_VSSDComponent.c b/src/Virt_VSSDComponent.c index 35bffde..734271c 100644 --- a/src/Virt_VSSDComponent.c +++ b/src/Virt_VSSDComponent.c @@ -141,6 +141,7 @@ static char* part_component[] = { "KVM_GraphicsResourceAllocationSettingData", "KVM_InputResourceAllocationSettingData", "KVM_ConsoleResourceAllocationSettingData", + "KVM_ControllerResourceAllocationSettingData", "LXC_DiskResourceAllocationSettingData", "LXC_MemResourceAllocationSettingData", "LXC_NetResourceAllocationSettingData", -- 1.8.5.3
Signed-off-by: John Ferlan <jferlan@redhat.com> --- schema/ResourceAllocationSettingData.mof | 48 +++++++++++++++------- schema/ResourceAllocationSettingData.registration | 2 - src/Virt_ElementSettingData.c | 1 + src/Virt_RASD.c | 49 +++++++++++++++++++---- src/Virt_SettingsDefineState.c | 1 + src/Virt_VSSDComponent.c | 1 + 6 files changed, 78 insertions(+), 24 deletions(-)
diff --git a/schema/ResourceAllocationSettingData.mof b/schema/ResourceAllocationSettingData.mof index 5c76a1c..bec49b8 100644 --- a/schema/ResourceAllocationSettingData.mof +++ b/schema/ResourceAllocationSettingData.mof @@ -322,31 +322,49 @@ class LXC_InputResourceAllocationSettingData : LXC_ResourceAllocationSettingData string BusType; };
-[Description ("Xen virtual controller device"), - Provider("cmpi::Virt_RASD") -] class Xen_ControllerResourceAllocationSettingData : Xen_ResourceAllocationSettingData { - string Type; - string Model; }; If you do not register the class why do you leave the class defined?
-[Description ("KVM virtual controller device"), +[Description ("KVM virtual controller device. It is identified by: " + "CIM_ResourceAllocationSettingData.ResourceType=1 ('Other'), " + "CIM_ResourceAllocationSettingData.OtherResourceType='controller'" + " and CIM_ResourceAllocationSettingData.ResourceSubType set to " + "one of 'ide', 'fdc', 'scsi', 'sata', 'ccid', 'virtio-serial', " + "or 'pci'."), Provider("cmpi::Virt_RASD") ] class KVM_ControllerResourceAllocationSettingData : KVM_ResourceAllocationSettingData { - string Type; - string Model; -}; + [Description ("Order in which the bus controller is encountered. " + "The order is controller type scoped.")] + uint64 Index;
-[Description ("LXC virtual controller device"), - Provider("cmpi::Virt_RASD") -] -class LXC_ControllerResourceAllocationSettingData : LXC_ResourceAllocationSettingData -{ - string Type; + [Description ("Optional string providing a specific model " + "information based on the controller type.")] string Model; + + [Description ("The 'virtio-serial' controller uses the Ports and " + "Vectors to control how many devices can be connected " + "through the controller.")] + sint64 Ports; + sint64 Vectors; Why are you using signed 64-bit integers and not unsigned ones? I do not
+ + [Description ("")]
On 03/13/2014 11:27 PM, John Ferlan wrote: think that negative numbers are making sense! that is very brief :-) ("Number of queues for the controller.")
+ uint64 Queues; + + [Description ("Device master property names")] + string MasterProperties[]; + + [Description ("Device master property values")] + string MasterValues[]; + + [Description ("For controllers that are themselves devices on a " + "PCI or USB bus an optional element to specify the " "PCI or USB" should be removed to remain generic or you simply write the description as above for Master... [Description ("Device address property names")] ... [Description ("Device address property values")] + "exact relationship of the controller to its master " + "bus. Stored in the property and value arrays.")] + string AddressProperties[]; + string AddressValues[]; };
[Description ("Xen virtual network pool settings"), diff --git a/schema/ResourceAllocationSettingData.registration b/schema/ResourceAllocationSettingData.registration index 1142376..da0b7d9 100644 --- a/schema/ResourceAllocationSettingData.registration +++ b/schema/ResourceAllocationSettingData.registration @@ -7,7 +7,6 @@ Xen_MemResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance Xen_GraphicsResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance Xen_InputResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance Xen_ConsoleResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance -Xen_ControllerResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance KVM_DiskResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance KVM_NetResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance KVM_ProcResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance @@ -22,4 +21,3 @@ LXC_ProcResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance LXC_GraphicsResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance LXC_InputResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance LXC_ConsoleResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance -LXC_ControllerResourceAllocationSettingData root/virt Virt_RASD Virt_RASD instance diff --git a/src/Virt_ElementSettingData.c b/src/Virt_ElementSettingData.c index c088e49..1fa81f2 100644 --- a/src/Virt_ElementSettingData.c +++ b/src/Virt_ElementSettingData.c @@ -137,6 +137,7 @@ static char* resource_allocation_setting_data[] = { "KVM_GraphicsResourceAllocationSettingData", "KVM_ConsoleResourceAllocationSettingData", "KVM_InputResourceAllocationSettingData", + "KVM_ControllerResourceAllocationSettingData", "LXC_DiskResourceAllocationSettingData", "LXC_MemResourceAllocationSettingData", "LXC_NetResourceAllocationSettingData",
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
From: Xu Wang <gesaint@linux.vnet.ibm.com> Signed-off-by: Xu Wang <gesaint@linux.vnet.ibm.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/Virt_VirtualSystemManagementService.c | 44 +++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index 3e7785e..9634481 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -1838,6 +1838,28 @@ static const char *input_rasd_to_vdev(CMPIInstance *inst, return NULL; } +static const char *controller_rasd_to_vdev(CMPIInstance *inst, + struct virt_device *dev) +{ + const char *val; + + if (cu_get_str_prop(inst, "Type", &val) != CMPI_RC_OK) { + CU_DEBUG("ControllerRASD Type field not valid"); + goto out; + } + dev->dev.controller.type = strdup(val); + + if (cu_get_str_prop(inst, "Model", &val) != CMPI_RC_OK) { + CU_DEBUG("Invalid value for Model in ControllerRASD"); + goto out; + } + dev->dev.controller.model = strdup(val); + + out: + + return NULL; +} + static const char *_sysvirt_rasd_to_vdev(CMPIInstance *inst, struct virt_device *dev, uint16_t type, @@ -1858,6 +1880,8 @@ static const char *_sysvirt_rasd_to_vdev(CMPIInstance *inst, return console_rasd_to_vdev(inst, dev); } else if (type == CIM_RES_TYPE_INPUT) { return input_rasd_to_vdev(inst, dev); + } else if (type == CIM_RES_TYPE_CONTROLLER) { + return controller_rasd_to_vdev(inst, dev); } return "Resource type not supported on this platform"; @@ -1878,6 +1902,8 @@ static const char *_container_rasd_to_vdev(CMPIInstance *inst, return lxc_proc_rasd_to_vdev(inst, dev); } else if (type == CIM_RES_TYPE_INPUT) { return input_rasd_to_vdev(inst, dev); + } else if (type == CIM_RES_TYPE_CONTROLLER) { + return controller_rasd_to_vdev(inst, dev); } return "Resource type not supported on this platform"; @@ -1987,6 +2013,9 @@ static const char *classify_resources(CMPIArray *resources, if (!make_space(&domain->dev_input, domain->dev_input_ct, count)) return "Failed to alloc input list"; + if (!make_space(&domain->dev_controller, domain->dev_controller_ct, count)) + return "Failed to alloc controller list"; + for (i = 0; i < count; i++) { CMPIObjectPath *op; CMPIData item; @@ -2101,7 +2130,16 @@ static const char *classify_resources(CMPIArray *resources, &domain->dev_input[0], ns, p_error); + } else if (type == CIM_RES_TYPE_CONTROLLER) { + msg = rasd_to_vdev(inst, + domain, + &domain->dev_controller[domain->dev_controller_ct], + ns, + p_error); + if (msg == NULL) + domain->dev_controller_ct += 1; } + if (msg != NULL) return msg; @@ -2908,6 +2946,9 @@ static struct virt_device **find_list(struct domain *dominfo, } else if (type == CIM_RES_TYPE_INPUT) { list = &dominfo->dev_input; *count = &dominfo->dev_input_ct; + } else if (type == CIM_RES_TYPE_CONTROLLER) { + list = &dominfo->dev_controller; + *count = &dominfo->dev_controller_ct; } return list; @@ -3029,6 +3070,7 @@ static CMPIStatus resource_del(struct domain *dominfo, if (STREQ(dev->id, devid)) { if ((type == CIM_RES_TYPE_GRAPHICS) || (type == CIM_RES_TYPE_CONSOLE) || + (type == CIM_RES_TYPE_CONTROLLER) || (type == CIM_RES_TYPE_INPUT)) cu_statusf(_BROKER, &s, CMPI_RC_OK, ""); else { @@ -3111,6 +3153,7 @@ static CMPIStatus resource_add(struct domain *dominfo, if ((type == CIM_RES_TYPE_GRAPHICS) || (type == CIM_RES_TYPE_INPUT) || + (type == CIM_RES_TYPE_CONTROLLER) || (type == CIM_RES_TYPE_CONSOLE)) { (*count)++; cu_statusf(_BROKER, &s, CMPI_RC_OK, ""); @@ -3188,6 +3231,7 @@ static CMPIStatus resource_mod(struct domain *dominfo, if ((type == CIM_RES_TYPE_GRAPHICS) || (type == CIM_RES_TYPE_INPUT) || + (type == CIM_RES_TYPE_CONTROLLER) || (type == CIM_RES_TYPE_CONSOLE)) cu_statusf(_BROKER, &s, CMPI_RC_OK, ""); else { -- 1.8.5.3
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/Virt_VirtualSystemManagementService.c | 38 ++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index 9634481..b478d41 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -1841,23 +1841,42 @@ static const char *input_rasd_to_vdev(CMPIInstance *inst, static const char *controller_rasd_to_vdev(CMPIInstance *inst, struct virt_device *dev) { - const char *val; + const char *val = NULL; + const char *msg = NULL; + int ret; - if (cu_get_str_prop(inst, "Type", &val) != CMPI_RC_OK) { - CU_DEBUG("ControllerRASD Type field not valid"); + if (cu_get_str_prop(inst, "ResourceSubType", &val) != CMPI_RC_OK) { + CU_DEBUG("ControllerRASD ResourceSubType field not valid"); goto out; } dev->dev.controller.type = strdup(val); + if (cu_get_u32_prop(inst, "Index", + &dev->dev.controller.index) != CMPI_RC_OK) { + CU_DEBUG("ControllerRASD Index field not valid"); + goto out; + } + if (cu_get_str_prop(inst, "Model", &val) != CMPI_RC_OK) { CU_DEBUG("Invalid value for Model in ControllerRASD"); goto out; } dev->dev.controller.model = strdup(val); + ret = asprintf(&dev->id, "controller:%s:%s", + dev->dev.controller.type, + dev->dev.controller.index); + if (ret == -1) { + msg = "Failed to create controller string"; + goto out; + } + + msg = rasd_to_device_address(inst, &dev->dev.controller.address); + + out: - return NULL; + return ; } static const char *_sysvirt_rasd_to_vdev(CMPIInstance *inst, @@ -2131,13 +2150,20 @@ static const char *classify_resources(CMPIArray *resources, ns, p_error); } else if (type == CIM_RES_TYPE_CONTROLLER) { + struct virt_device dev; + int dcount = count + domain->dev_controller_ct; + + memset(&dev, 0, sizeof(dev)); msg = rasd_to_vdev(inst, domain, - &domain->dev_controller[domain->dev_controller_ct], + &dev, ns, p_error); if (msg == NULL) - domain->dev_controller_ct += 1; + msg = add_device_nodup(&dev, + domain->dev_controller, + dcount, + &domain->dev_controller_ct); } if (msg != NULL) -- 1.8.5.3
----- Original Message -----
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/Virt_VirtualSystemManagementService.c | 38 ++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 6 deletions(-)
diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index 9634481..b478d41 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -1841,23 +1841,42 @@ static const char *input_rasd_to_vdev(CMPIInstance *inst, static const char *controller_rasd_to_vdev(CMPIInstance *inst, struct virt_device *dev) { - const char *val; + const char *val = NULL; + const char *msg = NULL; + int ret;
- if (cu_get_str_prop(inst, "Type", &val) != CMPI_RC_OK) { - CU_DEBUG("ControllerRASD Type field not valid"); + if (cu_get_str_prop(inst, "ResourceSubType", &val) != CMPI_RC_OK) { + CU_DEBUG("ControllerRASD ResourceSubType field not valid"); goto out; } dev->dev.controller.type = strdup(val);
+ if (cu_get_u32_prop(inst, "Index", + &dev->dev.controller.index) != CMPI_RC_OK) { + CU_DEBUG("ControllerRASD Index field not valid"); + goto out; + } + if (cu_get_str_prop(inst, "Model", &val) != CMPI_RC_OK) { CU_DEBUG("Invalid value for Model in ControllerRASD"); goto out; } dev->dev.controller.model = strdup(val);
+ ret = asprintf(&dev->id, "controller:%s:%s", + dev->dev.controller.type, + dev->dev.controller.index); + if (ret == -1) { + msg = "Failed to create controller string"; + goto out; + } + + msg = rasd_to_device_address(inst, &dev->dev.controller.address); + + out:
- return NULL; + return ;
I am not clear why here is just 'return' ? IMHO, if this function doesn't return anything, should it be defined returning void? Otherwise return NULL is more safe, that is because if someone wanna reference this return value, 'return' will give him a ramdom value(from a place of stack), That will be SEGV.
}
static const char *_sysvirt_rasd_to_vdev(CMPIInstance *inst, @@ -2131,13 +2150,20 @@ static const char *classify_resources(CMPIArray *resources, ns, p_error); } else if (type == CIM_RES_TYPE_CONTROLLER) { + struct virt_device dev; + int dcount = count + domain->dev_controller_ct; + + memset(&dev, 0, sizeof(dev)); msg = rasd_to_vdev(inst, domain, - &domain->dev_controller[domain->dev_controller_ct], + &dev, ns, p_error); if (msg == NULL) - domain->dev_controller_ct += 1; + msg = add_device_nodup(&dev, + domain->dev_controller, + dcount, + &domain->dev_controller_ct); }
if (msg != NULL) -- 1.8.5.3
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
On 03/13/2014 11:27 PM, John Ferlan wrote:
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/Virt_VirtualSystemManagementService.c | 38 ++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 6 deletions(-)
diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index 9634481..b478d41 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -1841,23 +1841,42 @@ static const char *input_rasd_to_vdev(CMPIInstance *inst, static const char *controller_rasd_to_vdev(CMPIInstance *inst, struct virt_device *dev) { - const char *val; + const char *val = NULL; + const char *msg = NULL; + int ret;
- if (cu_get_str_prop(inst, "Type", &val) != CMPI_RC_OK) { - CU_DEBUG("ControllerRASD Type field not valid"); + if (cu_get_str_prop(inst, "ResourceSubType", &val) != CMPI_RC_OK) { + CU_DEBUG("ControllerRASD ResourceSubType field not valid"); goto out; } dev->dev.controller.type = strdup(val);
cu_get_u64_prop if Index is of type uint64
+ if (cu_get_u32_prop(inst, "Index", + &dev->dev.controller.index) != CMPI_RC_OK) { CU_DEBUG("ControllerRASD Index not set - DEFAULT"); dev->dev.controller.index = CONTROLLER_INDEX_NOT_SET; } else { if (asprintf(&dev->id, "controller:%s:%" PRIu64, dev->dev.controller.type, dev->dev.controller.index) == -1) { CU_DEBUG("Failed to create controller id string"); msg = "Failed to create controller id string"; goto out; } } It is possible that index during the creation of domain is not specified and than libvirt will define the index itself.
+ CU_DEBUG("ControllerRASD Index field not valid"); + goto out; + } + if (cu_get_str_prop(inst, "Model", &val) != CMPI_RC_OK) { CU_DEBUG("Invalid value for Model in ControllerRASD"); goto out; } dev->dev.controller.model = strdup(val);
+ ret = asprintf(&dev->id, "controller:%s:%s", + dev->dev.controller.type, + dev->dev.controller.index); + if (ret == -1) { + msg = "Failed to create controller string"; + goto out; + } ---- snap ----
---- snip ---- the above block could be deleted
+ + msg = rasd_to_device_address(inst, &dev->dev.controller.address); + + out:
- return NULL; + return ; This should probably be return msg; }
static const char *_sysvirt_rasd_to_vdev(CMPIInstance *inst, @@ -2131,13 +2150,20 @@ static const char *classify_resources(CMPIArray *resources, ns, p_error); } else if (type == CIM_RES_TYPE_CONTROLLER) { + struct virt_device dev; + int dcount = count + domain->dev_controller_ct; + + memset(&dev, 0, sizeof(dev)); msg = rasd_to_vdev(inst, domain, - &domain->dev_controller[domain->dev_controller_ct], + &dev, ns, p_error); if (msg == NULL) - domain->dev_controller_ct += 1; + msg = add_device_nodup(&dev, + domain->dev_controller, + dcount, + &domain->dev_controller_ct); }
if (msg != NULL)
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 03/13/2014 11:26 PM, John Ferlan wrote:
I took the "liberty" of making some adjustments based on the base series. I have left the base code "as is" in patches 1, 3, and 6. Just my observation: In leaving these untouched it makes it more difficult to review this overall but I understand that you, John, want to preserve the credits for the initial version of Xu Wang. My preference would be to have a patch series that contains the bare required changes without the history of how it evolved. John and Xu, would that be OK for both of you?
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 03/14/2014 04:32 AM, Boris Fiuczynski wrote:
On 03/13/2014 11:26 PM, John Ferlan wrote:
I took the "liberty" of making some adjustments based on the base series. I have left the base code "as is" in patches 1, 3, and 6. Just my observation: In leaving these untouched it makes it more difficult to review this overall but I understand that you, John, want to preserve the credits for the initial version of Xu Wang. My preference would be to have a patch series that contains the bare required changes without the history of how it evolved. John and Xu, would that be OK for both of you?
I can merge the pieces - no problem. Author/attribution for me is not that important. I was trying to show what I added. If you pulled the series into your own git branch and rebased/merged 1/2, 3/4/5, and 6/7 - then you'd have pretty much what I'll be doing. Give me a bit of time for the coffee to start coursing through my veins and the sleep cobwebs to clear and I'll repost. John
participants (3)
-
Boris Fiuczynski -
Jincheng Miao -
John Ferlan