Re: [Libvirt-cim] [PATCH 1/3] libxutil: Controller Support

Here the other address stuff On 03/12/2014 05:36 PM, Boris Fiuczynski wrote: > On 03/03/2014 08:38 AM, Xu Wang wrote: >> Signed-off-by: Xu Wang <gesaint(a)linux.vnet.ibm.com&gt; >> --- >> 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 d2d3859..1937132 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(a)us.ibm.com&gt; >> @@ -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" >> @@ -308,6 +309,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); cleanup_device_address(&dev->address); >> +} >> + >> void cleanup_virt_device(struct virt_device *dev) >> { >> if (dev == NULL) >> @@ -325,6 +335,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); >> >> @@ -1107,6 +1119,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"); >> + for (child = node->children; child != NULL; child = child->next) { if (XSTREQ(child->name, "address")) { parse_device_address(child, &cdev->address); } } >> + if (cdev->type == NULL) >> + goto err; >> + >> + vdev->type = CIM_RES_TYPE_CONTROLLER; >> + >> + ret = asprintf(&vdev->id, "%s", cdev->type); > What is going to happen if you have more than one instance of the same > type of controller? In other words, I do not think that type is unique > to be the ID. To make it unique you also need the index which is also a > mandatory attribute. >> + 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; >> @@ -1230,6 +1278,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; >> @@ -1351,7 +1403,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); duplicate_device_address(&dev->dev.controller.address, &_dev->dev.controller.address);

>> } >> + >> return dev; >> } >> >> @@ -1731,6 +1787,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; >> >> @@ -1819,6 +1878,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 a92e223..cc58970 100644 >> --- a/libxkutil/device_parsing.h >> +++ b/libxkutil/device_parsing.h >> @@ -163,6 +163,11 @@ struct input_device { >> char *bus; >> }; >> >> +struct controller_device { >> + char *type; >> + char *model; struct device_address address; >> +}; >> + >> struct virt_device { >> uint16_t type; >> union { >> @@ -174,6 +179,7 @@ struct virt_device { >> struct graphics_device graphics; >> struct console_device console; >> struct input_device input; >> + struct controller_device controller; >> } dev; >> char *id; >> }; >> @@ -249,6 +255,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 18c4765..537238d 100644 >> --- a/libxkutil/xmlgen.c >> +++ b/libxkutil/xmlgen.c >> @@ -798,6 +798,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); > Model is an optional attribute. So if I read this correctly dev->model > might be NULL and you create a new property with it. I am not sure what > libvirt is going to do about it. if (dev->address.ct > 0) return device_address_xml(tmp, &dev->address);