On Thu, Sep 08, 2011 at 02:59:56PM +0800, Hu Tao wrote:
This patch adds a parameter --weight-device to virsh command blkiotune for setting/getting blkio.weight_device. --- daemon/remote.c | 5 + include/libvirt/libvirt.h.in | 9 ++ src/conf/domain_conf.c | 142 ++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 15 ++++ src/libvirt_private.syms | 1 + src/qemu/qemu_cgroup.c | 22 ++++++ src/qemu/qemu_driver.c | 170 +++++++++++++++++++++++++++++++++++++++++- src/util/cgroup.c | 33 ++++++++ src/util/cgroup.h | 3 + tools/virsh.c | 31 ++++++++ tools/virsh.pod | 5 +- 11 files changed, 430 insertions(+), 6 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index a9d0daa..ec91526 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1503,6 +1503,7 @@ remoteDispatchDomainGetBlkioParameters(virNetServerPtr server ATTRIBUTE_UNUSED, int nparams = args->nparams; unsigned int flags; int rv = -1; + int i; struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
@@ -1547,6 +1548,10 @@ success: cleanup: if (rv < 0) virNetMessageSaveError(rerr); + for (i = 0; i < nparams; i++) { + if (params[i].type == VIR_TYPED_PARAM_STRING) + VIR_FREE(params[i].value.s); + } VIR_FREE(params); if (dom) virDomainFree(dom); diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index e57241c..c65d8f7 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1137,6 +1137,15 @@ char * virDomainGetSchedulerType(virDomainPtr domain,
#define VIR_DOMAIN_BLKIO_WEIGHT "weight"
+/** + * VIR_DOMAIN_BLKIO_WEIGHT_DEVICE: + * + * Macro for the blkio tunable weight_device: it represents the + * per device weight. + */ + +#define VIR_DOMAIN_BLKIO_WEIGHT_DEVICE "weight_device" + /* Set Blkio tunables for the domain*/ int virDomainSetBlkioParameters(virDomainPtr domain, virTypedParameterPtr params, diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 74f8d6a..d10e30c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -565,6 +565,108 @@ VIR_ENUM_IMPL(virDomainNumatuneMemMode, VIR_DOMAIN_NUMATUNE_MEM_LAST, #define VIR_DOMAIN_XML_WRITE_FLAGS VIR_DOMAIN_XML_SECURE #define VIR_DOMAIN_XML_READ_FLAGS VIR_DOMAIN_XML_INACTIVE
+/** + * 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 ? Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|