[libvirt] [PATCH v2 2/2] add interface for blkio.weight_device

[Hu Tao]

> > +/**
> > + * virBlkioWeightDeviceToStr:
> > + *
> > + * This function returns a string representing device weights that is
> > + * suitable for writing to /cgroup/blkio/blkio.weight_device, given
> > + * a list of weight devices.
> > + */
> > +int virBlkioWeightDeviceToStr(virBlkioWeightDevicePtr weightdevices,
> > +                              int ndevices,
> > +                              char **result)
> > +{
> > +    int len = 0;
> > +    int ret = -1;
> > +    int i, j;
> > +    char **weight_devices;
> > +    char *str;
> > +
> > +    if (VIR_ALLOC_N(weight_devices, ndevices) < 0) {
> > +        goto fail_nomem1;
> > +    }
> > +    for (i = 0; i < ndevices; i++) {
> > +        int tmp;
> > +        tmp = virAsprintf(&weight_devices[i], "%d:%d %d",
> > +                          weightdevices[i].major,
> > +                          weightdevices[i].minor,
> > +                          weightdevices[i].weight);
> > +        if (tmp < 0) {
> > +            goto fail_nomem2;
> > +        }
> > +        len += tmp + 1; /* 1 for '\n' and the trailing '\0' */
> > +    }
> > +
> > +    if (VIR_ALLOC_N(str, len) < 0) {
> > +        goto fail_nomem2;
> > +    }
> > +    for (i = 0; i < ndevices; i++) {
> > +        strcat(str, weight_devices[i]);
> > +        strcat(str, "\n");
> > +    }
> > +    str[len-1] = '\0';
> > +
> > +    *result = str;
> > +
> > +    ret = 0;
> > +
> > +fail_nomem2:
> > +    for (j = 0; j < i; j++)
> > +        VIR_FREE(weight_devices[i]);
> > +    VIR_FREE(weight_devices);
> > +fail_nomem1:
> > +    if (ret != 0)
> > +        virReportOOMError();
> > +    return ret;
> > +}
> 
> I think this method would become alot simpler if you switch
> over to use virBufferPtr for all the string concatenation
> and formatting.

Thanks, I'll look into this.

> 
> > +
> > +/**
> > + * virDomainBlkioWeightDeviceParseXML
> > + *
> > + * this function parses a XML node:
> > + *
> > + *   <device>
> > + *     <major>major</major>
> > + *     <minor>minor</minor>
> > + *     <weight>weight</weight>
> > + *   </device>
> > + *
> > + * and fills a virBlkioWeightDevice struct.
> > + */
> 
> I'm not really seeing the benefit in using major, minor in the XML for
> this. The <disk> element is using the /dev/hda1 path for the host
> device, so I'd expect the same path to be usable for the block I/O
> tuning.
> 
> How does the scope work here, does major,minor have to refer to a block
> device, or can it refer to a partition ?  If we have multiple <device>
> elements, each giving a different partition on the same device can we
> set different weight for each partition ?

blkio doesn't support io control on a partition, so I add major, minor
to refer to a block device as a whole, rather than utilizing the existing
<disk> element.

-- 
Thanks,
Hu Tao