Re: [libvirt] [PATCH 2/2] qemu: amend existing table of device weights

Prior to this patch, for a running dom, the commands: $ virsh blkiotune dom --device-weights /dev/sda,502,/dev/sdb,498 $ virsh blkiotune dom --device-weights /dev/sda,503 $ virsh blkiotune dom weight : 500 device_weight : /dev/sda,503 claim that /dev/sdb no longer has a non-default weight, but directly querying cgroups says otherwise: $ cat /cgroup/blkio/libvirt/qemu/dom/blkio.weight_device 8:0 503 8:16 498 After this patch, an explicit 0 is required to remove a device path from the XML, and omitting a device path that was previously specified leaves that device path untouched in the XML, to match cgroups behavior. * src/qemu/qemu_driver.c (parseBlkioWeightDeviceStr): Rename... (qemuDomainParseDeviceWeightStr): ...and use correct type. (qemuDomainSetBlkioParameters): After parsing string, modify rather than replacing existing table. * tools/virsh.pod (blkiotune): Tweak wording. --- src/qemu/qemu_driver.c | 80 +++++++++++++++++++++++++++++++++++------------ tools/virsh.pod | 4 ++- 2 files changed, 62 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 105bdde..81d11c2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5887,8 +5887,8 @@ cleanup: * for example, /dev/disk/by-path/pci-0000:00:1f.2-scsi-0:0:0:0,800 */ static int -parseBlkioWeightDeviceStr(char *deviceWeightStr, - virBlkioDeviceWeightPtr *dw, int *size) +qemuDomainParseDeviceWeightStr(char *deviceWeightStr, + virBlkioDeviceWeightPtr *dw, size_t *size) { char *temp; int ndevices = 0; @@ -5965,6 +5965,41 @@ cleanup: return -1; } +/* Modify def to reflect all device weight changes described in tmp. */ +static int +qemuDomainMergeDeviceWeights(virBlkioDeviceWeightPtr *def, size_t *def_size, + virBlkioDeviceWeightPtr tmp, size_t tmp_size) +{ + int i, j; + virBlkioDeviceWeightPtr dw; + + for (i = 0; i < tmp_size; i++) { + bool found = false; + + dw = &tmp[i]; + for (j = 0; j < *def_size; j++) { + if (STREQ(dw->path, (*def)[j].path)) { + found = true; + (*def)[j].weight = dw->weight; + break; + } + } + if (!found) { + if (!dw->weight) + continue; + if (VIR_EXPAND_N(*def, *def_size, 1) < 0) { + virReportOOMError(); + return -1; + } + (*def)[*def_size - 1].path = dw->path; + (*def)[*def_size - 1].weight = dw->weight; + dw->path = NULL; + } + } + + return 0; +} + static int qemuDomainSetBlkioParameters(virDomainPtr dom, virTypedParameterPtr params, int nparams, @@ -6056,7 +6091,7 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, ret = -1; } } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { - int ndevices; + size_t ndevices; virBlkioDeviceWeightPtr devices = NULL; if (param->type != VIR_TYPED_PARAM_STRING) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", @@ -6066,9 +6101,9 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, continue; } - if (parseBlkioWeightDeviceStr(params[i].value.s, - &devices, - &ndevices) < 0) { + if (qemuDomainParseDeviceWeightStr(params[i].value.s, + &devices, + &ndevices) < 0) { ret = -1; continue; } @@ -6088,14 +6123,16 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, ret = -1; continue; } - virBlkioDeviceWeightArrayClear(vm->def->blkio.devices, - vm->def->blkio.ndevices); - VIR_FREE(vm->def->blkio.devices); - vm->def->blkio.devices = devices; - vm->def->blkio.ndevices = ndevices; + if (qemuDomainMergeDeviceWeights(&vm->def->blkio.devices, + &vm->def->blkio.ndevices, + devices, ndevices) < 0) + ret = -1; + virBlkioDeviceWeightArrayClear(devices, ndevices); + VIR_FREE(devices); } else { qemuReportError(VIR_ERR_INVALID_ARG, - _("Parameter `%s' not supported"), param->field); + _("Parameter `%s' not supported"), + param->field); ret = -1; } } @@ -6127,7 +6164,7 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, persistentDef->blkio.weight = params[i].value.ui; } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { virBlkioDeviceWeightPtr devices = NULL; - int ndevices; + size_t ndevices; if (param->type != VIR_TYPED_PARAM_STRING) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", _("invalid type for device_weight tunable, " @@ -6135,17 +6172,18 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, ret = -1; continue; } - if (parseBlkioWeightDeviceStr(params[i].value.s, - &devices, - &ndevices) < 0) { + if (qemuDomainParseDeviceWeightStr(params[i].value.s, + &devices, + &ndevices) < 0) { ret = -1; continue; } - virBlkioDeviceWeightArrayClear(persistentDef->blkio.devices, - persistentDef->blkio.ndevices); - VIR_FREE(persistentDef->blkio.devices); - persistentDef->blkio.devices = devices; - persistentDef->blkio.ndevices = ndevices; + if (qemuDomainMergeDeviceWeights(&vm->def->blkio.devices, + &vm->def->blkio.ndevices, + devices, ndevices) < 0) + ret = -1; + virBlkioDeviceWeightArrayClear(devices, ndevices); + VIR_FREE(devices); } else { qemuReportError(VIR_ERR_INVALID_ARG, _("Parameter `%s' not supported"), diff --git a/tools/virsh.pod b/tools/virsh.pod index d606f99..01b8538 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1055,7 +1055,9 @@ I<--weight> is in range [100, 1000]. B<device-weights> is a single string listing one or more device/weight pairs, in the format of /path/to/device,weight,/path/to/device,weight. Each weight is in the range [100, 1000], or the value 0 to remove that -device from per-device listings. +device from per-device listings. Only the devices listed in the string +are modified; any existing per-device weights for other devices remain +unchanged. If I<--live> is specified, affect a running guest. If I<--config> is specified, affect the next boot of a persistent guest. -- 1.7.7.3