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

John Ferlan jferlan at redhat.com
Thu Mar 13 14:00:35 UTC 2014



On 03/12/2014 12:36 PM, Boris Fiuczynski wrote:
> On 03/03/2014 08:38 AM, Xu Wang wrote:
>> Signed-off-by: Xu Wang <gesaint at linux.vnet.ibm.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 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 at 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"
>> @@ -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.
>> +        }
>> +
>> +        return NULL;
>> +}
>> +
>>   static char *system_xml(xmlNodePtr root, struct domain *domain)
>>   {
>>           xmlNodePtr tmp;
>> @@ -1129,6 +1152,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;
>> @@ -1167,6 +1194,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
> 33 is DMTF reserved as an Ethernet Connection the next logical number in 
> libvirt-cim terms would be 32771 (see below)
>>   #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
>>
> 
> 

Because the "importance" of this ratcheted up a bit, I've gone ahead and
started trying to add the missing parts myself.  I probably have gone
overboard, but I added the index, ports, queues, vectors, address, and
master fields as described in the controller section of the domain.

The "master" inside libvirt is parsed similarly to the address as a list
of pairs that we don't care what the contents are - just that we copy
them over.

Taking the 1/3 changes, I've attached the adjustments. I've run *just*
this pair of changes thru cimtest without any new failures than I found
with the rawio/sgio changes.  You should be able to just "git am" them
to the existing 1/3 changes.

John

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Adjustments-to-patch-1-3.patch
Type: text/x-patch
Size: 6583 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvirt-cim/attachments/20140313/733fff64/attachment.bin>


More information about the Libvirt-cim mailing list