Looks good. +1. On 12/30/2011 07:54 AM, Gareth S Bestor wrote: > From: Gareth S. Bestor&lt;bestor(a)us.ibm.com&gt; > > This patch updates the earlier patch to libvirt-cim for network bandwidth QOS > support. Previous patch had to perform all external host 'tc' (traffic control) > commands from within libvirt-cim providers, because earlier versions of > libvirt lacked native QoS support. libvirt 0.9.4 added native QoS support; > this new patch for libvirt-cim removes earlier QoS hacks in favor of directly > exploiting libvirt equivalent QoS function and configuration. Note, libvirt > supports three tunables for network bandwidth QoS: average, peak and burst, > for both inbound and outbound traffic independently. > This revised libvirt-cim patch only exposes - as previously - inbound traffic > QoS, but adds peak to previous average tunable support. > Selection of earlier (libvirt-cim QoS hack) vs latter (native libvirt QoS) > is done at compile-time based on libvirt version being built against. > As previously, QoS capabilites are exposed on the virtual network pool, and > the QoS setting for a specific guest's NIC are exposed on the associated > network device RASD. QoS settings may be specified during guest creation > (DefineSystem) or guest modification (ModifyResourceSettings). > > Signed-off-by: Gareth S. Bestor&lt;bestor(a)us.ibm.com&gt; > --- > libxkutil/device_parsing.c | 34 +++++++++++++++++++++ > libxkutil/device_parsing.h | 2 + > libxkutil/xmlgen.c | 39 ++++++++++++++++++++++++ > src/Virt_RASD.c | 33 ++++++++++++++++++-- > src/Virt_SettingsDefineCapabilities.c | 47 +++++++++++++++++++++++++---- > src/Virt_VirtualSystemManagementService.c | 22 +++++++++++++- > 6 files changed, 166 insertions(+), 11 deletions(-) > > diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c > index 371838f..7eaa63e 100644 > --- a/libxkutil/device_parsing.c > +++ b/libxkutil/device_parsing.c > @@ -444,7 +444,39 @@ static int parse_net_device(xmlNode *inode, struct > virt_device **vdevs) > ndev->filter_ref = get_attr_value(child, "filter"); > } else if (XSTREQ(child->name, "virtualport")) { > parse_vsi_device(child, ndev); > +#if LIBVIR_VERSION_NUMBER>= 9000 > + } else if (XSTREQ(child->name, "bandwidth")) { > + /* Network QoS bandwidth support */ > + xmlNode *grandchild = NULL; > + for (grandchild = child->children; > + grandchild != NULL; > + grandchild = grandchild->next) { > + if (XSTREQ(grandchild->name, "inbound")) { > + /* Only expose inbound bandwidth */ > + char *val; > + > + val = get_attr_value(grandchild, > + "average"); > + if (val != NULL) { > + sscanf(val, "%" PRIu64, > +&ndev->reservation); > + free(val); > + } else > + ndev->reservation = 0; > + > + val = get_attr_value(grandchild, > + "peak"); > + if (val != NULL) { > + sscanf(val, "%" PRIu64, > +&ndev->limit); > + free(val); > + } else > + ndev->limit = 0; > + break; > + } > + } > } > +#endif > > } > > @@ -861,6 +893,8 @@ struct virt_device *virt_device_dup(struct virt_device > *_dev) > DUP_FIELD(dev, _dev, dev.net.vsi.instance_id); > DUP_FIELD(dev, _dev, dev.net.vsi.filter_ref); > DUP_FIELD(dev, _dev, dev.net.vsi.profile_id); > + dev->dev.net.reservation = _dev->dev.net.reservation; > + dev->dev.net.limit = _dev->dev.net.limit; > } else if (dev->type == CIM_RES_TYPE_DISK) { > DUP_FIELD(dev, _dev, dev.disk.type); > DUP_FIELD(dev, _dev, dev.disk.device); > diff --git a/libxkutil/device_parsing.h b/libxkutil/device_parsing.h > index ab104d9..f24268b 100644 > --- a/libxkutil/device_parsing.h > +++ b/libxkutil/device_parsing.h > @@ -66,6 +66,8 @@ struct net_device { > char *device; > char *net_mode; > char *filter_ref; > + uint64_t reservation; > + uint64_t limit; > struct vsi_device vsi; > }; > > diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c > index aae1e51..638cffe 100644 > --- a/libxkutil/xmlgen.c > +++ b/libxkutil/xmlgen.c > @@ -320,6 +320,45 @@ static const char *net_xml(xmlNodePtr root, struct > domain *dominfo) > BAD_CAST net->filter_ref); > } > > +#if LIBVIR_VERSION_NUMBER>= 9000 > + /* Network QoS settings saved under<bandwidth> XML section */ > + if (net->reservation || net->limit) { > + int ret; > + char *string = NULL; > + > + tmp = xmlNewChild(nic, NULL, > + BAD_CAST "bandwidth", NULL); > + if (tmp == NULL) > + return XML_ERROR; > + > + /* Set inbound bandwidth from Reservation& Limit */ > + tmp = xmlNewChild(tmp, NULL, > + BAD_CAST "inbound", NULL); > + if (tmp == NULL) > + return XML_ERROR; > + > + if (net->reservation) { > + ret = asprintf(&string, "%" PRIu64, > + net->reservation); > + if (ret == -1) > + return XML_ERROR; > + xmlNewProp(tmp, BAD_CAST "average", > + BAD_CAST string); > + free(string); > + } > + > + if (net->limit) { > + ret = asprintf(&string, "%" PRIu64, > + net->limit); > + if (ret == -1) > + return XML_ERROR; > + xmlNewProp(tmp, BAD_CAST "peak", > + BAD_CAST string); > + free(string); > + } > + } > +#endif > + > if (STREQ(dev->dev.net.type, "network")) > msg = set_net_source(nic, net, "network"); > else if (STREQ(dev->dev.net.type, "bridge")) > diff --git a/src/Virt_RASD.c b/src/Virt_RASD.c > index e148674..a868c21 100644 > --- a/src/Virt_RASD.c > +++ b/src/Virt_RASD.c > @@ -39,6 +39,7 @@ > #include "svpc_types.h" > #include "Virt_Device.h" > > +#if LIBVIR_VERSION_NUMBER< 9000 > /* Network QoS support */ > #define QOSCMD_MAC2BANDWIDTH "_ROOT=$(tc class show dev %s | awk > '($4==\"root\")\ > {print $3}')\n _ID=$(tc filter show dev %s | awk 'BEGIN {RS=\"\\nfilter\"} > (NR>2)\ > @@ -48,6 +49,7 @@ m1,m2,m3,m4,m5,m6,$18)}' | awk -v mm=%s '($1==mm){print > $2}')\n \ > if [[ -n \"$_ID\" ]]; then\n tc class show dev %s | awk -v rr=$_ROOT -v > id=$_ID \ > '($4==\"parent\"&& $5==rr&& $3==id){print \ > substr($13,1,(index($13,\"Kbit\")-1))}'\n fi\n" > +#endif > > const static CMPIBroker *_BROKER; > > @@ -463,11 +465,7 @@ static CMPIStatus set_net_rasd_params(const CMPIBroker > *broker, > const struct virt_device *dev, > CMPIInstance *inst) > { > - FILE *pipe = NULL; > - char *cmd = NULL; > - uint64_t val = 0; > CMPIStatus s = {CMPI_RC_OK, NULL}; > - int i; > > CMSetProperty(inst, > "NetworkType", > @@ -485,8 +483,14 @@ static CMPIStatus set_net_rasd_params(const CMPIBroker > *broker, > (CMPIValue *)dev->dev.net.source, > CMPI_chars); > > +#if LIBVIR_VERSION_NUMBER< 9000 > /* Network QoS support */ > if ((dev->dev.net.mac != NULL)&& (dev->dev.net.source != NULL)) { > + FILE *pipe = NULL; > + char *cmd = NULL; > + uint64_t val = 0; > + int i; > + > /* Get tc performance class bandwidth for this MAC addr */ > i = asprintf(&cmd, QOSCMD_MAC2BANDWIDTH, dev->dev.net.source, > dev->dev.net.source, > @@ -511,6 +515,25 @@ static CMPIStatus set_net_rasd_params(const CMPIBroker > *broker, > } > free(cmd); > } > +#else > + if (dev->dev.net.reservation) { > + CMSetProperty(inst, > + "Reservation", > + (CMPIValue *)&(dev->dev.net.reservation), > + CMPI_uint64); > + > + if (dev->dev.net.limit) > + CMSetProperty(inst, > + "Limit", > + (CMPIValue *)&(dev->dev.net.limit), > + CMPI_uint64); > + > + CMSetProperty(inst, > + "AllocationUnits", > + (CMPIValue *)"KiloBytes per Second", > + CMPI_chars); > + } > +#endif > > if ((dev->dev.net.source != NULL)&& > (STREQ(dev->dev.net.type, "direct"))) > @@ -543,7 +566,9 @@ static CMPIStatus set_net_rasd_params(const CMPIBroker > *broker, > (CMPIValue *)dev->dev.net.poolid, > CMPI_chars); > > +#if LIBVIR_VERSION_NUMBER< 9000 > out: > +#endif > return s; > } > > diff --git a/src/Virt_SettingsDefineCapabilities.c > b/src/Virt_SettingsDefineCapabilities.c > index 129749e..660103a 100644 > --- a/src/Virt_SettingsDefineCapabilities.c > +++ b/src/Virt_SettingsDefineCapabilities.c > @@ -74,9 +74,11 @@ const static CMPIBroker *_BROKER; > #define NEW_VOL_RASD 2 > > /* QoS Network support */ > +#if LIBVIR_VERSION_NUMBER< 9000 > #define QOSCMD_LISTCLASSES "_ROOT=$(tc class show dev %s | awk > '($4==\"root\")\ > {print $3}')\n tc class show dev %s | awk -v rr=$_ROOT \ > '($4==\"parent\"&& $5==rr){print $3\" \"$13}'\n" > +#endif > > static bool system_has_vt(virConnectPtr conn) > { > @@ -567,6 +569,8 @@ static CMPIStatus set_net_props(int type, > const char *net_type, > const char *net_name, > uint64_t num_nics, > + uint64_t reservation, > + uint64_t limit, > const char *device, > const char *src_dev, > const char *net_mode, > @@ -594,6 +598,21 @@ static CMPIStatus set_net_props(int type, > CMSetProperty(inst, "VirtualQuantity", > (CMPIValue *)&num_nics, CMPI_uint64); > > +#if LIBVIR_VERSION_NUMBER>= 9000 > + /* Network QoS support for later libvirt versions */ > + if (reservation) > + CMSetProperty(inst, "Reservation", > + (CMPIValue *)&reservation, CMPI_uint64); > + > + if (limit) > + CMSetProperty(inst, "Limit", > + (CMPIValue *)&reservation, CMPI_uint64); > + > + if (reservation || limit) > + CMSetProperty(inst, "AllocationUnits", > + (CMPIValue *)"KiloBytes per Second", CMPI_chars); > +#endif > + > if (device != NULL) > CMSetProperty(inst, "VirtualDevice", > (CMPIValue *)device, CMPI_chars); > @@ -647,6 +666,8 @@ static CMPIStatus net_template(const CMPIObjectPath *ref, > { > bool ret; > uint64_t num_nics; > + uint64_t reservation = 0; > + uint64_t limit = 0; > const char *id; > CMPIStatus s = {CMPI_RC_OK, NULL}; > int i,j; > @@ -658,16 +679,21 @@ static CMPIStatus net_template(const CMPIObjectPath *ref, > switch (template_type) { > case SDC_RASD_MIN: > num_nics = 0; > + reservation = 1; > + limit = 1; > id = "Minimum"; > break; > case SDC_RASD_MAX: > ret = get_max_nics(ref,&num_nics,&s); > if (!ret) > goto out; > + /* No apparant maximum reservation or limit QoS setting in > libvirt! */ > id = "Maximum"; > break; > case SDC_RASD_INC: > num_nics = 1; > + reservation = 1; > + limit = 1; > id = "Increment"; > break; > case SDC_RASD_DEF: > @@ -689,7 +715,9 @@ static CMPIStatus net_template(const CMPIObjectPath *ref, > id, > type[i], > name[i], > - num_nics, > + num_nics, > + reservation, > + limit, > device[j], > NULL, > NULL, > @@ -707,22 +735,26 @@ static CMPIStatus net_template(const CMPIObjectPath *ref, > } > > s = set_net_props(template_type, ref, id, "direct", NULL, > - num_nics, NULL, "eth1", "vepa", NULL, > + num_nics, reservation, limit, > + NULL, "eth1", "vepa", NULL, > NULL, NULL, NULL, NULL, NULL, NULL, list); > /* profile id*/ > s = set_net_props(template_type, ref, id, "direct", NULL, > - num_nics, NULL, "eth1", "vepa", NULL, > + num_nics, reservation, limit, > + NULL, "eth1", "vepa", NULL, > "802.1Qbh", NULL, NULL, NULL, > NULL, "my_profile", list); > /* no profile id but with instance id*/ > s = set_net_props(template_type, ref, id, "direct", NULL, > - num_nics, NULL, "eth1", "vepa", NULL, > + num_nics, reservation, limit, > + NULL, "eth1", "vepa", NULL, > "802.1Qbg", "managerid", "typeid", > "typeidversion", "instanceid", NULL, > list); > /* no profile id and no instance id*/ > s = set_net_props(template_type, ref, id, "direct", NULL, > - num_nics, NULL, "eth1", "vepa", NULL, > + num_nics, reservation, limit, > + NULL, "eth1", "vepa", NULL, > "802.1Qbg", "managerid", "typeid", > "typeidversion", "NULL", "NULL", list); > > @@ -814,6 +846,7 @@ static CMPIStatus set_net_pool_props(const CMPIObjectPath > *ref, > return s; > } > > +#if LIBVIR_VERSION_NUMBER< 9000 > static char * get_bridge_name(virConnectPtr conn, const char *name) > { > char *bridge = NULL; > @@ -913,7 +946,7 @@ static CMPIStatus qos_hack( > > return s; > } > - > +#endif > > static CMPIStatus net_pool_template(const CMPIObjectPath *ref, > int template_type, > @@ -2086,8 +2119,10 @@ static CMPIStatus sdc_rasds_for_type(const > CMPIObjectPath *ref, > } > } > > +#if LIBVIR_VERSION_NUMBER< 9000 > if (type == CIM_RES_TYPE_NET) > s = qos_hack(ref, list); > +#endif > > out: > return s; > diff --git a/src/Virt_VirtualSystemManagementService.c > b/src/Virt_VirtualSystemManagementService.c > index 0141515..4e16b81 100644 > --- a/src/Virt_VirtualSystemManagementService.c > +++ b/src/Virt_VirtualSystemManagementService.c > @@ -68,6 +68,7 @@ > #define RASD_IND_DELETED "ResourceAllocationSettingDataDeletedIndication" > #define RASD_IND_MODIFIED "ResourceAllocationSettingDataModifiedIndication" > > +#if LIBVIR_VERSION_NUMBER< 9000 > /* Network QoS support */ > #define QOSCMD_BANDWIDTH2ID "_ROOT=$(tc class show dev %s | awk > '($4==\"root\")\ > {print $3}')\n tc class show dev %s | awk -v rr=$_ROOT -v bw=%uKbit \ > @@ -96,6 +97,7 @@ $U32 match u16 0x0800 0xFFFF at -2 match u16 0x$ME2 0xFFFF > at -4 match u32 \ > ffff: prio 50 u32\"; $U32 match u16 0x0800 0xFFFF at -2 match u32 0x$MI2 \ > 0xFFFFFFFF at -12 match u16 0x$MI1 0xFFFF at -14 police rate %uKbit burst > 15k \ > drop\n" > +#endif > > const static CMPIBroker *_BROKER; > > @@ -105,6 +107,7 @@ enum ResourceAction { > RESOURCE_MOD, > }; > > +#if LIBVIR_VERSION_NUMBER< 9000 > /* Network QoS support */ > static CMPIStatus add_qos_for_mac(const uint64_t qos, > const char *mac, > @@ -202,6 +205,7 @@ static CMPIStatus remove_qos_for_mac(const uint64_t qos, > free(cmd); > return s; > } > +#endif > > static CMPIStatus check_uuid_in_use(const CMPIObjectPath *ref, > struct domain *domain) > @@ -973,6 +977,15 @@ static const char *net_rasd_to_vdev(CMPIInstance *inst, > dev->dev.net.model = NULL; > else > dev->dev.net.model = strdup(val); > + > + if (cu_get_u64_prop(inst, "Reservation", > +&dev->dev.net.reservation) != CMPI_RC_OK) > + dev->dev.net.reservation = 0; > + > + if (cu_get_u64_prop(inst, "Limit", > +&dev->dev.net.limit) != CMPI_RC_OK) > + dev->dev.net.limit = 0; > + > out: > free(network); > return msg; > @@ -1645,8 +1658,10 @@ static const char *classify_resources(CMPIArray > *resources, > } else if (type == CIM_RES_TYPE_NET) { > struct virt_device dev; > int ncount = count + domain->dev_net_ct; > +#if LIBVIR_VERSION_NUMBER< 9000 > uint64_t qos_val = 0; > const char *qos_unitstr; > +#endif > > memset(&dev, 0, sizeof(dev)); > msg = rasd_to_vdev(inst, > @@ -1659,6 +1674,7 @@ static const char *classify_resources(CMPIArray > *resources, > ncount, > &domain->dev_net_ct); > > +#if LIBVIR_VERSION_NUMBER< 9000 > /* Network QoS support */ > if (((&dev)->dev.net.mac != NULL)&& > ((&dev)->dev.net.source != NULL)&& > @@ -1672,6 +1688,7 @@ static const char *classify_resources(CMPIArray > *resources, > (&dev)->dev.net.mac, > (&dev)->dev.net.source); > } > +#endif > } else if (type == CIM_RES_TYPE_GRAPHICS) { > struct virt_device dev; > int gcount = count + domain->dev_graphics_ct; > @@ -2763,14 +2780,16 @@ static CMPIStatus resource_mod(struct domain *dominfo, > (type == CIM_RES_TYPE_INPUT)) > cu_statusf(_BROKER,&s, CMPI_RC_OK, ""); > else { > +#if LIBVIR_VERSION_NUMBER< 9000 > uint64_t qos_val = 0; > const char *qos_unitstr; > - > +#endif > s = _resource_dynamic(dominfo, > dev, > RESOURCE_MOD, > CLASSNAME(op)); > > +#if LIBVIR_VERSION_NUMBER< 9000 > /* Network QoS support */ > if ((type == CIM_RES_TYPE_NET)&& > (dev->dev.net.mac != NULL)&& > @@ -2785,6 +2804,7 @@ static CMPIStatus resource_mod(struct domain *dominfo, > dev->dev.net.mac, > > dev->dev.net.source); > } > +#endif > } > break; > }