-----Original Message----- From: John Ferlan [mailto:jferlan@redhat.com] Sent: Tuesday, November 6, 2018 10:41 PM To: Wang, Huaqiang <huaqiang.wang@intel.com>; libvir-list@redhat.com Cc: Feng, Shaohe <shaohe.feng@intel.com>; bing.niu@intel.com; Ding, Jian- feng <jian-feng.ding@intel.com>; Zang, Rui <rui.zang@intel.com> Subject: Re: [PATCHv7 05/18] util: Refactor code for adding PID to the resource group
On 11/6/18 3:41 AM, Huaqiang,Wang wrote:
On 2018年11月05日 23:03, John Ferlan wrote:
On 10/22/18 4:01 AM, Wang Huaqiang wrote:
The code of adding PID to the allocation could be reused, refactor it for later reuse.
Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com> --- src/util/virresctrl.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
Thanks for the review. Huaqiang
While working through patch1 adjustments, I noted an extra space in a comment, so I fixed it:
/* If the allocation is empty, then it is impossible to add a PID to * allocation due to lacking of its 'tasks' file. Just return */ ^^ Of course that resulted in a merge conflict in this patch where I (now) note you changed the comment to:
/* If allocation is empty, then no resctrl directory and the 'tasks' file * exists, just return */
I'm going to restore the original comment; however, it made me stop and think about future patch14 which used the *tasks (and a new local *pid list) - perhaps you need to rethink the changes... Even patch1...
What's the use of altering the *tasks file at all then? Fortunately it seems it's not really used for much more than logging the tids that are running the vcpu.
I'll hold off on pushing anything... So far there's no real impact, but you may decide that you need to 'design in' a way to handle this default/system path/issue.
I'll send out my v8 patches today, and will be covered in my new series, please make a review. Thanks for review.
John