From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ingo Molnar Subject: Re: [PATCH] kvm tools: Change method of retrieving process name Date: Tue, 9 Aug 2011 16:59:20 +0200 Message-ID: <20110809145920.GD28228@elte.hu> References: <1312885059-842-1-git-send-email-levinsasha928@gmail.com> <20110809145331.GC28228@elte.hu> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Sasha Levin , psuriset@linux.vnet.ibm.com, asias.hejun@gmail.com, prasadjoshi124@gmail.com, gorcunov@gmail.com, kvm@vger.kernel.org To: Pekka Enberg Return-path: Received: from mx3.mail.elte.hu ([157.181.1.138]:43973 "EHLO mx3.mail.elte.hu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751250Ab1HIPAM (ORCPT ); Tue, 9 Aug 2011 11:00:12 -0400 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: * Pekka Enberg wrote: > On Tue, Aug 9, 2011 at 5:53 PM, Ingo Molnar wrote: > > > > * Sasha Levin wrote: > > > >> This patch changes './kvm list' to retrieve process name from > >> '/proc//stat' instead of '/proc//comm' as it appears the= latter > >> does not exist by default on several systems. > >> > >> Reported-by: pradeep > >> Signed-off-by: Sasha Levin > >> --- > >> =A0tools/kvm/builtin-list.c | =A0 21 ++++++++++++++------- > >> =A01 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 @@ > >> =A0static void print_guest(const char *name, int pid) > >> =A0{ > >> =A0 =A0 =A0 char proc_name[PATH_MAX]; > >> - =A0 =A0 char comm[sizeof(PROCESS_NAME)]; > >> - =A0 =A0 int fd; > >> + =A0 =A0 char *comm =3D NULL; > >> + =A0 =A0 FILE *fd; > >> > >> - =A0 =A0 sprintf(proc_name, "/proc/%d/comm", pid); > >> - =A0 =A0 fd =3D open(proc_name, O_RDONLY); > >> - =A0 =A0 if (fd <=3D 0) > >> + =A0 =A0 sprintf(proc_name, "/proc/%d/stat", pid); > >> + =A0 =A0 fd =3D fopen(proc_name, "r"); > >> + =A0 =A0 if (fd =3D=3D NULL) > >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto cleanup; > > > > No, instead we should fall back to 'stat' if the 'comm' access fail= s. > > The 'stat' field contains a lot more data and is thus slower - whil= e > > 'comm' only outputs the comm. >=20 > Doh. I already merged the patch. Sasha, can you please send an > incremental one to fix it up? well, it's not bad code per se - the speedup probably isnt really=20 noticeable either - but we should do it nevertheless, out of=20 principle :) Thanks, Ingo