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

[Praveen K Paladugu]

On 11/11/2021 3:25 AM, Michal Prívozník wrote:
> On 11/10/21 9:49 PM, 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 at linux.microsoft.com>
>>>>
>>>> These helper methods are used to capture vcpu information
>>>> in ch driver.
>>>>
>>>> Signed-off-by: Vineeth Pillai <viremana at linux.microsoft.com>
>>>> Signed-off-by: Praveen K Paladugu <prapal at linux.microsoft.com>
>>>> ---
>>>>    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?
> 
> We like to have one semantic change per commit. Therefore, if you are
> fixing indentation in a code that's unrelated (i.e. you are fixing
> indentation in a function you are not touching) then that's considered
> as another semantic change.
> 
> What I was aiming at is that new code should be indented well from the
> beginning. That's obviously one semantic change and as such should be in
> one patch (i.e. new code that's well formatted).
> 
> Have I answered your question or did I misunderstand?
> 
> Michal
> 
Thanks Michal.

I will keep this in mind for V2 of this patch set.

-- 
Regards,
Praveen K Paladugu