[PATCH] virresctrl: Fix updating the mask for a cache resource
In 'virResctrlAllocUpdateMask', mask is updated only if 'previous mask' is NULL. By default, the bitmask for a cache resource for a VM is initialized with 'default-resctrl-group' bitmask. So the 'previous mask' would not be NULL and mask won't get updated if cachetune is configured for a VM. This causes libvirt to use same bitmask as 'default-resctrl-group' bitmask for a cache resource for a VM. This patch fixes the issue. Fixes: d8a354954aba9cd45ab0317915a0a2be27c04767 Signed-off-by: Vinayak Kale <vkale@nvidia.com> --- src/util/virresctrl.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 35e92022db..a7d36f492c 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -1104,8 +1104,10 @@ virResctrlAllocUpdateMask(virResctrlAlloc *alloc, VIR_EXPAND_N(a_type->masks, a_type->nmasks, cache - a_type->nmasks + 1); - if (!a_type->masks[cache]) - a_type->masks[cache] = virBitmapNewCopy(mask); + if (a_type->masks[cache]) + virBitmapFree(a_type->masks[cache]); + + a_type->masks[cache] = virBitmapNewCopy(mask); return 0; } -- 2.17.1
On a %A in %Y, Vinayak Kale wrote:
In 'virResctrlAllocUpdateMask', mask is updated only if 'previous mask' is NULL.
By default, the bitmask for a cache resource for a VM is initialized with 'default-resctrl-group' bitmask. So the 'previous mask' would not be NULL and mask won't get updated if cachetune is configured for a VM. This causes libvirt to use same bitmask as 'default-resctrl-group' bitmask for a cache resource for a VM. This patch fixes the issue.
Fixes: d8a354954aba9cd45ab0317915a0a2be27c04767
Thanks for catching this, it seems to be the only such case in that commit.
Signed-off-by: Vinayak Kale <vkale@nvidia.com> --- src/util/virresctrl.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> I will push it after the pipeline passes: https://gitlab.com/janotomko/libvirt/-/pipelines/333213773 Jano
On 07-Jul-21 6:46 PM, Jano Tomko wrote:
Signed-off-by: Vinayak Kale <vkale@nvidia.com> --- src/util/virresctrl.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com>
I will push it after the pipeline passes: https://gitlab.com/janotomko/libvirt/-/pipelines/333213773
Jano
Thanks Jano and Daniel.
On 7/2/21 4:23 AM, Vinayak Kale wrote:
In 'virResctrlAllocUpdateMask', mask is updated only if 'previous mask' is NULL.
By default, the bitmask for a cache resource for a VM is initialized with 'default-resctrl-group' bitmask. So the 'previous mask' would not be NULL and mask won't get updated if cachetune is configured for a VM. This causes libvirt to use same bitmask as 'default-resctrl-group' bitmask for a cache resource for a VM. This patch fixes the issue.
Fixes: d8a354954aba9cd45ab0317915a0a2be27c04767
Signed-off-by: Vinayak Kale <vkale@nvidia.com> ---
I was going to suggest whether cachetune couldn't just overwrite the default mask if it's configured, but that would just shift the problem somewhere else. This fix seems adequate. Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
src/util/virresctrl.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 35e92022db..a7d36f492c 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -1104,8 +1104,10 @@ virResctrlAllocUpdateMask(virResctrlAlloc *alloc, VIR_EXPAND_N(a_type->masks, a_type->nmasks, cache - a_type->nmasks + 1);
- if (!a_type->masks[cache]) - a_type->masks[cache] = virBitmapNewCopy(mask); + if (a_type->masks[cache]) + virBitmapFree(a_type->masks[cache]); + + a_type->masks[cache] = virBitmapNewCopy(mask);
return 0; }
participants (3)
-
Daniel Henrique Barboza -
Jano Tomko -
Vinayak Kale