[libvirt] [PATCH 3/8] Implement virDomain{Set, Get}BlockIoTune for the qemu driver
Implement the block I/O throttle setting and getting support to qemu driver. Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 342 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 37 +++++ src/qemu/qemu_monitor.h | 22 +++ src/qemu/qemu_monitor_json.c | 186 +++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 10 ++ src/qemu/qemu_monitor_text.c | 165 ++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 10 ++ 7 files changed, 772 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5f4a18d..0a4eaa9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -94,6 +94,8 @@ #define QEMU_NB_MEM_PARAM 3 +#define QEMU_NB_BLKIOTHROTTLE_PARAM 6 + #if HAVE_LINUX_KVM_H # include <linux/kvm.h> #endif @@ -10775,6 +10777,344 @@ cleanup: return ret; } +static int +qemuDomainSetBlockIoTune(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + qemuDomainObjPrivatePtr priv; + virDomainDefPtr persistentDef = NULL; + virDomainBlockIoTuneInfo info; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + const char *device = NULL; + int ret = -1; + int i; + bool isActive; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + memset(&info, 0, sizeof(info)); + + qemuDriverLock(driver); + virUUIDFormat(dom->uuid, uuidstr); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + if (!vm) { + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + device = qemuDiskPathToAlias(vm, disk); + if (!device) { + goto cleanup; + } + + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + isActive = virDomainObjIsActive(vm); + + if (flags == VIR_DOMAIN_AFFECT_CURRENT) { + if (isActive) + flags = VIR_DOMAIN_AFFECT_LIVE; + else + flags = VIR_DOMAIN_AFFECT_CONFIG; + } + + if (!isActive && (flags & VIR_DOMAIN_AFFECT_LIVE)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is not running")); + goto endjob; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (!vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change persistent config of a transient domain")); + goto endjob; + } + if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) + goto endjob; + } + + for (i = 0; i < nparams; i++) { + virTypedParameterPtr param = ¶ms[i]; + + if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC)) { + + info.total_bytes_sec = params[i].value.ul; + } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC)) { + info.read_bytes_sec = params[i].value.ul; + } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC)) { + info.write_bytes_sec = params[i].value.ul; + } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC)) { + info.total_iops_sec = params[i].value.ul; + } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC)) { + info.read_iops_sec = params[i].value.ul; + } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC)) { + info.write_iops_sec = params[i].value.ul; + } else { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Unrecognized parameter")); + goto endjob; + } + } + + if ((info.total_bytes_sec && info.read_bytes_sec) || + (info.total_bytes_sec && info.write_bytes_sec)) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("total and read/write of bytes_sec cannot be set at the same time")); + goto endjob; + } + + if ((info.total_iops_sec && info.read_iops_sec) || + (info.total_iops_sec && info.write_iops_sec)) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("total and read/write of iops_sec cannot be set at the same time")); + goto endjob; + } + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + priv = vm->privateData; + qemuDomainObjEnterMonitorWithDriver(driver, vm); + ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info, 1); + qemuDomainObjExitMonitorWithDriver(driver, vm); + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + sa_assert(persistentDef); + int idx = virDomainDiskIndexByName(vm->def, disk, true); + if (i < 0) + goto endjob; + persistentDef->disks[idx]->blkdeviotune.total_bytes_sec = info.total_bytes_sec; + persistentDef->disks[idx]->blkdeviotune.read_bytes_sec = info.read_bytes_sec; + persistentDef->disks[idx]->blkdeviotune.write_bytes_sec = info.write_bytes_sec; + persistentDef->disks[idx]->blkdeviotune.total_iops_sec = info.total_iops_sec; + persistentDef->disks[idx]->blkdeviotune.read_iops_sec = info.read_iops_sec; + persistentDef->disks[idx]->blkdeviotune.write_iops_sec = info.write_iops_sec; + persistentDef->disks[idx]->blkdeviotune.mark = 1; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + ret = virDomainSaveConfig(driver->configDir, persistentDef); + if (ret < 0) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Write to config file failed")); + goto endjob; + } + } + +endjob: + if (qemuDomainObjEndJob(driver, vm) == 0) + vm = NULL; + +cleanup: + VIR_FREE(device); + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + +static int +qemuDomainGetBlockIoTune(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + qemuDomainObjPrivatePtr priv; + virDomainDefPtr persistentDef = NULL; + virDomainBlockIoTuneInfo reply; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + const char *device = NULL; + int ret = -1; + int i; + bool isActive; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG | + VIR_TYPED_PARAM_STRING_OKAY, -1); + + /* We don't return strings, and thus trivially support this flag. */ + flags &= ~VIR_TYPED_PARAM_STRING_OKAY; + + qemuDriverLock(driver); + virUUIDFormat(dom->uuid, uuidstr); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + if (!vm) { + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + device = qemuDiskPathToAlias(vm, disk); + + if (!device) { + goto cleanup; + } + + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + isActive = virDomainObjIsActive(vm); + + if (flags == VIR_DOMAIN_AFFECT_CURRENT) { + if (isActive) + flags = VIR_DOMAIN_AFFECT_LIVE; + else + flags = VIR_DOMAIN_AFFECT_CONFIG; + } + + if ((flags & VIR_DOMAIN_AFFECT_LIVE) && (flags & VIR_DOMAIN_AFFECT_CONFIG)) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("Cannot query with --live and --config together")); + goto endjob; + } + + if (!isActive && (flags & VIR_DOMAIN_AFFECT_LIVE)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is not running")); + goto endjob; + } + + if ((*nparams) == 0) { + /* Current number of parameters supported by QEMU Block I/O Throttling */ + *nparams = QEMU_NB_BLKIOTHROTTLE_PARAM; + ret = 0; + goto endjob; + } + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + priv = vm->privateData; + qemuDomainObjEnterMonitorWithDriver(driver, vm); + ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply, 0); + qemuDomainObjExitMonitorWithDriver(driver, vm); + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (!vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is transient")); + goto endjob; + } + if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) + goto endjob; + + sa_assert(persistentDef); + int idx = virDomainDiskIndexByName(vm->def, disk, true); + if (idx < 0) + goto endjob; + reply.total_bytes_sec = persistentDef->disks[idx]->blkdeviotune.total_bytes_sec; + reply.read_bytes_sec = persistentDef->disks[idx]->blkdeviotune.read_bytes_sec; + reply.write_bytes_sec = persistentDef->disks[idx]->blkdeviotune.write_bytes_sec; + reply.total_iops_sec = persistentDef->disks[idx]->blkdeviotune.total_iops_sec; + reply.read_iops_sec = persistentDef->disks[idx]->blkdeviotune.read_iops_sec; + reply.write_iops_sec = persistentDef->disks[idx]->blkdeviotune.write_iops_sec; + ret = 0; + } + + if ((ret != 0) && ((*nparams) < QEMU_NB_BLKIOTHROTTLE_PARAM)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Cannot get block I/O parameters")); + goto endjob; + } + + for (i = 0; i < QEMU_NB_BLKIOTHROTTLE_PARAM; i++) { + virTypedParameterPtr param = ¶ms[i]; + param->value.ul = 0; + param->type = VIR_TYPED_PARAM_ULLONG; + + switch(i) { + case 0: + if (virStrcpyStatic(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC); + goto endjob; + } + param->value.ul = reply.total_bytes_sec; + break; + + case 1: + if (virStrcpyStatic(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC); + goto endjob; + } + param->value.ul = reply.read_bytes_sec; + break; + + case 2: + if (virStrcpyStatic(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC); + goto endjob; + } + param->value.ul = reply.write_bytes_sec; + break; + + case 3: + if (virStrcpyStatic(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC); + goto endjob; + } + param->value.ul = reply.total_iops_sec; + break; + + case 4: + if (virStrcpyStatic(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC); + goto endjob; + } + param->value.ul = reply.read_iops_sec; + break; + + case 5: + if (virStrcpyStatic(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC); + goto endjob; + } + param->value.ul = reply.write_iops_sec; + break; + default: + break; + } + } + ret = 0; + +endjob: + if (qemuDomainObjEndJob(driver, vm) == 0) + vm = NULL; + +cleanup: + VIR_FREE(device); + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} static virDriver qemuDriver = { .no = VIR_DRV_QEMU, @@ -10919,6 +11259,8 @@ static virDriver qemuDriver = { .domainGetBlockJobInfo = qemuDomainGetBlockJobInfo, /* 0.9.4 */ .domainBlockJobSetSpeed = qemuDomainBlockJobSetSpeed, /* 0.9.4 */ .domainBlockPull = qemuDomainBlockPull, /* 0.9.4 */ + .domainSetBlockIoTune = qemuDomainSetBlockIoTune, /* 0.9.8 */ + .domainGetBlockIoTune = qemuDomainGetBlockIoTune, /* 0.9.8 */ }; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 73e5ea9..c0b711f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2567,6 +2567,43 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, return ret; } +int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, + const char *device, + virDomainBlockIoTuneInfoPtr info, + unsigned int flags) +{ + int ret; + + VIR_DEBUG("mon=%p, device=%p, info=%p, flags=%x", + mon, device, info, flags); + + if (mon->json) { + ret = qemuMonitorJSONSetBlockIoThrottle(mon, device, info, flags); + } else { + ret = qemuMonitorTextSetBlockIoThrottle(mon, device, info, flags); + } + return ret; +} + +int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon, + const char *device, + virDomainBlockIoTuneInfoPtr reply, + unsigned int flags) +{ + int ret; + + VIR_DEBUG("mon=%p, device=%p, reply=%p, flags=%x", + mon, device, reply, flags); + + if (mon->json) { + ret = qemuMonitorJSONGetBlockIoThrottle(mon, device, reply, flags); + } else { + ret = qemuMonitorTextGetBlockIoThrottle(mon, device, reply, flags); + } + return ret; +} + + int qemuMonitorVMStatusToPausedReason(const char *status) { int st; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 883e0aa..48c6db0 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -521,6 +521,28 @@ int qemuMonitorOpenGraphics(qemuMonitorPtr mon, const char *fdname, bool skipauth); + +typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo; +struct _virDomainBlockIoTuneInfo { + unsigned long long total_bytes_sec; + unsigned long long read_bytes_sec; + unsigned long long write_bytes_sec; + unsigned long long total_iops_sec; + unsigned long long read_iops_sec; + unsigned long long write_iops_sec; +}; +typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr; + +int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, + const char *device, + virDomainBlockIoTuneInfoPtr info, + unsigned int flags); + +int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon, + const char *device, + virDomainBlockIoTuneInfoPtr reply, + unsigned int flags); + /** * When running two dd process and using <> redirection, we need a * shell that will not truncate files. These two strings serve that diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 56a62db..925031e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3276,3 +3276,189 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon, virJSONValueFree(reply); return ret; } + + +static int +qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, + const char *device, + virDomainBlockIoTuneInfoPtr reply) +{ + virJSONValuePtr io_throttle; + int ret = -1; + int i; + int found = 0; + + io_throttle = virJSONValueObjectGet(result, "return"); + + if (!io_throttle || io_throttle->type != VIR_JSON_TYPE_ARRAY) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _(" block_io_throttle reply was missing device list ")); + goto cleanup; + } + + for (i = 0; i < virJSONValueArraySize(io_throttle); i++) { + virJSONValuePtr temp_dev = virJSONValueArrayGet(io_throttle, i); + virJSONValuePtr inserted; + const char *current_dev; + + if (!temp_dev || temp_dev->type != VIR_JSON_TYPE_OBJECT) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("block io throttle device entry was not in expected format")); + goto cleanup; + } + + if ((current_dev = virJSONValueObjectGetString(temp_dev, "device")) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("block io throttle device entry was not in expected format")); + goto cleanup; + } + + if(STRPREFIX(current_dev, QEMU_DRIVE_HOST_PREFIX)) + current_dev += strlen(QEMU_DRIVE_HOST_PREFIX); + + if (STREQ(current_dev, device)) + continue; + + found = 1; + if ((inserted = virJSONValueObjectGet(temp_dev, "inserted")) == NULL || + inserted->type != VIR_JSON_TYPE_OBJECT) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("block io throttle inserted entry was not in expected format")); + goto cleanup; + } + + if (virJSONValueObjectGetNumberUlong(inserted, "bps", &reply->total_bytes_sec) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s"), "total_bytes_sec"); + goto cleanup; + } + + if (virJSONValueObjectGetNumberUlong(inserted, "bps_rd", &reply->read_bytes_sec) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s"), "read_bytes_sec"); + goto cleanup; + } + + if (virJSONValueObjectGetNumberUlong(inserted, "bps_wr", &reply->write_bytes_sec) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s"), "write_bytes_sec"); + goto cleanup; + } + + if (virJSONValueObjectGetNumberUlong(inserted, "iops", &reply->total_iops_sec) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s"), "total_iops_sec"); + goto cleanup; + } + + if (virJSONValueObjectGetNumberUlong(inserted, "iops_rd", &reply->read_iops_sec) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s"), "read_iops_sec"); + goto cleanup; + } + + if (virJSONValueObjectGetNumberUlong(inserted, "iops_wr", &reply->write_iops_sec) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s"), "write_iops_sec"); + goto cleanup; + } + break; + } + + if (!found) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot found info for device '%s'"), + device); + goto cleanup; + } + ret = 0; + +cleanup: + return ret; +} + +int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, + const char *device, + virDomainBlockIoTuneInfoPtr info, + unsigned int flags) +{ + int ret = -1; + virJSONValuePtr cmd = NULL; + virJSONValuePtr result = NULL; + + if (flags) { + cmd = qemuMonitorJSONMakeCommand("block_set_io_throttle", + "s:device", device, + "U:bps", info->total_bytes_sec, + "U:bps_rd", info->read_bytes_sec, + "U:bps_wr", info->write_bytes_sec, + "U:iops", info->total_iops_sec, + "U:iops_rd", info->read_iops_sec, + "U:iops_wr", info->write_iops_sec, + NULL); + } + + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &result); + + if (ret == 0 && virJSONValueObjectHasKey(result, "error")) { + if (qemuMonitorJSONHasError(result, "DeviceNotActive")) + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("No active operation on device: %s"), device); + else if (qemuMonitorJSONHasError(result, "NotSupported")) + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Operation is not supported for device: %s"), device); + else + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unexpected error")); + ret = -1; + } + + virJSONValueFree(cmd); + virJSONValueFree(result); + return ret; +} + +int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, + const char *device, + virDomainBlockIoTuneInfoPtr reply, + unsigned int flags) +{ + int ret = -1; + virJSONValuePtr cmd = NULL; + virJSONValuePtr result = NULL; + + if (!flags) { + cmd = qemuMonitorJSONMakeCommand("query-block", + NULL); + } + + if (!cmd) { + return -1; + } + + ret = qemuMonitorJSONCommand(mon, cmd, &result); + + if (ret == 0 && virJSONValueObjectHasKey(result, "error")) { + if (qemuMonitorJSONHasError(result, "DeviceNotActive")) + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("No active operation on device: %s"), device); + else if (qemuMonitorJSONHasError(result, "NotSupported")) + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Operation is not supported for device: %s"), device); + else + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unexpected error")); + ret = -1; + } + + if (ret == 0 && !flags) + ret = qemuMonitorJSONBlockIoThrottleInfo(result, device, reply); + + virJSONValueFree(cmd); + virJSONValueFree(result); + return ret; +} + diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index f10d7d2..bf12dc5 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -255,4 +255,14 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon, const char *fdname, bool skipauth); +int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, + const char *device, + virDomainBlockIoTuneInfoPtr info, + unsigned int flags); + +int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, + const char *device, + virDomainBlockIoTuneInfoPtr reply, + unsigned int flags); + #endif /* QEMU_MONITOR_JSON_H */ diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 5de4d24..0ccc111 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -3429,3 +3429,168 @@ cleanup: VIR_FREE(cmd); return ret; } + + +int qemuMonitorTextSetBlockIoThrottle(qemuMonitorPtr mon, + const char *device, + virDomainBlockIoTuneInfoPtr info, + unsigned int flags) +{ + char *cmd = NULL; + char *result = NULL; + int ret = 0; + const char *cmd_name = NULL; + + /* For the not specified fields, 0 by default */ + if (flags) { + cmd_name = "block_set_io_throttle"; + ret = virAsprintf(&cmd, "%s %s %llu %llu %llu %llu %llu %llu", cmd_name, + device, info->total_bytes_sec, info->read_bytes_sec, + info->write_bytes_sec, info->total_iops_sec, + info->read_iops_sec, info->write_iops_sec); + } + + if (ret < 0) { + virReportOOMError(); + return -1; + } + + if (qemuMonitorHMPCommand(mon, cmd, &result) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot run monitor command")); + ret = -1; + goto cleanup; + } + + if (qemuMonitorTextCommandNotFound(cmd_name, result)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Command '%s' is not found"), cmd_name); + ret = -1; + goto cleanup; + } + +cleanup: + VIR_FREE(cmd); + VIR_FREE(result); + return ret; +} + +static int qemuMonitorTextParseBlockIoThrottle(const char *result, + const char *device, + virDomainBlockIoTuneInfoPtr reply) +{ + char *dummy = NULL; + int ret = -1; + const char *p, *eol; + int devnamelen = strlen(device); + + p = result; + + while (*p) { + if (STRPREFIX(p, QEMU_DRIVE_HOST_PREFIX)) + p += strlen(QEMU_DRIVE_HOST_PREFIX); + + if (STREQLEN(p, device, devnamelen) && + p[devnamelen] == ':' && p[devnamelen+1] == ' ') { + + eol = strchr(p, '\n'); + if (!eol) + eol = p + strlen(p); + + p += devnamelen + 2; /*Skip to first label. */ + + while (*p) { + if (STRPREFIX(p, "bps=")) { + p += strlen("bps="); + if (virStrToLong_ull(p, &dummy, 10, &reply->total_bytes_sec) == -1) + VIR_DEBUG("error reading total_bytes_sec: %s", p); + } else if (STRPREFIX(p, "bps_rd=")) { + p += strlen("bps_rd="); + if (virStrToLong_ull(p, &dummy, 10, &reply->read_bytes_sec) == -1) + VIR_DEBUG("error reading read_bytes_sec: %s", p); + } else if (STRPREFIX(p, "bps_wr=")) { + p += strlen("bps_wr="); + if (virStrToLong_ull(p, &dummy, 10, &reply->write_bytes_sec) == -1) + VIR_DEBUG("error reading write_bytes_sec: %s", p); + } else if (STRPREFIX(p, "iops=")) { + p += strlen("iops="); + if (virStrToLong_ull(p, &dummy, 10, &reply->total_iops_sec) == -1) + VIR_DEBUG("error reading total_iops_sec: %s", p); + } else if (STRPREFIX(p, "iops_rd=")) { + p += strlen("iops_rd="); + if (virStrToLong_ull(p, &dummy, 10, &reply->read_iops_sec) == -1) + VIR_DEBUG("error reading read_iops_sec: %s", p); + } else if (STRPREFIX(p, "iops_wr=")) { + p += strlen("iops_wr="); + if (virStrToLong_ull(p, &dummy, 10, &reply->write_iops_sec) == -1) + VIR_DEBUG("error reading write_iops_sec: %s", p); + } else { + VIR_DEBUG(" unknown block info %s", p); + } + + /* Skip to next label. */ + p = strchr (p, ' '); + if (!p || p >= eol) + break; + p++; + } + ret = 0; + goto cleanup; + } + + /* Skip to next line. */ + p = strchr (p, '\n'); + if (!p) + break; + p++; + } + + qemuReportError(VIR_ERR_INVALID_ARG, + _("No info for device '%s'"), device); + +cleanup: + return ret; +} + +int qemuMonitorTextGetBlockIoThrottle(qemuMonitorPtr mon, + const char *device, + virDomainBlockIoTuneInfoPtr reply, + unsigned int flags) +{ + char *cmd = NULL; + char *result = NULL; + int ret = 0; + const char *cmd_name = NULL; + + if (flags) { + cmd_name = "info block"; + ret = virAsprintf(&cmd, "%s", cmd_name); + } + + if (ret < 0) { + virReportOOMError(); + return -1; + } + + if (qemuMonitorHMPCommand(mon, cmd, &result) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot run monitor command")); + ret = -1; + goto cleanup; + } + + if (qemuMonitorTextCommandNotFound(cmd_name, result)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Command '%s' is not found"), cmd_name); + ret = -1; + goto cleanup; + } + + ret = qemuMonitorTextParseBlockIoThrottle(result, device, reply); + +cleanup: + VIR_FREE(cmd); + VIR_FREE(result); + return ret; +} + diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index f32fce0..1c47d39 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -248,4 +248,14 @@ int qemuMonitorTextOpenGraphics(qemuMonitorPtr mon, const char *fdname, bool skipauth); +int qemuMonitorTextSetBlockIoThrottle(qemuMonitorPtr mon, + const char *device, + virDomainBlockIoTuneInfoPtr info, + unsigned int flags); + +int qemuMonitorTextGetBlockIoThrottle(qemuMonitorPtr mon, + const char *device, + virDomainBlockIoTuneInfoPtr reply, + unsigned int flags); + #endif /* QEMU_MONITOR_TEXT_H */ -- 1.7.1
On 11/15/2011 02:02 AM, Lei Li wrote:
Implement the block I/O throttle setting and getting support to qemu driver.
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 342 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 37 +++++ src/qemu/qemu_monitor.h | 22 +++ src/qemu/qemu_monitor_json.c | 186 +++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 10 ++ src/qemu/qemu_monitor_text.c | 165 ++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 10 ++ 7 files changed, 772 insertions(+), 0 deletions(-)
'make syntax-check' didn't like this one: libvirt_unmarked_diagnostics src/qemu/qemu_monitor_json.c-3332- ), "total_bytes_sec"); src/qemu/qemu_monitor_json.c-3338- ), "read_bytes_sec"); src/qemu/qemu_monitor_json.c-3344- ), "write_bytes_sec"); src/qemu/qemu_monitor_json.c-3350- ), "total_iops_sec"); src/qemu/qemu_monitor_json.c-3356- ), "read_iops_sec"); src/qemu/qemu_monitor_json.c-3362- ), "write_iops_sec"); maint.mk: found unmarked diagnostic(s) I ended up shuffling those lines around. Also, prohibit_empty_lines_at_EOF src/qemu/qemu_monitor_json.c src/qemu/qemu_monitor_text.c maint.mk: empty line(s) or no newline at EOF I stripped the spurious whitespace.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5f4a18d..0a4eaa9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -94,6 +94,8 @@
#define QEMU_NB_MEM_PARAM 3
+#define QEMU_NB_BLKIOTHROTTLE_PARAM 6
Naming should match the public API. After fixing the syntax errors, I then got: qemu/qemu_driver.c: In function 'qemuDomainSetBlockIoTune': qemu/qemu_driver.c:10881:34: error: 'virDomainDiskDef' has no member named 'blkdeviotune' Ouch - your patches are submitted out of order. Looks like I'll have to defer further review of this one until domain_conf.h is updated. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 11/15/2011 02:02 AM, Lei Li wrote:
Implement the block I/O throttle setting and getting support to qemu driver.
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 342 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 37 +++++ src/qemu/qemu_monitor.h | 22 +++ src/qemu/qemu_monitor_json.c | 186 +++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 10 ++ src/qemu/qemu_monitor_text.c | 165 ++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 10 ++ 7 files changed, 772 insertions(+), 0 deletions(-)
Now that I've finished reviewing 4/8 and 6/8, I'm back to this one. In addition to the syntax check fixes I previously mentioned...
+static int +qemuDomainSetBlockIoTune(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{
+ + device = qemuDiskPathToAlias(vm, disk);
Huh, I just noticed that qemuDiskPathToAlias returns a malloc'd string as const char*, which is not const-correct. I'll fix that separately.
+ if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + isActive = virDomainObjIsActive(vm); + + if (flags == VIR_DOMAIN_AFFECT_CURRENT) { + if (isActive) + flags = VIR_DOMAIN_AFFECT_LIVE; + else + flags = VIR_DOMAIN_AFFECT_CONFIG; + } + + if (!isActive && (flags & VIR_DOMAIN_AFFECT_LIVE)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is not running")); + goto endjob; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (!vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change persistent config of a transient domain")); + goto endjob; + } + if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) + goto endjob; + }
Hmm, we repeat this chunk of code in several functions; maybe it's time to factor it into a helper method. But that's a patch for another day.
+ + for (i = 0; i < nparams; i++) { + virTypedParameterPtr param = ¶ms[i]; + + if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC)) { + + info.total_bytes_sec = params[i].value.ul; + } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC)) { + info.read_bytes_sec = params[i].value.ul; + } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC)) { + info.write_bytes_sec = params[i].value.ul; + } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC)) { + info.total_iops_sec = params[i].value.ul; + } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC)) { + info.read_iops_sec = params[i].value.ul; + } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC)) { + info.write_iops_sec = params[i].value.ul;
This is not type-safe. You have to also validate that the user passed a ullong value before you blindly dereference the value.ul union memory. Hmm - passing nparams==1 to set just read_bytes_sec has the side effect of wiping out any existing total_iops_sec, even though those two parameters seem somewhat orthogonal, all because we zero-initialized the struct that we pass on to the monitor command. Is that intended? I can live with it (but it probably ought to be documented), but we may want to consider being more flexible, by using '0' to clear a previous limit, but initializing to '-1' to imply the limit does not change. Then the qemu_monitor_json code should only emit the arguments that are actually being changed, rather than blindly always outputting 6 parameters even when the user only passed in one parameter. But I'm okay delaying that to a separate patch based on whether others think it would be a good improvement.
+ } else { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Unrecognized parameter"));
Why not tell the user which parameter name was not recognized? Also, this is not an OPERATION_INVALID, but an INVALID_ARG (it is bad input, not input that might be valid if the domain were in a different state).
+ goto endjob; + } + } + + if ((info.total_bytes_sec && info.read_bytes_sec) || + (info.total_bytes_sec && info.write_bytes_sec)) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("total and read/write of bytes_sec cannot be set at the same time")); + goto endjob; + } + + if ((info.total_iops_sec && info.read_iops_sec) || + (info.total_iops_sec && info.write_iops_sec)) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("total and read/write of iops_sec cannot be set at the same time")); + goto endjob; + }
Good - this is consistent with the domain_conf restrictions on the XML parse.
+ + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + priv = vm->privateData; + qemuDomainObjEnterMonitorWithDriver(driver, vm); + ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info, 1); + qemuDomainObjExitMonitorWithDriver(driver, vm); + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + sa_assert(persistentDef); + int idx = virDomainDiskIndexByName(vm->def, disk, true);
Here, you want false for the third parameter, to ensure we don't get confused by an ambiguous name.
+ if (i < 0) + goto endjob; + persistentDef->disks[idx]->blkdeviotune.total_bytes_sec = info.total_bytes_sec; + persistentDef->disks[idx]->blkdeviotune.read_bytes_sec = info.read_bytes_sec; + persistentDef->disks[idx]->blkdeviotune.write_bytes_sec = info.write_bytes_sec; + persistentDef->disks[idx]->blkdeviotune.total_iops_sec = info.total_iops_sec; + persistentDef->disks[idx]->blkdeviotune.read_iops_sec = info.read_iops_sec; + persistentDef->disks[idx]->blkdeviotune.write_iops_sec = info.write_iops_sec; + persistentDef->disks[idx]->blkdeviotune.mark = 1;
I got rid of the mark field during my edits to 4/8. And thinking about it further, field-by-field copying is tedious. If we move virDomainBlockIoTuneInfo out of qemu_monitor.h (which is a fishy name, anyways, since structs in that file should be in the qemuMonitor namespace), and into domain_conf.h (where the name is already appropriate), then we can reuse that named struct instead of an anonymous one, allowing direct struct copying.
+ } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
No need for two adjacent if blocks with the same condition. Conversely, we want to do the disk lookup prior to making any modification to live settings if both LIVE and CONFIG are present, to ensure that we don't strand the user with a live change but then can't make the persistent change.
+static int +qemuDomainGetBlockIoTune(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{
+ + device = qemuDiskPathToAlias(vm, disk); + + if (!device) { + goto cleanup; + } +
+ + if ((*nparams) == 0) { + /* Current number of parameters supported by QEMU Block I/O Throttling */ + *nparams = QEMU_NB_BLKIOTHROTTLE_PARAM; + ret = 0; + goto endjob; + }
These need re-ordering, given my change in 1/8 to allow NULL disk when probing max params.
+ + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + priv = vm->privateData; + qemuDomainObjEnterMonitorWithDriver(driver, vm); + ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply, 0);
No need for a flags argument if you never pass any flags down to the monitor level.
+ qemuDomainObjExitMonitorWithDriver(driver, vm); + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (!vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is transient")); + goto endjob; + } + if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) + goto endjob; + + sa_assert(persistentDef);
No need for this particular assert (it only helps static analyzers when you have split if() conditionals; but here there is no split).
+ int idx = virDomainDiskIndexByName(vm->def, disk, true); + if (idx < 0) + goto endjob; + reply.total_bytes_sec = persistentDef->disks[idx]->blkdeviotune.total_bytes_sec; + reply.read_bytes_sec = persistentDef->disks[idx]->blkdeviotune.read_bytes_sec; + reply.write_bytes_sec = persistentDef->disks[idx]->blkdeviotune.write_bytes_sec; + reply.total_iops_sec = persistentDef->disks[idx]->blkdeviotune.total_iops_sec; + reply.read_iops_sec = persistentDef->disks[idx]->blkdeviotune.read_iops_sec; + reply.write_iops_sec = persistentDef->disks[idx]->blkdeviotune.write_iops_sec;
Another place where struct copy is nicer.
+ ret = 0; + } + + if ((ret != 0) && ((*nparams) < QEMU_NB_BLKIOTHROTTLE_PARAM)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Cannot get block I/O parameters"));
Why the comparison of *nparams? That has no bearing on whether the monitor command succeeded. Besides, the monitor call should have already reported a message. This can simply be: if (ret < 0) goto endjob.
+ goto endjob; + } + + for (i = 0; i < QEMU_NB_BLKIOTHROTTLE_PARAM; i++) {
This needs to check that we don't exceed nparams as well.
+ param->value.ul = reply.write_iops_sec; + break; + default: + break; + } + } + ret = 0;
You need to properly set *nparams before returning.
+++ b/src/qemu/qemu_monitor.c @@ -2567,6 +2567,43 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, return ret; }
+int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, + const char *device, + virDomainBlockIoTuneInfoPtr info, + unsigned int flags) +{ + int ret; + + VIR_DEBUG("mon=%p, device=%p, info=%p, flags=%x", + mon, device, info, flags);
See my earlier comment about removing the flags argument, since we weren't exploiting it for any changes in behavior.
+++ b/src/qemu/qemu_monitor.h @@ -521,6 +521,28 @@ int qemuMonitorOpenGraphics(qemuMonitorPtr mon, const char *fdname, bool skipauth);
+ +typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo; +struct _virDomainBlockIoTuneInfo { + unsigned long long total_bytes_sec; + unsigned long long read_bytes_sec; + unsigned long long write_bytes_sec; + unsigned long long total_iops_sec; + unsigned long long read_iops_sec; + unsigned long long write_iops_sec; +}; +typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr;
I already moved this hunk into 4/8, in order to use struct copying.
+static int +qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, + const char *device, + virDomainBlockIoTuneInfoPtr reply) +{ + virJSONValuePtr io_throttle; + int ret = -1; + int i; + int found = 0; + + io_throttle = virJSONValueObjectGet(result, "return"); + + if (!io_throttle || io_throttle->type != VIR_JSON_TYPE_ARRAY) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _(" block_io_throttle reply was missing device list "));
trailing space in the message
+ + if (virJSONValueObjectGetNumberUlong(inserted, "bps", &reply->total_bytes_sec) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s"), "total_bytes_sec"); + goto cleanup; + }
I haven't yet tested this with qemu (right now, I'm focusing on code review), so I may have to do a further patch: if talking to an older qemu that doesn't output I/O throttling limits, then we can safely assume that there are no limits, and we should gracefully return all fields 0 in that case instead of an error message. I'm also assuming that patched qemu honors the same rules about total being exclusive with read/write values, and that qemu always outputs all six values with 0 in the proper places, if it supports throttling. If my assumptions are wrong, then libvirt may have to do a bit more work in parsing qemu output.
+ if (!found) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot found info for device '%s'"),
s/found/find/
+ if (flags) { + cmd = qemuMonitorJSONMakeCommand("block_set_io_throttle", + "s:device", device, + "U:bps", info->total_bytes_sec, + "U:bps_rd", info->read_bytes_sec, + "U:bps_wr", info->write_bytes_sec, + "U:iops", info->total_iops_sec, + "U:iops_rd", info->read_iops_sec, + "U:iops_wr", info->write_iops_sec, + NULL); + } + + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &result); + + if (ret == 0 && virJSONValueObjectHasKey(result, "error")) {
This may also need tweaking when talking to older qemu that lacks the block_set_io_throttle monitor command, to give a sensible error message (unless all 6 parameters are 0, in which case return success).
+static int qemuMonitorTextParseBlockIoThrottle(const char *result, + const char *device, + virDomainBlockIoTuneInfoPtr reply) +{ + char *dummy = NULL; + int ret = -1; + const char *p, *eol; + int devnamelen = strlen(device); + + p = result; + + while (*p) { + if (STRPREFIX(p, QEMU_DRIVE_HOST_PREFIX)) + p += strlen(QEMU_DRIVE_HOST_PREFIX); + + if (STREQLEN(p, device, devnamelen) && + p[devnamelen] == ':' && p[devnamelen+1] == ' ') { + + eol = strchr(p, '\n'); + if (!eol) + eol = p + strlen(p); + + p += devnamelen + 2; /*Skip to first label. */
Typically a space between /* and the comment body. This was copy-and-paste, though. As written, 3/8 only touched the monitor command; but to be complete, you must also implement the command-line changes. Thankfully, that was already done in a hunk of 4/8 that I moved here. The end result - here's everything I plan on squashing, even without any qemu testing (I may post further tweaks later once I start testing this with old and new qemu) diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c index 85b34bb..905afa6 100644 --- i/src/qemu/qemu_command.c +++ w/src/qemu/qemu_command.c @@ -1866,6 +1866,37 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, } } + /*block I/O throttling*/ + if (disk->blkdeviotune.total_bytes_sec) { + virBufferAsprintf(&opt, ",bps=%llu", + disk->blkdeviotune.total_bytes_sec); + } + + if (disk->blkdeviotune.read_bytes_sec) { + virBufferAsprintf(&opt, ",bps_rd=%llu", + disk->blkdeviotune.read_bytes_sec); + } + + if (disk->blkdeviotune.write_bytes_sec) { + virBufferAsprintf(&opt, ",bps_wr=%llu", + disk->blkdeviotune.write_bytes_sec); + } + + if (disk->blkdeviotune.total_iops_sec) { + virBufferAsprintf(&opt, ",iops=%llu", + disk->blkdeviotune.total_iops_sec); + } + + if (disk->blkdeviotune.read_iops_sec) { + virBufferAsprintf(&opt, ",iops_rd=%llu", + disk->blkdeviotune.read_iops_sec); + } + + if (disk->blkdeviotune.write_iops_sec) { + virBufferAsprintf(&opt, ",iops_wr=%llu", + disk->blkdeviotune.write_iops_sec); + } + if (virBufferError(&opt)) { virReportOOMError(); goto error; diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c index e350ffa..0308a41 100644 --- i/src/qemu/qemu_driver.c +++ w/src/qemu/qemu_driver.c @@ -93,7 +93,7 @@ #define QEMU_NB_MEM_PARAM 3 -#define QEMU_NB_BLKIOTHROTTLE_PARAM 6 +#define QEMU_NB_BLOCK_IO_TUNE_PARAM 6 #if HAVE_LINUX_KVM_H # include <linux/kvm.h> @@ -10833,22 +10833,34 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, for (i = 0; i < nparams; i++) { virTypedParameterPtr param = ¶ms[i]; - if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC)) { + if (param->type != VIR_TYPED_PARAM_ULLONG) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("expected unsigned long long for parameter %s"), + param->field); + goto endjob; + } - info.total_bytes_sec = params[i].value.ul; - } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC)) { - info.read_bytes_sec = params[i].value.ul; - } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC)) { - info.write_bytes_sec = params[i].value.ul; - } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC)) { - info.total_iops_sec = params[i].value.ul; - } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC)) { - info.read_iops_sec = params[i].value.ul; - } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC)) { - info.write_iops_sec = params[i].value.ul; + if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC)) { + info.total_bytes_sec = param->value.ul; + } else if (STREQ(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC)) { + info.read_bytes_sec = param->value.ul; + } else if (STREQ(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC)) { + info.write_bytes_sec = param->value.ul; + } else if (STREQ(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC)) { + info.total_iops_sec = param->value.ul; + } else if (STREQ(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC)) { + info.read_iops_sec = param->value.ul; + } else if (STREQ(param->field, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC)) { + info.write_iops_sec = param->value.ul; } else { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Unrecognized parameter")); + qemuReportError(VIR_ERR_INVALID_ARG, + _("Unrecognized parameter %s"), + param->field); goto endjob; } } @@ -10867,25 +10879,19 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, goto endjob; } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { - priv = vm->privateData; - qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info, 1); - qemuDomainObjExitMonitorWithDriver(driver, vm); - } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { sa_assert(persistentDef); int idx = virDomainDiskIndexByName(vm->def, disk, true); if (i < 0) goto endjob; - persistentDef->disks[idx]->blkdeviotune.total_bytes_sec = info.total_bytes_sec; - persistentDef->disks[idx]->blkdeviotune.read_bytes_sec = info.read_bytes_sec; - persistentDef->disks[idx]->blkdeviotune.write_bytes_sec = info.write_bytes_sec; - persistentDef->disks[idx]->blkdeviotune.total_iops_sec = info.total_iops_sec; - persistentDef->disks[idx]->blkdeviotune.read_iops_sec = info.read_iops_sec; - persistentDef->disks[idx]->blkdeviotune.write_iops_sec = info.write_iops_sec; - persistentDef->disks[idx]->blkdeviotune.mark = 1; + persistentDef->disks[idx]->blkdeviotune = info; + } + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + priv = vm->privateData; + qemuDomainObjEnterMonitorWithDriver(driver, vm); + ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info); + qemuDomainObjExitMonitorWithDriver(driver, vm); } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { @@ -10943,6 +10949,13 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, goto cleanup; } + if ((*nparams) == 0) { + /* Current number of parameters supported by QEMU Block I/O Throttling */ + *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM; + ret = 0; + goto cleanup; + } + device = qemuDiskPathToAlias(vm, disk); if (!device) { @@ -10973,18 +10986,13 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, goto endjob; } - if ((*nparams) == 0) { - /* Current number of parameters supported by QEMU Block I/O Throttling */ - *nparams = QEMU_NB_BLKIOTHROTTLE_PARAM; - ret = 0; - goto endjob; - } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { priv = vm->privateData; qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply, 0); + ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply); qemuDomainObjExitMonitorWithDriver(driver, vm); + if (ret < 0) + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { @@ -10996,26 +11004,13 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) goto endjob; - sa_assert(persistentDef); int idx = virDomainDiskIndexByName(vm->def, disk, true); if (idx < 0) goto endjob; - reply.total_bytes_sec = persistentDef->disks[idx]->blkdeviotune.total_bytes_sec; - reply.read_bytes_sec = persistentDef->disks[idx]->blkdeviotune.read_bytes_sec; - reply.write_bytes_sec = persistentDef->disks[idx]->blkdeviotune.write_bytes_sec; - reply.total_iops_sec = persistentDef->disks[idx]->blkdeviotune.total_iops_sec; - reply.read_iops_sec = persistentDef->disks[idx]->blkdeviotune.read_iops_sec; - reply.write_iops_sec = persistentDef->disks[idx]->blkdeviotune.write_iops_sec; - ret = 0; + reply = persistentDef->disks[idx]->blkdeviotune; } - if ((ret != 0) && ((*nparams) < QEMU_NB_BLKIOTHROTTLE_PARAM)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Cannot get block I/O parameters")); - goto endjob; - } - - for (i = 0; i < QEMU_NB_BLKIOTHROTTLE_PARAM; i++) { + for (i = 0; i < QEMU_NB_BLOCK_IO_TUNE_PARAM && i < *nparams; i++) { virTypedParameterPtr param = ¶ms[i]; param->value.ul = 0; param->type = VIR_TYPED_PARAM_ULLONG; @@ -11090,6 +11085,9 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, break; } } + + if (*nparams > QEMU_NB_BLOCK_IO_TUNE_PARAM) + *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM; ret = 0; endjob: diff --git i/src/qemu/qemu_monitor.c w/src/qemu/qemu_monitor.c index c0b711f..660aa11 100644 --- i/src/qemu/qemu_monitor.c +++ w/src/qemu/qemu_monitor.c @@ -2569,36 +2569,32 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr info, - unsigned int flags) + virDomainBlockIoTuneInfoPtr info) { int ret; - VIR_DEBUG("mon=%p, device=%p, info=%p, flags=%x", - mon, device, info, flags); + VIR_DEBUG("mon=%p, device=%p, info=%p", mon, device, info); if (mon->json) { - ret = qemuMonitorJSONSetBlockIoThrottle(mon, device, info, flags); + ret = qemuMonitorJSONSetBlockIoThrottle(mon, device, info); } else { - ret = qemuMonitorTextSetBlockIoThrottle(mon, device, info, flags); + ret = qemuMonitorTextSetBlockIoThrottle(mon, device, info); } return ret; } int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply, - unsigned int flags) + virDomainBlockIoTuneInfoPtr reply) { int ret; - VIR_DEBUG("mon=%p, device=%p, reply=%p, flags=%x", - mon, device, reply, flags); + VIR_DEBUG("mon=%p, device=%p, reply=%p", mon, device, reply); if (mon->json) { - ret = qemuMonitorJSONGetBlockIoThrottle(mon, device, reply, flags); + ret = qemuMonitorJSONGetBlockIoThrottle(mon, device, reply); } else { - ret = qemuMonitorTextGetBlockIoThrottle(mon, device, reply, flags); + ret = qemuMonitorTextGetBlockIoThrottle(mon, device, reply); } return ret; } diff --git i/src/qemu/qemu_monitor.h w/src/qemu/qemu_monitor.h index 48c6db0..0b14e0c 100644 --- i/src/qemu/qemu_monitor.h +++ w/src/qemu/qemu_monitor.h @@ -521,27 +521,13 @@ int qemuMonitorOpenGraphics(qemuMonitorPtr mon, const char *fdname, bool skipauth); - -typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo; -struct _virDomainBlockIoTuneInfo { - unsigned long long total_bytes_sec; - unsigned long long read_bytes_sec; - unsigned long long write_bytes_sec; - unsigned long long total_iops_sec; - unsigned long long read_iops_sec; - unsigned long long write_iops_sec; -}; -typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr; - int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr info, - unsigned int flags); + virDomainBlockIoTuneInfoPtr info); int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply, - unsigned int flags); + virDomainBlockIoTuneInfoPtr reply); /** * When running two dd process and using <> redirection, we need a diff --git i/src/qemu/qemu_monitor_json.c w/src/qemu/qemu_monitor_json.c index 925031e..fb2a1fd 100644 --- i/src/qemu/qemu_monitor_json.c +++ w/src/qemu/qemu_monitor_json.c @@ -3292,7 +3292,7 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, if (!io_throttle || io_throttle->type != VIR_JSON_TYPE_ARRAY) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _(" block_io_throttle reply was missing device list ")); + _(" block_io_throttle reply was missing device list")); goto cleanup; } @@ -3328,38 +3328,38 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, } if (virJSONValueObjectGetNumberUlong(inserted, "bps", &reply->total_bytes_sec) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s"), "total_bytes_sec"); + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot read total_bytes_sec")); goto cleanup; } if (virJSONValueObjectGetNumberUlong(inserted, "bps_rd", &reply->read_bytes_sec) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s"), "read_bytes_sec"); + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot read read_bytes_sec")); goto cleanup; } if (virJSONValueObjectGetNumberUlong(inserted, "bps_wr", &reply->write_bytes_sec) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s"), "write_bytes_sec"); + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot read write_bytes_sec")); goto cleanup; } if (virJSONValueObjectGetNumberUlong(inserted, "iops", &reply->total_iops_sec) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s"), "total_iops_sec"); + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot read total_iops_sec")); goto cleanup; } if (virJSONValueObjectGetNumberUlong(inserted, "iops_rd", &reply->read_iops_sec) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s"), "read_iops_sec"); + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot read read_iops_sec")); goto cleanup; } if (virJSONValueObjectGetNumberUlong(inserted, "iops_wr", &reply->write_iops_sec) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s"), "write_iops_sec"); + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot read write_iops_sec")); goto cleanup; } break; @@ -3367,7 +3367,7 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, if (!found) { qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot found info for device '%s'"), + _("cannot find throttling info for device '%s'"), device); goto cleanup; } @@ -3379,25 +3379,21 @@ cleanup: int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr info, - unsigned int flags) + virDomainBlockIoTuneInfoPtr info) { int ret = -1; virJSONValuePtr cmd = NULL; virJSONValuePtr result = NULL; - if (flags) { - cmd = qemuMonitorJSONMakeCommand("block_set_io_throttle", - "s:device", device, - "U:bps", info->total_bytes_sec, - "U:bps_rd", info->read_bytes_sec, - "U:bps_wr", info->write_bytes_sec, - "U:iops", info->total_iops_sec, - "U:iops_rd", info->read_iops_sec, - "U:iops_wr", info->write_iops_sec, - NULL); - } - + cmd = qemuMonitorJSONMakeCommand("block_set_io_throttle", + "s:device", device, + "U:bps", info->total_bytes_sec, + "U:bps_rd", info->read_bytes_sec, + "U:bps_wr", info->write_bytes_sec, + "U:iops", info->total_iops_sec, + "U:iops_rd", info->read_iops_sec, + "U:iops_wr", info->write_iops_sec, + NULL); if (!cmd) return -1; @@ -3423,18 +3419,13 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply, - unsigned int flags) + virDomainBlockIoTuneInfoPtr reply) { int ret = -1; virJSONValuePtr cmd = NULL; virJSONValuePtr result = NULL; - if (!flags) { - cmd = qemuMonitorJSONMakeCommand("query-block", - NULL); - } - + cmd = qemuMonitorJSONMakeCommand("query-block", NULL); if (!cmd) { return -1; } @@ -3454,11 +3445,10 @@ int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, ret = -1; } - if (ret == 0 && !flags) + if (ret == 0) ret = qemuMonitorJSONBlockIoThrottleInfo(result, device, reply); virJSONValueFree(cmd); virJSONValueFree(result); return ret; } - diff --git i/src/qemu/qemu_monitor_json.h w/src/qemu/qemu_monitor_json.h index bf12dc5..1b8625f 100644 --- i/src/qemu/qemu_monitor_json.h +++ w/src/qemu/qemu_monitor_json.h @@ -257,12 +257,10 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon, int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr info, - unsigned int flags); + virDomainBlockIoTuneInfoPtr info); int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply, - unsigned int flags); + virDomainBlockIoTuneInfoPtr reply); #endif /* QEMU_MONITOR_JSON_H */ diff --git i/src/qemu/qemu_monitor_text.c w/src/qemu/qemu_monitor_text.c index 0ccc111..c734892 100644 --- i/src/qemu/qemu_monitor_text.c +++ w/src/qemu/qemu_monitor_text.c @@ -812,7 +812,7 @@ int qemuMonitorTextGetBlockInfo(qemuMonitorPtr mon, if (!eol) eol = p + strlen(p); - p += devnamelen + 2; /*Skip to first label. */ + p += devnamelen + 2; /* Skip to first label. */ while (*p) { if (STRPREFIX(p, "removable=")) { @@ -3433,8 +3433,7 @@ cleanup: int qemuMonitorTextSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr info, - unsigned int flags) + virDomainBlockIoTuneInfoPtr info) { char *cmd = NULL; char *result = NULL; @@ -3442,13 +3441,11 @@ int qemuMonitorTextSetBlockIoThrottle(qemuMonitorPtr mon, const char *cmd_name = NULL; /* For the not specified fields, 0 by default */ - if (flags) { - cmd_name = "block_set_io_throttle"; - ret = virAsprintf(&cmd, "%s %s %llu %llu %llu %llu %llu %llu", cmd_name, - device, info->total_bytes_sec, info->read_bytes_sec, - info->write_bytes_sec, info->total_iops_sec, - info->read_iops_sec, info->write_iops_sec); - } + cmd_name = "block_set_io_throttle"; + ret = virAsprintf(&cmd, "%s %s %llu %llu %llu %llu %llu %llu", cmd_name, + device, info->total_bytes_sec, info->read_bytes_sec, + info->write_bytes_sec, info->total_iops_sec, + info->read_iops_sec, info->write_iops_sec); if (ret < 0) { virReportOOMError(); @@ -3475,9 +3472,10 @@ cleanup: return ret; } -static int qemuMonitorTextParseBlockIoThrottle(const char *result, - const char *device, - virDomainBlockIoTuneInfoPtr reply) +static int +qemuMonitorTextParseBlockIoThrottle(const char *result, + const char *device, + virDomainBlockIoTuneInfoPtr reply) { char *dummy = NULL; int ret = -1; @@ -3497,7 +3495,7 @@ static int qemuMonitorTextParseBlockIoThrottle(const char *result, if (!eol) eol = p + strlen(p); - p += devnamelen + 2; /*Skip to first label. */ + p += devnamelen + 2; /* Skip to first label. */ while (*p) { if (STRPREFIX(p, "bps=")) { @@ -3554,25 +3552,13 @@ cleanup: int qemuMonitorTextGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply, - unsigned int flags) + virDomainBlockIoTuneInfoPtr reply) { - char *cmd = NULL; char *result = NULL; int ret = 0; - const char *cmd_name = NULL; + const char *cmd_name = "info block"; - if (flags) { - cmd_name = "info block"; - ret = virAsprintf(&cmd, "%s", cmd_name); - } - - if (ret < 0) { - virReportOOMError(); - return -1; - } - - if (qemuMonitorHMPCommand(mon, cmd, &result) < 0) { + if (qemuMonitorHMPCommand(mon, cmd_name, &result) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot run monitor command")); ret = -1; @@ -3589,8 +3575,6 @@ int qemuMonitorTextGetBlockIoThrottle(qemuMonitorPtr mon, ret = qemuMonitorTextParseBlockIoThrottle(result, device, reply); cleanup: - VIR_FREE(cmd); VIR_FREE(result); return ret; } - diff --git i/src/qemu/qemu_monitor_text.h w/src/qemu/qemu_monitor_text.h index 1c47d39..c3e97e0 100644 --- i/src/qemu/qemu_monitor_text.h +++ w/src/qemu/qemu_monitor_text.h @@ -250,12 +250,10 @@ int qemuMonitorTextOpenGraphics(qemuMonitorPtr mon, int qemuMonitorTextSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr info, - unsigned int flags); + virDomainBlockIoTuneInfoPtr info); int qemuMonitorTextGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply, - unsigned int flags); + virDomainBlockIoTuneInfoPtr reply); #endif /* QEMU_MONITOR_TEXT_H */ -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Wed, Nov 23, 2011 at 10:41:32AM -0700, Eric Blake wrote:
Hmm - passing nparams==1 to set just read_bytes_sec has the side effect of wiping out any existing total_iops_sec, even though those two parameters seem somewhat orthogonal, all because we zero-initialized the struct that we pass on to the monitor command. Is that intended? I can live with it (but it probably ought to be documented), but we may want to consider being more flexible, by using '0' to clear a previous limit, but initializing to '-1' to imply the limit does not change. Then the qemu_monitor_json code should only emit the arguments that are actually being changed, rather than blindly always outputting 6 parameters even when the user only passed in one parameter. But I'm okay delaying that to a separate patch based on whether others think it would be a good improvement.
+1. I believe I had pointed this out previously as well (albeit not as concisely as this). -- Adam Litke <agl@us.ibm.com> IBM Linux Technology Center
On 11/28/2011 11:24 PM, Adam Litke wrote:
On Wed, Nov 23, 2011 at 10:41:32AM -0700, Eric Blake wrote:
Hmm - passing nparams==1 to set just read_bytes_sec has the side effect of wiping out any existing total_iops_sec, even though those two parameters seem somewhat orthogonal, all because we zero-initialized the struct that we pass on to the monitor command. Is that intended? I can live with it (but it probably ought to be documented), but we may want to consider being more flexible, by using '0' to clear a previous limit, but initializing to '-1' to imply the limit does not change. Then the qemu_monitor_json code should only emit the arguments that are actually being changed, rather than blindly always outputting 6 parameters even when the user only passed in one parameter. But I'm okay delaying that to a separate patch based on whether others think it would be a good improvement. +1. I believe I had pointed this out previously as well (albeit not as concisely as this).
Well, here is the description of block I/O throttling command 'block_io_set_throttle' in qmp-commands.hx. EQMP { .name = "block_set_io_throttle", .args_type = "device:B,bps:i?,bps_rd:i?,bps_wr:i?,iops:i?,iops_rd:i?,iops_wr:i?", .params = "device [bps] [bps_rd] [bps_wr] [iops] [iops_rd] [iops_wr]", .help = "change I/O throttle limits for a block drive", .user_print = monitor_user_noop, .mhandler.cmd_new = do_block_set_io_throttle, }, SQMP block_set_io_throttle ------------ Change I/O throttle limits for a block drive. Arguments: - "device": device name (json-string) - "bps": total throughput limit in bytes per second(json-int, optional) - "bps_rd": read throughput limit in bytes per second(json-int, optional) - "bps_wr": read throughput limit in bytes per second(json-int, optional) - "iops": total I/O operations per second(json-int, optional) - "iops_rd": read I/O operations per second(json-int, optional) - "iops_wr": write I/O operations per second(json-int, optional) Example: -> { "execute": "block_set_io_throttle", "arguments": { "device": "virtio0", "bps": "1000000", "bps_rd": "0", "bps_wr": "0", "iops": "0", "iops_rd": "0", "iops_wr": "0" } } <- { "return": {} } This qmp command need all these 6 parameters at one time in qemu, so zero-initialized the struct to meet If there is no setting value for some of the fields. -- Lei
participants (3)
-
Adam Litke -
Eric Blake -
Lei Li