[libvirt] [PATCHv7 13/18] qemu: enable resctrl monitor in qemu

[John Ferlan]

On 10/22/18 4:01 AM, Wang Huaqiang wrote:
> Add functions for creating, destroying, reconnecting resctrl
> monitor in qemu according to the configuration in domain XML.
> 
> Signed-off-by: Wang Huaqiang <huaqiang.wang at intel.com>
> ---
>  src/qemu/qemu_process.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 65 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index e9c7618..fba4fb4 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c

[...]

> @@ -5440,11 +5452,42 @@ qemuProcessSetupVcpu(virDomainObjPtr vm,
>          return -1;
>  
>      for (i = 0; i < vm->def->nresctrls; i++) {
> +        size_t j = 0;
>          virDomainResctrlDefPtr ct = vm->def->resctrls[i];
>  
>          if (virBitmapIsBitSet(ct->vcpus, vcpuid)) {>              if (virResctrlAllocAddPID(ct->alloc, vcpupid) < 0)
>                  return -1;
> +
> +            /* The order of invoking virResctrlMonitorAddPID matters, it is
> +             * required to invoke this function first for monitor that has
> +             * the same vcpus setting as the allocation in same def->resctrl.
> +             * Otherwise, some other monitor's pid may be removed from its
> +             * resource group's 'tasks' file.*/
> +            for (j = 0; j < vm->def->resctrls[i]->nmonitors; j++) {

s/vm->def->resctrls[i]/ct/  (above and below)

> +                mon = vm->def->resctrls[i]->monitors[j];
> +
> +                if (!virBitmapEqual(ct->vcpus, mon->vcpus))
> +                    continue;
> +
> +                if (virBitmapIsBitSet(mon->vcpus, vcpuid)) {
> +                    if (virResctrlMonitorAddPID(mon->instance, vcpupid) < 0)
> +                        return -1;
> +                }
> +                break;

It seems this break should be inside the IsBitSet, right (as is for the
ct->alloc, vcpupid match)?  Otherwise, we run the loop just once and not
run until we find our vcpuid in mon->vcpus

> +            }
> +

The next loop is duplicitous and can be removed, right?


with some adjustments (which I can make as described),

Reviewed-by: John Ferlan <jferlan at redhat.com>

John

[...]