[Libvirt-cim] [PATCH 2/7] Adjustments to patch 1/3
Boris Fiuczynski
fiuczy at linux.vnet.ibm.com
Fri Mar 14 09:45:51 UTC 2014
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 at 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
More information about the Libvirt-cim
mailing list