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

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);

> +} > + > 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"); > +

> + 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);

> } > + > 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 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.