[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Tue, Nov 29, 2011 at 02:11:44PM -0700, Eric Blake wrote:
> 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

ACK.

-- 
Thanks,
Hu Tao


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]