Re: [libvirt] [PATCHv5 15/19] qemu: enable resctrl monitor in qemu

2018-10-12 Thread Wang, Huaqiang 



> -Original Message-
> From: John Ferlan [mailto:jfer...@redhat.com]
> Sent: Thursday, October 11, 2018 4:59 AM
> To: Wang, Huaqiang ; libvir-list@redhat.com
> Cc: Feng, Shaohe ; Niu, Bing ;
> Ding, Jian-feng ; Zang, Rui 
> Subject: Re: [libvirt] [PATCHv5 15/19] qemu: enable resctrl monitor in qemu
> 
> 
> 
> On 10/9/18 6:30 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 
> > ---
> >  src/qemu/qemu_process.c | 49
> > ++---
> >  1 file changed, 46 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index
> > e9c7618..a4bbef6 100644
> > --- a/src/qemu/qemu_process.c
> > +++ b/src/qemu/qemu_process.c
> > @@ -2611,10 +2611,22 @@ qemuProcessResctrlCreate(virQEMUDriverPtr
> driver,
> >  return -1;
> >
> >  for (i = 0; i < vm->def->nresctrls; i++) {
> > +size_t j = 0;
> >  if (virResctrlAllocCreate(caps->host.resctrl,
> >vm->def->resctrls[i]->alloc,
> >priv->machineName) < 0)
> >  goto cleanup;
> > +
> > +for (j = 0; j < vm->def->resctrls[i]->nmonitors; j++) {
> > +virDomainResctrlMonDefPtr mon = NULL;
> > +
> > +mon = vm->def->resctrls[i]->monitors[j];
> > +if (virResctrlMonitorCreate(vm->def->resctrls[i]->alloc,
> > +mon->instance,
> > +priv->machineName) < 0)
> > +goto cleanup;
> > +
> > +}
> >  }
> >
> >  ret = 0;
> > @@ -5440,11 +5452,22 @@ 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;
> > +
> > +for (j = 0; j < vm->def->resctrls[i]->nmonitors; j++) {
> > +virDomainResctrlMonDefPtr mon = NULL;
> > +
> > +mon = vm->def->resctrls[i]->monitors[j];
> > +if (virBitmapIsBitSet(mon->vcpus, vcpuid)) {
> > +if (virResctrlMonitorAddPID(mon->instance, vcpupid) < 
> > 0)
> > +return -1;
> > +}
> > +}
> >  break;
> >  }
> >  }
> > @@ -7207,8 +7230,18 @@ void qemuProcessStop(virQEMUDriverPtr driver,
> >  /* Remove resctrl allocation after cgroups are cleaned up which makes 
> > it
> >   * kind of safer (although removing the allocation should work even 
> > with
> >   * pids in tasks file */
> > -for (i = 0; i < vm->def->nresctrls; i++)
> > +for (i = 0; i < vm->def->nresctrls; i++) {
> > +size_t j = 0;
> > +
> > +for (j = 0; j < vm->def->resctrls[i]->nmonitors; j++) {
> > +virDomainResctrlMonDefPtr mon = NULL;
> > +
> > +mon = vm->def->resctrls[i]->monitors[j];
> > +virResctrlMonitorRemove(mon->instance);
> > +}
> > +
> >  virResctrlAllocRemove(vm->def->resctrls[i]->alloc);
> > +}
> >
> >  qemuProcessRemoveDomainStatus(driver, vm);
> >
> > @@ -7222,8 +7255,7 @@ void qemuProcessStop(virQEMUDriverPtr driver,
> >  virPortAllocatorRelease(graphics->data.vnc.port);
> >  } else if (graphics->data.vnc.portReserved) {
> >  virPortAllocatorRelease(graphics->data.vnc.port);
> > -graphics->data.vnc.portReserved = false;
> > -}
> > +graphics->data.vnc.portReserved = false; }
> 
> Rogue edit?  Caused an issue w/ my Coverity environment because there's a
> false positive surrounding the data.vnc.websocket value being -1 and thus
> passing that to virPortAllocatorRelease and using it without checking > 0 in a
> few lines.
> 
> Anyway this hunk needs to be removed.

I have no idea why I made such a cha

Re: [libvirt] [PATCHv5 15/19] qemu: enable resctrl monitor in qemu

2018-10-10 Thread John Ferlan 



On 10/9/18 6:30 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 
> ---
>  src/qemu/qemu_process.c | 49 
> ++---
>  1 file changed, 46 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index e9c7618..a4bbef6 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -2611,10 +2611,22 @@ qemuProcessResctrlCreate(virQEMUDriverPtr driver,
>  return -1;
>  
>  for (i = 0; i < vm->def->nresctrls; i++) {
> +size_t j = 0;
>  if (virResctrlAllocCreate(caps->host.resctrl,
>vm->def->resctrls[i]->alloc,
>priv->machineName) < 0)
>  goto cleanup;
> +
> +for (j = 0; j < vm->def->resctrls[i]->nmonitors; j++) {
> +virDomainResctrlMonDefPtr mon = NULL;
> +
> +mon = vm->def->resctrls[i]->monitors[j];
> +if (virResctrlMonitorCreate(vm->def->resctrls[i]->alloc,
> +mon->instance,
> +priv->machineName) < 0)
> +goto cleanup;
> +
> +}
>  }
>  
>  ret = 0;
> @@ -5440,11 +5452,22 @@ 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;
> +
> +for (j = 0; j < vm->def->resctrls[i]->nmonitors; j++) {
> +virDomainResctrlMonDefPtr mon = NULL;
> +
> +mon = vm->def->resctrls[i]->monitors[j];
> +if (virBitmapIsBitSet(mon->vcpus, vcpuid)) {
> +if (virResctrlMonitorAddPID(mon->instance, vcpupid) < 0)
> +return -1;
> +}
> +}
>  break;
>  }
>  }
> @@ -7207,8 +7230,18 @@ void qemuProcessStop(virQEMUDriverPtr driver,
>  /* Remove resctrl allocation after cgroups are cleaned up which makes it
>   * kind of safer (although removing the allocation should work even with
>   * pids in tasks file */
> -for (i = 0; i < vm->def->nresctrls; i++)
> +for (i = 0; i < vm->def->nresctrls; i++) {
> +size_t j = 0;
> +
> +for (j = 0; j < vm->def->resctrls[i]->nmonitors; j++) {
> +virDomainResctrlMonDefPtr mon = NULL;
> +
> +mon = vm->def->resctrls[i]->monitors[j];
> +virResctrlMonitorRemove(mon->instance);
> +}
> +
>  virResctrlAllocRemove(vm->def->resctrls[i]->alloc);
> +}
>  
>  qemuProcessRemoveDomainStatus(driver, vm);
>  
> @@ -7222,8 +7255,7 @@ void qemuProcessStop(virQEMUDriverPtr driver,
>  virPortAllocatorRelease(graphics->data.vnc.port);
>  } else if (graphics->data.vnc.portReserved) {
>  virPortAllocatorRelease(graphics->data.vnc.port);
> -graphics->data.vnc.portReserved = false;
> -}
> +graphics->data.vnc.portReserved = false; }

Rogue edit?  Caused an issue w/ my Coverity environment because there's
a false positive surrounding the data.vnc.websocket value being -1 and
thus passing that to virPortAllocatorRelease and using it without
checking > 0 in a few lines.

Anyway this hunk needs to be removed.

John

(OK, so I didn't look too hard at these last two, but I'm going back to
the start to read what you've written today).

>  if (graphics->data.vnc.websocketGenerated) {
>  virPortAllocatorRelease(graphics->data.vnc.websocket);
>  graphics->data.vnc.websocketGenerated = false;
> @@ -7939,9 +7971,20 @@ qemuProcessReconnect(void *opaque)
>  goto error;
>  
>  for (i = 0; i < obj->def->nresctrls; i++) {
> +size_t j = 0;
> +
>  if (virResctrlAllocDeterminePath(obj->def->resctrls[i]->alloc,
>   priv->machineName) < 0)
>  goto error;
> +
> +for (j = 0; j < obj->def->resctrls[i]->nmonitors; j++) {
> +virDomainResctrlMonDefPtr mon = NULL;
> +
> +mon = obj->def->resctrls[i]->monitors[j];
> +if (virResctrlMonitorDeterminePath(mon->instance,
> +   priv->machineName) < 0)
> +goto error;
> +}
>  }
>  
>  /* update domain state XML with possibly updated state in virDomainObj */
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv5 15/19] qemu: enable resctrl monitor in qemu

2018-10-09 Thread Wang Huaqiang 
Add functions for creating, destroying, reconnecting resctrl
monitor in qemu according to the configuration in domain XML.

Signed-off-by: Wang Huaqiang 
---
 src/qemu/qemu_process.c | 49 ++---
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e9c7618..a4bbef6 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2611,10 +2611,22 @@ qemuProcessResctrlCreate(virQEMUDriverPtr driver,
 return -1;
 
 for (i = 0; i < vm->def->nresctrls; i++) {
+size_t j = 0;
 if (virResctrlAllocCreate(caps->host.resctrl,
   vm->def->resctrls[i]->alloc,
   priv->machineName) < 0)
 goto cleanup;
+
+for (j = 0; j < vm->def->resctrls[i]->nmonitors; j++) {
+virDomainResctrlMonDefPtr mon = NULL;
+
+mon = vm->def->resctrls[i]->monitors[j];
+if (virResctrlMonitorCreate(vm->def->resctrls[i]->alloc,
+mon->instance,
+priv->machineName) < 0)
+goto cleanup;
+
+}
 }
 
 ret = 0;
@@ -5440,11 +5452,22 @@ 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;
+
+for (j = 0; j < vm->def->resctrls[i]->nmonitors; j++) {
+virDomainResctrlMonDefPtr mon = NULL;
+
+mon = vm->def->resctrls[i]->monitors[j];
+if (virBitmapIsBitSet(mon->vcpus, vcpuid)) {
+if (virResctrlMonitorAddPID(mon->instance, vcpupid) < 0)
+return -1;
+}
+}
 break;
 }
 }
@@ -7207,8 +7230,18 @@ void qemuProcessStop(virQEMUDriverPtr driver,
 /* Remove resctrl allocation after cgroups are cleaned up which makes it
  * kind of safer (although removing the allocation should work even with
  * pids in tasks file */
-for (i = 0; i < vm->def->nresctrls; i++)
+for (i = 0; i < vm->def->nresctrls; i++) {
+size_t j = 0;
+
+for (j = 0; j < vm->def->resctrls[i]->nmonitors; j++) {
+virDomainResctrlMonDefPtr mon = NULL;
+
+mon = vm->def->resctrls[i]->monitors[j];
+virResctrlMonitorRemove(mon->instance);
+}
+
 virResctrlAllocRemove(vm->def->resctrls[i]->alloc);
+}
 
 qemuProcessRemoveDomainStatus(driver, vm);
 
@@ -7222,8 +7255,7 @@ void qemuProcessStop(virQEMUDriverPtr driver,
 virPortAllocatorRelease(graphics->data.vnc.port);
 } else if (graphics->data.vnc.portReserved) {
 virPortAllocatorRelease(graphics->data.vnc.port);
-graphics->data.vnc.portReserved = false;
-}
+graphics->data.vnc.portReserved = false; }
 if (graphics->data.vnc.websocketGenerated) {
 virPortAllocatorRelease(graphics->data.vnc.websocket);
 graphics->data.vnc.websocketGenerated = false;
@@ -7939,9 +7971,20 @@ qemuProcessReconnect(void *opaque)
 goto error;
 
 for (i = 0; i < obj->def->nresctrls; i++) {
+size_t j = 0;
+
 if (virResctrlAllocDeterminePath(obj->def->resctrls[i]->alloc,
  priv->machineName) < 0)
 goto error;
+
+for (j = 0; j < obj->def->resctrls[i]->nmonitors; j++) {
+virDomainResctrlMonDefPtr mon = NULL;
+
+mon = obj->def->resctrls[i]->monitors[j];
+if (virResctrlMonitorDeterminePath(mon->instance,
+   priv->machineName) < 0)
+goto error;
+}
 }
 
 /* update domain state XML with possibly updated state in virDomainObj */
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list