[libvirt] [PATCH 1/2] Add VIR_TYPED_PARAM_STRING
--- daemon/remote.c | 15 +++++++++++++++ include/libvirt/libvirt.h.in | 4 +++- src/remote/remote_driver.c | 15 +++++++++++++++ src/remote/remote_protocol.x | 2 ++ 4 files changed, 35 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 0f088c6..afceab7 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -615,6 +615,13 @@ remoteSerializeTypedParameters(virTypedParameterPtr params, case VIR_TYPED_PARAM_BOOLEAN: val[i].value.remote_typed_param_value_u.b = params[i].value.b; break; + case VIR_TYPED_PARAM_STRING: + val[i].value.remote_typed_param_value_u.s = strdup(params[i].value.s); + if (val[i].value.remote_typed_param_value_u.s == NULL) { + virReportOOMError(); + goto cleanup; + } + break; default: virNetError(VIR_ERR_RPC, _("unknown parameter type: %d"), params[i].type); @@ -693,6 +700,14 @@ remoteDeserializeTypedParameters(remote_typed_param *args_params_val, params[i].value.b = args_params_val[i].value.remote_typed_param_value_u.b; break; + case VIR_TYPED_PARAM_STRING: + params[i].value.s = + strdup(args_params_val[i].value.remote_typed_param_value_u.s); + if (params[i].value.s == NULL) { + virReportOOMError(); + goto cleanup; + } + break; default: virNetError(VIR_ERR_INTERNAL_ERROR, _("unknown parameter type: %d"), params[i].type); diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 53a2f7d..da2080e 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -481,7 +481,8 @@ typedef enum { VIR_TYPED_PARAM_LLONG = 3, /* long long case */ VIR_TYPED_PARAM_ULLONG = 4, /* unsigned long long case */ VIR_TYPED_PARAM_DOUBLE = 5, /* double case */ - VIR_TYPED_PARAM_BOOLEAN = 6 /* boolean(character) case */ + VIR_TYPED_PARAM_BOOLEAN = 6, /* boolean(character) case */ + VIR_TYPED_PARAM_STRING = 7 /* string case */ } virTypedParameterType; /** @@ -512,6 +513,7 @@ struct _virTypedParameter { unsigned long long int ul; /* type is ULLONG */ double d; /* type is DOUBLE */ char b; /* type is BOOLEAN */ + char *s; /* type is STRING */ } value; /* parameter value */ }; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 603d589..8f9d09c 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1276,6 +1276,13 @@ remoteSerializeTypedParameters(virTypedParameterPtr params, case VIR_TYPED_PARAM_BOOLEAN: val[i].value.remote_typed_param_value_u.b = params[i].value.b; break; + case VIR_TYPED_PARAM_STRING: + val[i].value.remote_typed_param_value_u.s = strdup(params[i].value.s); + if (val[i].value.remote_typed_param_value_u.s == NULL) { + virReportOOMError(); + goto cleanup; + } + break; default: remoteError(VIR_ERR_RPC, _("unknown parameter type: %d"), params[i].type); @@ -1347,6 +1354,14 @@ remoteDeserializeTypedParameters(remote_typed_param *ret_params_val, params[i].value.b = ret_params_val[i].value.remote_typed_param_value_u.b; break; + case VIR_TYPED_PARAM_STRING: + params[i].value.s = + strdup(ret_params_val[i].value.remote_typed_param_value_u.s); + if (params[i].value.s == NULL) { + virReportOOMError(); + goto cleanup; + } + break; default: remoteError(VIR_ERR_RPC, _("unknown parameter type: %d"), params[i].type); diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 8f68808..c22a566 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -314,6 +314,8 @@ union remote_typed_param_value switch (int type) { double d; case VIR_TYPED_PARAM_BOOLEAN: int b; + case VIR_TYPED_PARAM_STRING: + remote_nonnull_string s; }; struct remote_typed_param { -- 1.7.3.1
This patch adds a parameter --weight-device to virsh command blkiotune for setting/getting blkio.weight_device. --- include/libvirt/libvirt.h.in | 9 ++ src/conf/domain_conf.c | 133 ++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 15 ++++ src/libvirt_private.syms | 1 + src/qemu/qemu_cgroup.c | 22 ++++++ src/qemu/qemu_driver.c | 171 +++++++++++++++++++++++++++++++++++++++++- src/util/cgroup.c | 33 ++++++++ src/util/cgroup.h | 3 + tools/virsh.c | 31 ++++++++ tools/virsh.pod | 5 +- 10 files changed, 417 insertions(+), 6 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index da2080e..5d5bc8d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1042,6 +1042,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 44212cf..d972a1b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -520,6 +520,101 @@ 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; +} + +/** + * virDomainBlkioWeightDeviceParseXML + * + * this function parses a XML node: + * + * <device> + * <major>major</major> + * <minor>minor</minor> + * <weight>weight</weight> + * </device> + * + * and fills a virBlkioWeightDevice struct. + */ +static int virDomainBlkioWeightDeviceParseXML(xmlNodePtr root, + virBlkioWeightDevicePtr dw) +{ + xmlNodePtr node; + + if (!dw) + return -1; + + node = root->children; + while (node) { + if (node->type == XML_ELEMENT_NODE) { + if (xmlStrEqual(node->name, BAD_CAST "major")) { + dw->major = atoi((const char *)xmlNodeGetContent(node)); + } else if (xmlStrEqual(node->name, BAD_CAST "minor")) { + dw->minor = atoi((const char *)xmlNodeGetContent(node)); + } else if (xmlStrEqual(node->name, BAD_CAST "weight")) { + dw->weight = atoi((const char *)xmlNodeGetContent(node)); + } + } + node = node->next; + } + + return 0; +} + + + static void virDomainObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED) { @@ -6070,6 +6165,20 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, &def->blkio.weight) < 0) def->blkio.weight = 0; + n = virXPathNodeSet("./blkiotune/device", ctxt, &nodes); + if (n > 0) { + if (VIR_ALLOC_N(def->blkio.weight_devices, n) < 0) { + virReportOOMError(); + goto error; + } + + for (i = 0; i < n; i++) { + virDomainBlkioWeightDeviceParseXML(nodes[i], &def->blkio.weight_devices[i]); + } + def->blkio.ndevices = n; + VIR_FREE(nodes); + } + /* Extract other memory tunables */ if (virXPathULong("string(./memtune/hard_limit)", ctxt, &def->mem.hard_limit) < 0) @@ -9948,10 +10057,28 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->mem.cur_balloon); /* add blkiotune only if there are any */ - if (def->blkio.weight) { + if (def->blkio.weight || def->blkio.weight_devices) { virBufferAsprintf(&buf, " <blkiotune>\n"); - virBufferAsprintf(&buf, " <weight>%u</weight>\n", - def->blkio.weight); + + if (def->blkio.weight) + virBufferAsprintf(&buf, " <weight>%u</weight>\n", + def->blkio.weight); + + if (def->blkio.weight_devices) { + int i; + + for (i = 0; i < def->blkio.ndevices; i++) { + virBufferAsprintf(&buf, " <device>\n"); + virBufferAsprintf(&buf, " <major>%d</major>\n", + def->blkio.weight_devices[i].major); + virBufferAsprintf(&buf, " <minor>%d</minor>\n", + def->blkio.weight_devices[i].minor); + virBufferAsprintf(&buf, " <weight>%d</weight>\n", + def->blkio.weight_devices[i].weight); + virBufferAsprintf(&buf, " </device>\n"); + } + } + virBufferAsprintf(&buf, " </blkiotune>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8382d28..e142649 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1174,6 +1174,19 @@ struct _virDomainNumatuneDef { /* Future NUMA tuning related stuff should go here. */ }; +typedef struct _virBlkioWeightDevice virBlkioWeightDevice; +typedef virBlkioWeightDevice *virBlkioWeightDevicePtr; +struct _virBlkioWeightDevice { + int major; + int minor; + int weight; +}; + +int virBlkioWeightDeviceToStr(virBlkioWeightDevicePtr deviceWeights, + int ndevices, + char **result); + + /* * Guest VM main configuration * @@ -1191,6 +1204,8 @@ struct _virDomainDef { struct { unsigned int weight; + int ndevices; + virBlkioWeightDevicePtr weight_devices; } blkio; struct { diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2a453bc..6dd2593 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -226,6 +226,7 @@ virDomainAuditVcpu; # domain_conf.h +virBlkioWeightDeviceToStr; virDiskNameToBusDeviceIndex; virDiskNameToIndex; virDomainActualNetDefFree; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 2a10bd2..eb06871 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -312,6 +312,28 @@ int qemuSetupCgroup(struct qemud_driver *driver, } } + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { + for (i = 0; i < vm->def->blkio.ndevices; i++) { + char *tmp = NULL; + + virAsprintf(&tmp, "%d:%d %d", + vm->def->blkio.weight_devices[i].major, + vm->def->blkio.weight_devices[i].minor, + vm->def->blkio.weight_devices[i].weight); + if (tmp) { + rc = virCgroupSetBlkioWeightDevice(cgroup, tmp); + VIR_FREE(tmp); + tmp = NULL; + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to set io device weight for domain %s"), + vm->def->name); + goto cleanup; + } + } + } + } + if (vm->def->mem.hard_limit != 0 || vm->def->mem.soft_limit != 0 || vm->def->mem.swap_hard_limit != 0) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f21122d..23d96e0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -44,6 +44,7 @@ #include <sys/ioctl.h> #include <sys/un.h> #include <byteswap.h> +#include <ctype.h> #include "qemu_driver.h" @@ -111,7 +112,7 @@ # define KVM_CAP_NR_VCPUS 9 /* returns max vcpus per vm */ #endif -#define QEMU_NB_BLKIO_PARAM 1 +#define QEMU_NB_BLKIO_PARAM 2 static void processWatchdogEvent(void *data, void *opaque); @@ -5601,6 +5602,86 @@ cleanup: return ret; } +/* weightDeviceStr in the form of major:minor,weight;nmajor:minor,weight + * for example, 8:0,300;8:16,800;8:32,1000 + */ +static int parseBlkioWeightDeviceStr(const char *weightDeviceStr, virBlkioWeightDevicePtr *dw, int *size) +{ + const char *temp; + int nDevice = 0; + int i; + virBlkioWeightDevicePtr result = NULL; + + if (!dw) + return -1; + if (*dw) + return -1; + + temp = weightDeviceStr; + while (temp) { + temp = strchr(temp, ';'); + if (temp) { + temp++; + if (*temp == '\0') + break; + nDevice++; + } + } + nDevice++; + + if (VIR_ALLOC_N(result, nDevice) < 0) { + virReportOOMError(); + return -1; + } + + i = 0; + temp = weightDeviceStr; + while (temp && i < nDevice) { + const char *p = temp; + while (isdigit(*p) && ++p); + if (!p || *p != ':') + goto fail; + + result[i].major = atoi(temp); + + temp = p + 1; + if (!temp) + goto fail; + + p = temp; + while (isdigit(*p) && ++p); + if (!p || *p != ',') + goto fail; + + result[i].minor = atoi(temp); + + temp = p + 1; + if (!temp) + goto fail; + + p = temp; + while (isdigit(*p) && ++p); + if (!p || (*p != ';' && *p != '\0')) + goto fail; + + result[i].weight = atoi(temp); + + i++; + if (*p == '\0') + break; + temp = p + 1; + } + + *dw = result; + *size = i; + + return 0; + +fail: + VIR_FREE(result); + return -1; +} + static int qemuDomainSetBlkioParameters(virDomainPtr dom, virTypedParameterPtr params, int nparams, @@ -5667,10 +5748,10 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, ret = 0; if (flags & VIR_DOMAIN_AFFECT_LIVE) { for (i = 0; i < nparams; i++) { + int rc; virTypedParameterPtr param = ¶ms[i]; if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) { - int rc; if (param->type != VIR_TYPED_PARAM_UINT) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", _("invalid type for blkio weight tunable, expected a 'unsigned int'")); @@ -5691,6 +5772,43 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, _("unable to set blkio weight tunable")); ret = -1; } + } else if(STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT_DEVICE)) { + int ndevices; + virBlkioWeightDevicePtr tmp = NULL; + if (param->type != VIR_TYPED_PARAM_STRING) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for blkio weight_device tunable, expected a 'char *'")); + ret = -1; + continue; + } + + if (parseBlkioWeightDeviceStr(params[i].value.s, + &tmp, + &ndevices) < 0) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid format for blkio weight_device")); + ret = -1; + continue; + } + for (i = 0; i < ndevices; i++) { + char *weight_device = NULL; + + virAsprintf(&weight_device, "%d:%d %d", + tmp[i].major, + tmp[i].minor, + tmp[i].weight); + if (weight_device) { + rc = virCgroupSetBlkioWeightDevice(group, weight_device); + VIR_FREE(weight_device); + weight_device = NULL; + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to set blkio weight_device tunable")); + ret = -1; + break; + } + } + } } else { qemuReportError(VIR_ERR_INVALID_ARG, _("Parameter `%s' not supported"), param->field); @@ -5720,6 +5838,21 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, } persistentDef->blkio.weight = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT_DEVICE)) { + virBlkioWeightDevicePtr tmp = NULL; + int ndevices; + if (parseBlkioWeightDeviceStr(params[i].value.s, + &tmp, + &ndevices) < 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("invalid device weight format: %s"), + params[i].value.s); + ret = -1; + continue; + } + VIR_FREE(persistentDef->blkio.weight_devices); + persistentDef->blkio.weight_devices = tmp; + persistentDef->blkio.ndevices = ndevices; } else { qemuReportError(VIR_ERR_INVALID_ARG, _("Parameter `%s' not supported"), param->field); @@ -5746,6 +5879,7 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom, { struct qemud_driver *driver = dom->conn->privateData; int i; + char *weight_device; virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; @@ -5839,6 +5973,23 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom, } param->value.ui = val; break; + case 1: /* blkio.weight_device */ + { + param->type = VIR_TYPED_PARAM_STRING; + rc = virCgroupGetBlkioWeightDevice(group, &weight_device); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to get blkio weight_device")); + goto cleanup; + } + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLKIO_WEIGHT_DEVICE) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field blkio weight_device too long for destination")); + goto cleanup; + } + param->value.s = weight_device; + break; + } default: break; @@ -5862,6 +6013,22 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom, param->value.ui = persistentDef->blkio.weight; break; + case 1: /* blkio.weight_device */ + { + if (virBlkioWeightDeviceToStr(persistentDef->blkio.weight_devices, + persistentDef->blkio.ndevices, + ¶m->value.s) < 0) + goto cleanup; + param->type = VIR_TYPED_PARAM_STRING; + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLKIO_WEIGHT_DEVICE) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field blkio weight_device too long for destination")); + goto cleanup; + } + + break; + } + default: break; /* should not hit here */ diff --git a/src/util/cgroup.c b/src/util/cgroup.c index f3ec80f..e29c964 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -977,6 +977,39 @@ int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight) } /** + * virCgroupSetBlkioWeightDevice: + * + * @group: The cgroup to change io weight device for + * @weight_device: The Weight device for this cgroup + * + * Returns: 0 on success + */ +int virCgroupSetBlkioWeightDevice(virCgroupPtr group, + const char *weight_device) +{ + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "blkio.weight_device", + weight_device); +} + +/** + * virCgroupGetBlkioWeightDevice: + * + * @group: The cgroup to get weight_device for + * @weight_device: returned weight_device string + * + * Returns: 0 on success + */ +int virCgroupGetBlkioWeightDevice(virCgroupPtr group, + char **weight_device) +{ + return virCgroupGetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "blkio.weight_device", weight_device); +} + +/** * virCgroupSetMemory: * * @group: The cgroup to change memory for diff --git a/src/util/cgroup.h b/src/util/cgroup.h index d190bb3..87e196b 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -55,6 +55,9 @@ int virCgroupAddTask(virCgroupPtr group, pid_t pid); int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight); int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight); +int virCgroupSetBlkioWeightDevice(virCgroupPtr group, const char *weight_device); +int virCgroupGetBlkioWeightDevice(virCgroupPtr group, char **weight_device); + int virCgroupSetMemory(virCgroupPtr group, unsigned long long kb); int virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb); diff --git a/tools/virsh.c b/tools/virsh.c index 15b9bdd..3ed8e70 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -3983,6 +3983,8 @@ static const vshCmdOptDef opts_blkiotune[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, {"weight", VSH_OT_INT, VSH_OFLAG_NONE, N_("IO Weight in range [100, 1000]")}, + {"weight-device", VSH_OT_STRING, VSH_OFLAG_NONE, + N_("per device IO Weight, in the form of major:minor,weight")}, {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, {"current", VSH_OT_BOOL, 0, N_("affect current domain")}, @@ -3993,6 +3995,7 @@ static bool cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) { virDomainPtr dom; + const char *weight_device = NULL; int weight = 0; int nparams = 0; int rv = 0; @@ -4037,6 +4040,16 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) } } + rv = vshCommandOptString(cmd, "weight-device", &weight_device); + if (rv < 0) { + vshError(ctl, "%s", + _("Unable to parse string parameter")); + goto cleanup; + } + if (rv > 0) { + nparams++; + } + if (nparams == 0) { /* get the number of blkio parameters */ if (virDomainGetBlkioParameters(dom, NULL, &nparams, flags) != 0) { @@ -4084,6 +4097,10 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) vshPrint(ctl, "%-15s: %d\n", params[i].field, params[i].value.b); break; + case VIR_TYPED_PARAM_STRING: + vshPrint(ctl, "%-15s: %s\n", params[i].field, + params[i].value.s); + break; default: vshPrint(ctl, "unimplemented blkio parameter type\n"); } @@ -4104,6 +4121,20 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) sizeof(temp->field)); weight = 0; } + + if (weight_device) { + int len = strlen(weight_device) + 1; + temp->type = VIR_TYPED_PARAM_STRING; + temp->value.s = vshMalloc(ctl, len); + if (!temp->value.s) { + goto cleanup; + } + strncpy(temp->value.s, weight_device, strlen(weight_device)); + temp->value.s[len-1] = '\0'; + weight_device = NULL; + strncpy(temp->field, VIR_DOMAIN_BLKIO_WEIGHT_DEVICE, + sizeof(temp->field)); + } } if (virDomainSetBlkioParameters(dom, params, nparams, flags) != 0) vshError(ctl, "%s", _("Unable to change blkio parameters")); diff --git a/tools/virsh.pod b/tools/virsh.pod index 81d7a1e..75bf24e 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -835,12 +835,15 @@ value are kilobytes (i.e. blocks of 1024 bytes). =back -=item B<blkiotune> I<domain-id> [I<--weight> B<weight>] [[I<--config>] +=item B<blkiotune> I<domain-id> [I<--weight> B<weight>] +[I<--weight-device> B<weight-device>] [[I<--config>] [I<--live>] | [I<--current>]] Display or set the blkio parameters. QEMU/KVM supports I<--weight>. I<--weight> is in range [100, 1000]. +B<weight-device> is in the format of major:minor,weight;major:minor,weight. + If I<--live> is specified, affect a running guest. If I<--config> is specified, affect the next boot of a persistent guest. If I<--current> is specified, affect the current guest state. -- 1.7.3.1
On Tue, Sep 06, 2011 at 06:18:40PM +0800, Hu Tao wrote:
--- daemon/remote.c | 15 +++++++++++++++ include/libvirt/libvirt.h.in | 4 +++- src/remote/remote_driver.c | 15 +++++++++++++++ src/remote/remote_protocol.x | 2 ++ 4 files changed, 35 insertions(+), 1 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 0f088c6..afceab7 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -615,6 +615,13 @@ remoteSerializeTypedParameters(virTypedParameterPtr params, case VIR_TYPED_PARAM_BOOLEAN: val[i].value.remote_typed_param_value_u.b = params[i].value.b; break; + case VIR_TYPED_PARAM_STRING: + val[i].value.remote_typed_param_value_u.s = strdup(params[i].value.s); + if (val[i].value.remote_typed_param_value_u.s == NULL) { + virReportOOMError(); + goto cleanup; + } + break; default: virNetError(VIR_ERR_RPC, _("unknown parameter type: %d"), params[i].type); @@ -693,6 +700,14 @@ remoteDeserializeTypedParameters(remote_typed_param *args_params_val, params[i].value.b = args_params_val[i].value.remote_typed_param_value_u.b; break; + case VIR_TYPED_PARAM_STRING: + params[i].value.s = + strdup(args_params_val[i].value.remote_typed_param_value_u.s); + if (params[i].value.s == NULL) { + virReportOOMError(); + goto cleanup; + } + break; default: virNetError(VIR_ERR_INTERNAL_ERROR, _("unknown parameter type: %d"), params[i].type); diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 53a2f7d..da2080e 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -481,7 +481,8 @@ typedef enum { VIR_TYPED_PARAM_LLONG = 3, /* long long case */ VIR_TYPED_PARAM_ULLONG = 4, /* unsigned long long case */ VIR_TYPED_PARAM_DOUBLE = 5, /* double case */ - VIR_TYPED_PARAM_BOOLEAN = 6 /* boolean(character) case */ + VIR_TYPED_PARAM_BOOLEAN = 6, /* boolean(character) case */ + VIR_TYPED_PARAM_STRING = 7 /* string case */ } virTypedParameterType;
/** @@ -512,6 +513,7 @@ struct _virTypedParameter { unsigned long long int ul; /* type is ULLONG */ double d; /* type is DOUBLE */ char b; /* type is BOOLEAN */ + char *s; /* type is STRING */ } value; /* parameter value */ };
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 603d589..8f9d09c 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1276,6 +1276,13 @@ remoteSerializeTypedParameters(virTypedParameterPtr params, case VIR_TYPED_PARAM_BOOLEAN: val[i].value.remote_typed_param_value_u.b = params[i].value.b; break; + case VIR_TYPED_PARAM_STRING: + val[i].value.remote_typed_param_value_u.s = strdup(params[i].value.s); + if (val[i].value.remote_typed_param_value_u.s == NULL) { + virReportOOMError(); + goto cleanup; + } + break; default: remoteError(VIR_ERR_RPC, _("unknown parameter type: %d"), params[i].type); @@ -1347,6 +1354,14 @@ remoteDeserializeTypedParameters(remote_typed_param *ret_params_val, params[i].value.b = ret_params_val[i].value.remote_typed_param_value_u.b; break; + case VIR_TYPED_PARAM_STRING: + params[i].value.s = + strdup(ret_params_val[i].value.remote_typed_param_value_u.s); + if (params[i].value.s == NULL) { + virReportOOMError(); + goto cleanup; + } + break; default: remoteError(VIR_ERR_RPC, _("unknown parameter type: %d"), params[i].type); diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 8f68808..c22a566 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -314,6 +314,8 @@ union remote_typed_param_value switch (int type) { double d; case VIR_TYPED_PARAM_BOOLEAN: int b; + case VIR_TYPED_PARAM_STRING: + remote_nonnull_string s; };
struct remote_typed_param {
Ohhh, good point and better to add this before the first official release to avoid possible ABI breakages in the future if we need it ! My only concern is remote actually working, we don't have any API yet exercizing this (I mean string values) and I wonder how to test this, In any case, ACK, Wen would you mind pushing this ? thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 09/06/2011 04:44 AM, Daniel Veillard wrote:
On Tue, Sep 06, 2011 at 06:18:40PM +0800, Hu Tao wrote:
--- daemon/remote.c | 15 +++++++++++++++ include/libvirt/libvirt.h.in | 4 +++- src/remote/remote_driver.c | 15 +++++++++++++++ src/remote/remote_protocol.x | 2 ++ 4 files changed, 35 insertions(+), 1 deletions(-)
+++ b/include/libvirt/libvirt.h.in @@ -481,7 +481,8 @@ typedef enum { VIR_TYPED_PARAM_LLONG = 3, /* long long case */ VIR_TYPED_PARAM_ULLONG = 4, /* unsigned long long case */ VIR_TYPED_PARAM_DOUBLE = 5, /* double case */ - VIR_TYPED_PARAM_BOOLEAN = 6 /* boolean(character) case */ + VIR_TYPED_PARAM_BOOLEAN = 6, /* boolean(character) case */ + VIR_TYPED_PARAM_STRING = 7 /* string case */
New enum value is probably okay,
} virTypedParameterType;
/** @@ -512,6 +513,7 @@ struct _virTypedParameter { unsigned long long int ul; /* type is ULLONG */ double d; /* type is DOUBLE */ char b; /* type is BOOLEAN */ + char *s; /* type is STRING */
and new char * field to a union doesn't change the struct size,
+++ b/src/remote/remote_protocol.x @@ -314,6 +314,8 @@ union remote_typed_param_value switch (int type) { double d; case VIR_TYPED_PARAM_BOOLEAN: int b; + case VIR_TYPED_PARAM_STRING: + remote_nonnull_string s;
but I'm quite worried about the on-the-wire compatibility aspect of this change. If a new server sends the new enum value and a string, will the old server that does not know that enum value properly reject the rpc call, or will it cause a crash or other bad things to happen?
};
struct remote_typed_param {
Ohhh, good point and better to add this before the first official release to avoid possible ABI breakages in the future if we need it !
Huh? remote_typed_param has already had an official release. The addition is to a union, but this really is a case of modifying an existing on-the-wire layout, so I hope that the fact that it was an enumerated union with a new enumeration value is what is making this safe. Also, before you push this, please install pdwtags (from the dwarves package if you use Fedora), and fix src/remote_protocol-structs so that 'make check' passes. Actually, please post that diff before pushing the patch - I want to verify that the size of remote_typed_param doesn't change; if it does change, then we have an incompatible change, and need to think of a different approach. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Tue, Sep 06, 2011 at 07:09:53AM -0600, Eric Blake wrote:
On 09/06/2011 04:44 AM, Daniel Veillard wrote:
On Tue, Sep 06, 2011 at 06:18:40PM +0800, Hu Tao wrote:
--- daemon/remote.c | 15 +++++++++++++++ include/libvirt/libvirt.h.in | 4 +++- src/remote/remote_driver.c | 15 +++++++++++++++ src/remote/remote_protocol.x | 2 ++ 4 files changed, 35 insertions(+), 1 deletions(-)
+++ b/include/libvirt/libvirt.h.in @@ -481,7 +481,8 @@ typedef enum { VIR_TYPED_PARAM_LLONG = 3, /* long long case */ VIR_TYPED_PARAM_ULLONG = 4, /* unsigned long long case */ VIR_TYPED_PARAM_DOUBLE = 5, /* double case */ - VIR_TYPED_PARAM_BOOLEAN = 6 /* boolean(character) case */ + VIR_TYPED_PARAM_BOOLEAN = 6, /* boolean(character) case */ + VIR_TYPED_PARAM_STRING = 7 /* string case */
New enum value is probably okay,
} virTypedParameterType;
/** @@ -512,6 +513,7 @@ struct _virTypedParameter { unsigned long long int ul; /* type is ULLONG */ double d; /* type is DOUBLE */ char b; /* type is BOOLEAN */ + char *s; /* type is STRING */
and new char * field to a union doesn't change the struct size,
Well on an hypothetic arch with 128 bits pointers, that would have broken the ABI since none of the existing type in the union are more than 64 bits, but right in this case I think we are safe but let's check this.
+++ b/src/remote/remote_protocol.x @@ -314,6 +314,8 @@ union remote_typed_param_value switch (int type) { double d; case VIR_TYPED_PARAM_BOOLEAN: int b; + case VIR_TYPED_PARAM_STRING: + remote_nonnull_string s;
but I'm quite worried about the on-the-wire compatibility aspect of this change. If a new server sends the new enum value and a string, will the old server that does not know that enum value properly reject the rpc call, or will it cause a crash or other bad things to happen?
yes as I said we need some testing, but we have no API yet making use of strings.
};
struct remote_typed_param {
Ohhh, good point and better to add this before the first official release to avoid possible ABI breakages in the future if we need it !
Huh? remote_typed_param has already had an official release. The
Yeah I got confused, I had checked on what I though was a 0.9.4 checkout but I made a mistake...
addition is to a union, but this really is a case of modifying an existing on-the-wire layout, so I hope that the fact that it was an enumerated union with a new enumeration value is what is making this safe.
Also, before you push this, please install pdwtags (from the dwarves package if you use Fedora), and fix src/remote_protocol-structs so that 'make check' passes. Actually, please post that diff before pushing the patch - I want to verify that the size of remote_typed_param doesn't change; if it does change, then we have an incompatible change, and need to think of a different approach.
Agreed, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Tue, Sep 06, 2011 at 07:09:53AM -0600, Eric Blake wrote:
On 09/06/2011 04:44 AM, Daniel Veillard wrote:
On Tue, Sep 06, 2011 at 06:18:40PM +0800, Hu Tao wrote:
--- daemon/remote.c | 15 +++++++++++++++ include/libvirt/libvirt.h.in | 4 +++- src/remote/remote_driver.c | 15 +++++++++++++++ src/remote/remote_protocol.x | 2 ++ 4 files changed, 35 insertions(+), 1 deletions(-)
+++ b/include/libvirt/libvirt.h.in @@ -481,7 +481,8 @@ typedef enum { VIR_TYPED_PARAM_LLONG = 3, /* long long case */ VIR_TYPED_PARAM_ULLONG = 4, /* unsigned long long case */ VIR_TYPED_PARAM_DOUBLE = 5, /* double case */ - VIR_TYPED_PARAM_BOOLEAN = 6 /* boolean(character) case */ + VIR_TYPED_PARAM_BOOLEAN = 6, /* boolean(character) case */ + VIR_TYPED_PARAM_STRING = 7 /* string case */
New enum value is probably okay,
} virTypedParameterType;
/** @@ -512,6 +513,7 @@ struct _virTypedParameter { unsigned long long int ul; /* type is ULLONG */ double d; /* type is DOUBLE */ char b; /* type is BOOLEAN */ + char *s; /* type is STRING */
and new char * field to a union doesn't change the struct size,
+++ b/src/remote/remote_protocol.x @@ -314,6 +314,8 @@ union remote_typed_param_value switch (int type) { double d; case VIR_TYPED_PARAM_BOOLEAN: int b; + case VIR_TYPED_PARAM_STRING: + remote_nonnull_string s;
but I'm quite worried about the on-the-wire compatibility aspect of this change. If a new server sends the new enum value and a string, will the old server that does not know that enum value properly reject the rpc call, or will it cause a crash or other bad things to happen?
If a new server sends a string parameter to an old client, you will get a fatal error on the client decoding the XDR data. This will cause libvirt to report an XDR decoding error. It (probably) isn't fatal, since we've read the entire packet off the wire the next RPC call should still work. It is still not too pleasant though since old virsh will not work with new libvirtd IIUC, so I don't think we can do this without getting a better compat story here which does not require changing existing apps like virsh. 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 :|
On 09/12/2011 09:18 AM, Daniel P. Berrange wrote:
but I'm quite worried about the on-the-wire compatibility aspect of this change. If a new server sends the new enum value and a string, will the old server that does not know that enum value properly reject the rpc call, or will it cause a crash or other bad things to happen?
If a new server sends a string parameter to an old client, you will get a fatal error on the client decoding the XDR data. This will cause libvirt to report an XDR decoding error. It (probably) isn't fatal, since we've read the entire packet off the wire the next RPC call should still work. It is still not too pleasant though since old virsh will not work with new libvirtd IIUC, so I don't think we can do this without getting a better compat story here which does not require changing existing apps like virsh.
I agree that things are not fatal, but still not desirable (the old virsh makes a query but can't get a response, because the new server sends a string type back that the old virsh has no chance of understanding). But I think we can solve this by use of a flag (we _do_ have an unsigned int flags argument at our disposal, right?): virDomainFoo(dom, typed_param_array, len, 0) means "give me back _only_ array values that older libvirt can understand - no strings" virDomainFoo(dom, typed_param_array, len, VIR_DOMAIN_FOO_TYPED_STRING_OKAY) means "I'm a newer client, and understand strings if you send them". Then newer virsh can be coded to auto-try the new flag value, then fall back to 0 flags if the flag value was rejected as unknown by older server; this means that older virsh will not get back string data from newer servers, but you avoid the issue of erroring out on new information without being able to get at the old information. This covers both older->newer and newer->older communications, and means that only newer->newer with the new flag can take advantage of the new string parameter. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Eric Blake -
Hu Tao