Re: [libvirt] [PATCHv5 14/19] Util: Add function for checking if monitor is running
> -Original Message- > From: John Ferlan [mailto:jfer...@redhat.com] > Sent: Thursday, October 11, 2018 4:58 AM > To: Wang, Huaqiang ; libvir-list@redhat.com > Cc: Feng, Shaohe ; Niu, Bing ; > Ding, Jian-feng ; Zang, Rui > Subject: Re: [libvirt] [PATCHv5 14/19] Util: Add function for checking if > monitor > is running > > > > On 10/9/18 6:30 AM, Wang Huaqiang wrote: > > Check monitor status by checking the PIDs are in file 'task' > > or not. > > > > Signed-off-by: Wang Huaqiang > > --- > > src/libvirt_private.syms | 1 + > > src/util/virresctrl.c| 84 > +++- > > src/util/virresctrl.h| 4 +++ > > 3 files changed, 88 insertions(+), 1 deletion(-) > > > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index > > 4b22ed4..c90e48a 100644 > > --- a/src/libvirt_private.syms > > +++ b/src/libvirt_private.syms > > @@ -2686,6 +2686,7 @@ virResctrlMonitorDeterminePath; > > virResctrlMonitorGetCacheLevel; virResctrlMonitorGetCacheOccupancy; > > virResctrlMonitorGetID; > > +virResctrlMonitorIsRunning; > > virResctrlMonitorNew; > > virResctrlMonitorRemove; > > virResctrlMonitorSetCacheLevel; > > diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index > > 41e8d48..67dfbb8 100644 > > --- a/src/util/virresctrl.c > > +++ b/src/util/virresctrl.c > > @@ -364,6 +364,9 @@ struct _virResctrlMonitor { > > char *path; > > /* Boolean flag for default monitor */ > > bool default_monitor; > > +/* Tracking the tasks' PID associated with this monitor */ > > +pid_t *pids; > > +size_t npids; > > /* The cache 'level', special for cache monitor */ > > unsigned int cache_level; > > }; > > @@ -425,6 +428,7 @@ virResctrlMonitorDispose(void *obj) > > virObjectUnref(monitor->alloc); > > VIR_FREE(monitor->id); > > VIR_FREE(monitor->path); > > +VIR_FREE(monitor->pids); > > } > > > > > > @@ -2491,7 +2495,13 @@ int > > virResctrlMonitorAddPID(virResctrlMonitorPtr monitor, > > pid_t pid) > > { > > -return virResctrlAddPID(monitor->path, pid); > > +if (virResctrlAddPID(monitor->path, pid) < 0) > > +return -1; > > + > > +if (VIR_APPEND_ELEMENT(monitor->pids, monitor->npids, pid) < 0) > > +return -1; > > + > > +return 0; > > } > > > > > > @@ -2762,3 +2772,75 @@ > > virResctrlMonitorSetDefault(virResctrlMonitorPtr monitor) { > > monitor->default_monitor = true; > > } > > + > > + > > +static int > > +virResctrlPIDCompare(const void *pida, const void *pidb) { > > +return *(pid_t*)pida - *(pid_t*)pidb; } > > + > > + > > +bool > > +virResctrlMonitorIsRunning(virResctrlMonitorPtr monitor) { > > +char *pidstr = NULL; > > +char **spids = NULL; > > +size_t nspids = 0; > > +pid_t *pids = NULL; > > +size_t npids = 0; > > +size_t i = 0; > > +int rv = -1; > > +bool ret = false; > > + > > +if (!monitor->path) > > +return false; > > + > > +if (monitor->npids == 0) > > +return false; > > + > > +rv = virFileReadValueString(, "%s/tasks", monitor->path); > > +if (rv == -2) > > +virReportError(VIR_ERR_INTERNAL_ERROR, > > + _("Task file '%s/tasks' does not exist"), > > + monitor->path); > > +if (rv < 0) > > +goto cleanup; > > + > > +/* no PID in task file */ > > +if (!*pidstr) > > +goto cleanup; > > + > > +spids = virStringSplitCount(pidstr, "\n", 0, ); > > +if (nspids != monitor->npids) > > +return false; > > This will cause a leak... "goto cleanup;" My bad. Will be corrected. > > > + > > +for (i = 0; i < nspids; i++) { > > +unsigned int val = 0; > > +pid_t pid = 0; > > + > > +if (virStrToLong_uip(spids[i], NULL, 0, ) < 0) > > +goto cleanup; > > + > > +pid = (pid_t)val; > > + > > +if (VIR_APPEND_ELEMENT(pids, npids, pid) < 0) > > +goto cleanup; > > +} > > + > > +qsort(pids, npids, sizeof(pid_t), virResctrlPIDCompare); > > +qsort(monitor->pids, monitor->npids, sizeof(pid_t), > > + virResctrlPIDCompare); > > + > > +for (i = 0; i < monitor->npids; i++) { > > +if (monitor->pids[i] != pids[i]) > > +goto cleanup; > > +} > > + > > +ret = true; > > + cleanup: > > NB: Any place where you get here without an error message, but w/ ret = false > may cause "issues" for the caller especially since some of the ways to get > here > w/ @false do set an error message. Got. Will add some comments for each return and each 'goto cleanup', and will refine the error message. > > John > Thanks for review. Huaqiang > > +virStringListFree(spids); > > +VIR_FREE(pids); > > +VIR_FREE(pidstr); > > + > > +return ret; > > +} > > diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h index > > 371df8a..c5794cb 100644 > > --- a/src/util/virresctrl.h > > +++
Re: [libvirt] [PATCHv5 14/19] Util: Add function for checking if monitor is running
On 10/9/18 6:30 AM, Wang Huaqiang wrote: > Check monitor status by checking the PIDs are in file 'task' > or not. > > Signed-off-by: Wang Huaqiang > --- > src/libvirt_private.syms | 1 + > src/util/virresctrl.c| 84 > +++- > src/util/virresctrl.h| 4 +++ > 3 files changed, 88 insertions(+), 1 deletion(-) > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index 4b22ed4..c90e48a 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -2686,6 +2686,7 @@ virResctrlMonitorDeterminePath; > virResctrlMonitorGetCacheLevel; > virResctrlMonitorGetCacheOccupancy; > virResctrlMonitorGetID; > +virResctrlMonitorIsRunning; > virResctrlMonitorNew; > virResctrlMonitorRemove; > virResctrlMonitorSetCacheLevel; > diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c > index 41e8d48..67dfbb8 100644 > --- a/src/util/virresctrl.c > +++ b/src/util/virresctrl.c > @@ -364,6 +364,9 @@ struct _virResctrlMonitor { > char *path; > /* Boolean flag for default monitor */ > bool default_monitor; > +/* Tracking the tasks' PID associated with this monitor */ > +pid_t *pids; > +size_t npids; > /* The cache 'level', special for cache monitor */ > unsigned int cache_level; > }; > @@ -425,6 +428,7 @@ virResctrlMonitorDispose(void *obj) > virObjectUnref(monitor->alloc); > VIR_FREE(monitor->id); > VIR_FREE(monitor->path); > +VIR_FREE(monitor->pids); > } > > > @@ -2491,7 +2495,13 @@ int > virResctrlMonitorAddPID(virResctrlMonitorPtr monitor, > pid_t pid) > { > -return virResctrlAddPID(monitor->path, pid); > +if (virResctrlAddPID(monitor->path, pid) < 0) > +return -1; > + > +if (VIR_APPEND_ELEMENT(monitor->pids, monitor->npids, pid) < 0) > +return -1; > + > +return 0; > } > > > @@ -2762,3 +2772,75 @@ virResctrlMonitorSetDefault(virResctrlMonitorPtr > monitor) > { > monitor->default_monitor = true; > } > + > + > +static int > +virResctrlPIDCompare(const void *pida, const void *pidb) > +{ > +return *(pid_t*)pida - *(pid_t*)pidb; > +} > + > + > +bool > +virResctrlMonitorIsRunning(virResctrlMonitorPtr monitor) > +{ > +char *pidstr = NULL; > +char **spids = NULL; > +size_t nspids = 0; > +pid_t *pids = NULL; > +size_t npids = 0; > +size_t i = 0; > +int rv = -1; > +bool ret = false; > + > +if (!monitor->path) > +return false; > + > +if (monitor->npids == 0) > +return false; > + > +rv = virFileReadValueString(, "%s/tasks", monitor->path); > +if (rv == -2) > +virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Task file '%s/tasks' does not exist"), > + monitor->path); > +if (rv < 0) > +goto cleanup; > + > +/* no PID in task file */ > +if (!*pidstr) > +goto cleanup; > + > +spids = virStringSplitCount(pidstr, "\n", 0, ); > +if (nspids != monitor->npids) > +return false; This will cause a leak... "goto cleanup;" > + > +for (i = 0; i < nspids; i++) { > +unsigned int val = 0; > +pid_t pid = 0; > + > +if (virStrToLong_uip(spids[i], NULL, 0, ) < 0) > +goto cleanup; > + > +pid = (pid_t)val; > + > +if (VIR_APPEND_ELEMENT(pids, npids, pid) < 0) > +goto cleanup; > +} > + > +qsort(pids, npids, sizeof(pid_t), virResctrlPIDCompare); > +qsort(monitor->pids, monitor->npids, sizeof(pid_t), > virResctrlPIDCompare); > + > +for (i = 0; i < monitor->npids; i++) { > +if (monitor->pids[i] != pids[i]) > +goto cleanup; > +} > + > +ret = true; > + cleanup: NB: Any place where you get here without an error message, but w/ ret = false may cause "issues" for the caller especially since some of the ways to get here w/ @false do set an error message. John > +virStringListFree(spids); > +VIR_FREE(pids); > +VIR_FREE(pidstr); > + > +return ret; > +} > diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h > index 371df8a..c5794cb 100644 > --- a/src/util/virresctrl.h > +++ b/src/util/virresctrl.h > @@ -230,4 +230,8 @@ virResctrlMonitorGetCacheOccupancy(virResctrlMonitorPtr > monitor, > unsigned int **bankcaches); > void > virResctrlMonitorSetDefault(virResctrlMonitorPtr monitor); > + > +bool > +virResctrlMonitorIsRunning(virResctrlMonitorPtr monitor); > + > #endif /* __VIR_RESCTRL_H__ */ > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv5 14/19] Util: Add function for checking if monitor is running
Check monitor status by checking the PIDs are in file 'task' or not. Signed-off-by: Wang Huaqiang --- src/libvirt_private.syms | 1 + src/util/virresctrl.c| 84 +++- src/util/virresctrl.h| 4 +++ 3 files changed, 88 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4b22ed4..c90e48a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2686,6 +2686,7 @@ virResctrlMonitorDeterminePath; virResctrlMonitorGetCacheLevel; virResctrlMonitorGetCacheOccupancy; virResctrlMonitorGetID; +virResctrlMonitorIsRunning; virResctrlMonitorNew; virResctrlMonitorRemove; virResctrlMonitorSetCacheLevel; diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 41e8d48..67dfbb8 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -364,6 +364,9 @@ struct _virResctrlMonitor { char *path; /* Boolean flag for default monitor */ bool default_monitor; +/* Tracking the tasks' PID associated with this monitor */ +pid_t *pids; +size_t npids; /* The cache 'level', special for cache monitor */ unsigned int cache_level; }; @@ -425,6 +428,7 @@ virResctrlMonitorDispose(void *obj) virObjectUnref(monitor->alloc); VIR_FREE(monitor->id); VIR_FREE(monitor->path); +VIR_FREE(monitor->pids); } @@ -2491,7 +2495,13 @@ int virResctrlMonitorAddPID(virResctrlMonitorPtr monitor, pid_t pid) { -return virResctrlAddPID(monitor->path, pid); +if (virResctrlAddPID(monitor->path, pid) < 0) +return -1; + +if (VIR_APPEND_ELEMENT(monitor->pids, monitor->npids, pid) < 0) +return -1; + +return 0; } @@ -2762,3 +2772,75 @@ virResctrlMonitorSetDefault(virResctrlMonitorPtr monitor) { monitor->default_monitor = true; } + + +static int +virResctrlPIDCompare(const void *pida, const void *pidb) +{ +return *(pid_t*)pida - *(pid_t*)pidb; +} + + +bool +virResctrlMonitorIsRunning(virResctrlMonitorPtr monitor) +{ +char *pidstr = NULL; +char **spids = NULL; +size_t nspids = 0; +pid_t *pids = NULL; +size_t npids = 0; +size_t i = 0; +int rv = -1; +bool ret = false; + +if (!monitor->path) +return false; + +if (monitor->npids == 0) +return false; + +rv = virFileReadValueString(, "%s/tasks", monitor->path); +if (rv == -2) +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Task file '%s/tasks' does not exist"), + monitor->path); +if (rv < 0) +goto cleanup; + +/* no PID in task file */ +if (!*pidstr) +goto cleanup; + +spids = virStringSplitCount(pidstr, "\n", 0, ); +if (nspids != monitor->npids) +return false; + +for (i = 0; i < nspids; i++) { +unsigned int val = 0; +pid_t pid = 0; + +if (virStrToLong_uip(spids[i], NULL, 0, ) < 0) +goto cleanup; + +pid = (pid_t)val; + +if (VIR_APPEND_ELEMENT(pids, npids, pid) < 0) +goto cleanup; +} + +qsort(pids, npids, sizeof(pid_t), virResctrlPIDCompare); +qsort(monitor->pids, monitor->npids, sizeof(pid_t), virResctrlPIDCompare); + +for (i = 0; i < monitor->npids; i++) { +if (monitor->pids[i] != pids[i]) +goto cleanup; +} + +ret = true; + cleanup: +virStringListFree(spids); +VIR_FREE(pids); +VIR_FREE(pidstr); + +return ret; +} diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h index 371df8a..c5794cb 100644 --- a/src/util/virresctrl.h +++ b/src/util/virresctrl.h @@ -230,4 +230,8 @@ virResctrlMonitorGetCacheOccupancy(virResctrlMonitorPtr monitor, unsigned int **bankcaches); void virResctrlMonitorSetDefault(virResctrlMonitorPtr monitor); + +bool +virResctrlMonitorIsRunning(virResctrlMonitorPtr monitor); + #endif /* __VIR_RESCTRL_H__ */ -- 2.7.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list