> -----Original Message----- > From: John Ferlan [mailto:jferlan@xxxxxxxxxx] > Sent: Tuesday, November 6, 2018 10:41 PM > To: Wang, Huaqiang <huaqiang.wang@xxxxxxxxx>; libvir-list@xxxxxxxxxx > Cc: Feng, Shaohe <shaohe.feng@xxxxxxxxx>; bing.niu@xxxxxxxxx; Ding, Jian- > feng <jian-feng.ding@xxxxxxxxx>; Zang, Rui <rui.zang@xxxxxxxxx> > 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@xxxxxxxxx> > >>> --- > >>> src/util/virresctrl.c | 30 +++++++++++++++++++----------- > >>> 1 file changed, 19 insertions(+), 11 deletions(-) > >>> > >> Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx> > >> > >> 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 libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
