> -----Original Message-----
> From: John Ferlan [mailto:[email protected]]
> Sent: Tuesday, November 6, 2018 10:41 PM
> To: Wang, Huaqiang <[email protected]>; [email protected]
> Cc: Feng, Shaohe <[email protected]>; [email protected]; Ding, Jian-
> feng <[email protected]>; Zang, Rui <[email protected]>
> 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 <[email protected]>
> >>> ---
> >>>   src/util/virresctrl.c | 30 +++++++++++++++++++-----------
> >>>   1 file changed, 19 insertions(+), 11 deletions(-)
> >>>
> >> Reviewed-by: John Ferlan <[email protected]>
> >>
> >> 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

--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to