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

> +/** > + * 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.

> + > +/** > + * 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 ?