Re: [libvirt] [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(a)intel.com>
>> ---
>> src/util/virresctrl.c | 30 +++++++++++++++++++-----------
>> 1 file changed, 19 insertions(+), 11 deletions(-)
>>
> Reviewed-by: John Ferlan <jferlan(a)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.
John