On Tue, Aug 9, 2011 at 6:02 PM, Sasha Levin <levinsasha928@xxxxxxxxx> wrote:
> On Tue, 2011-08-09 at 17:56 +0300, Pekka Enberg wrote:
>> On Tue, Aug 9, 2011 at 5:53 PM, Ingo Molnar <mingo@xxxxxxx> wrote:
>> >
>> > * Sasha Levin <levinsasha928@xxxxxxxxx> wrote:
>> >
>> >> This patch changes './kvm list' to retrieve process name from
>> >> '/proc/<pid>/stat' instead of '/proc/<pid>/comm' as it appears the latter
>> >> does not exist by default on several systems.
>> >>
>> >> Reported-by: pradeep <psuriset@xxxxxxxxxxxxxxxxxx>
>> >> Signed-off-by: Sasha Levin <levinsasha928@xxxxxxxxx>
>> >> ---
>> >> tools/kvm/builtin-list.c | 21 ++++++++++++++-------
>> >> 1 files changed, 14 insertions(+), 7 deletions(-)
>> >>
>> >> diff --git a/tools/kvm/builtin-list.c b/tools/kvm/builtin-list.c
>> >> index 2d37ecb..89a0465 100644
>> >> --- a/tools/kvm/builtin-list.c
>> >> +++ b/tools/kvm/builtin-list.c
>> >> @@ -13,25 +13,32 @@
>> >> static void print_guest(const char *name, int pid)
>> >> {
>> >> char proc_name[PATH_MAX];
>> >> - char comm[sizeof(PROCESS_NAME)];
>> >> - int fd;
>> >> + char *comm = NULL;
>> >> + FILE *fd;
>> >>
>> >> - sprintf(proc_name, "/proc/%d/comm", pid);
>> >> - fd = open(proc_name, O_RDONLY);
>> >> - if (fd <= 0)
>> >> + sprintf(proc_name, "/proc/%d/stat", pid);
>> >> + fd = fopen(proc_name, "r");
>> >> + if (fd == NULL)
>> >> goto cleanup;
>> >
>> > No, instead we should fall back to 'stat' if the 'comm' access fails.
>> > The 'stat' field contains a lot more data and is thus slower - while
>> > 'comm' only outputs the comm.
>>
>> Doh. I already merged the patch. Sasha, can you please send an
>> incremental one to fix it up?
>
> Does it matter when running './kvm list'? The speed difference isn't
> really noticeable and it's not something that runs all the time.
What's the downside? I think it's a reasonable optimization to do
although like Ingo already said, it's probably not very noticeable.
But hey, you never know what people will use the code for in the
future.
Pekka
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
