Re: [libvirt PATCH 01/13] util: Helper functions to get process info

On Wed, Nov 10, 2021 at 02:49:57PM -0600, Praveen K Paladugu wrote: > > > On 10/29/2021 7:31 AM, Michal Prívozník wrote: >> On 10/22/21 5:37 PM, Praveen K Paladugu wrote: >>> From: Vineeth Pillai <viremana(a)linux.microsoft.com&gt; >>> >>> These helper methods are used to capture vcpu information >>> in ch driver. >>> >>> Signed-off-by: Vineeth Pillai <viremana(a)linux.microsoft.com&gt; >>> Signed-off-by: Praveen K Paladugu <prapal(a)linux.microsoft.com&gt; >>> --- >>> src/util/virprocess.c | 136 ++++++++++++++++++++++++++++++++++++++++++ >>> src/util/virprocess.h | 5 ++ >>> 2 files changed, 141 insertions(+) >>> >>> diff --git a/src/util/virprocess.c b/src/util/virprocess.c >>> index 6de3f36f52..0164d70df6 100644 >>> --- a/src/util/virprocess.c >>> +++ b/src/util/virprocess.c >>> @@ -1721,3 +1721,139 @@ virProcessSetScheduler(pid_t pid G_GNUC_UNUSED, >>> } >>> #endif /* !WITH_SCHED_SETSCHEDULER */ >>> + >>> +/* >>> +TODO: This method was cloned from qemuGetProcessInfo in src/qemu/qemu_driver.c. >>> +Need to refactor qemu driver to use this shared function. >> >> There's no harm in doing that in this patch. In fact, it's desired. You >> can move a qemu function into src/util, rename it and fix all places >> where it is called, all in one patch. >> >> Also, please don't forget to export this function in >> src/libvirt_private.syms. >> > I am working on this now. Will make this part of next version. >>> +*/ >>> +int >>> +virProcessGetStatInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, >>> + pid_t pid, pid_t tid) >> >> Indentation's off. I've noticed other patches in the series suffer from >> mis-indentation too. Please fix that in another version. >> > Regarding indentation, many of these patches add/modify existing files. > Running indent will modify sections of the files not relevant to the code > changes in this patch set. > Should I run "indent" either way? Or should I make all indentation changes > for all files as a single commit? If thre are whitespace bugs in existing code, then it is preferrable to fix them in a standalone commit, so it is easily distinguished from functional changes. Regards, Daniel