[PATCH] xen-tmem-list-parse: fix ugly parse output

[PATCH] xen-tmem-list-parse: fix ugly parse output

[Dan Magenheimer]

[-- Attachment #1: Type: text/plain, Size: 1635 bytes --]

Hmmm... It appears I never posted the corrected version of this patch
so it never made it upstream.  See:

http://lists.xen.org/archives/html/xen-devel/2011-11/msg00587.html 
http://lists.xen.org/archives/html/xen-devel/2011-11/msg02145.html 

It would be good if this very minor fix was also applied to 4.2
(and, if possible, 4.1).

Thanks,
Dan

===================

The program xen-tmem-list-parse parses the output of xm/xl tmem-list
into human-readable format.  A missing NULL terminator sometimes
causes garbage to be spewed where the two-letter pool type
should be output.

Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
---
 tools/misc/xen-tmem-list-parse.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/misc/xen-tmem-list-parse.c b/tools/misc/xen-tmem-list-parse.c
index 977e4d3..f32b107 100644
--- a/tools/misc/xen-tmem-list-parse.c
+++ b/tools/misc/xen-tmem-list-parse.c
@@ -243,6 +243,7 @@ void parse_pool(char *s)
     unsigned long long flush_objs = parse(s,"ot");
 
     parse_string(s,"PT",pool_type,2);
+    pool_type[2] = '\0';
     if (pool_type[1] == 'S')
         return; /* no need to repeat print data for shared pools */
     printf("domid%lu,id%lu[%s]:pgp=%llu(max=%llu) obj=%llu(%llu) "
@@ -286,6 +287,7 @@ void parse_shared_pool(char *s)
     unsigned long long flush_objs = parse(s,"ot");
 
     parse_string(s,"PT",pool_type,2);
+    pool_type[2] = '\0';
     parse_sharers(s,"SC",buf,BUFSIZE);
     printf("poolid=%lu[%s] uuid=%llx.%llx, shared-by:%s: "
            "pgp=%llu(max=%llu) obj=%llu(%llu) "
-- 
1.7.1

[-- Attachment #2: 0001-Fix-ugly-parse-output-for-xen-tmem-list-parse.patch --]
[-- Type: application/octet-stream, Size: 1374 bytes --]

From: Dan Magenheimer <dan.magenheimer@oracle.com>
Date: Thu, 1 Nov 2012 10:24:56 -0600
Subject: [PATCH] Fix ugly parse output for xen-tmem-list-parse

The program xen-tmem-list-parse parses the output of xm/xl tmem-list
into human-readable format.  A missing NULL terminator sometimes
causes garbage to be spewed where the two-letter pool type
should be output.

Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
---
 tools/misc/xen-tmem-list-parse.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/misc/xen-tmem-list-parse.c b/tools/misc/xen-tmem-list-parse.c
index 977e4d3..f32b107 100644
--- a/tools/misc/xen-tmem-list-parse.c
+++ b/tools/misc/xen-tmem-list-parse.c
@@ -243,6 +243,7 @@ void parse_pool(char *s)
     unsigned long long flush_objs = parse(s,"ot");
 
     parse_string(s,"PT",pool_type,2);
+    pool_type[2] = '\0';
     if (pool_type[1] == 'S')
         return; /* no need to repeat print data for shared pools */
     printf("domid%lu,id%lu[%s]:pgp=%llu(max=%llu) obj=%llu(%llu) "
@@ -286,6 +287,7 @@ void parse_shared_pool(char *s)
     unsigned long long flush_objs = parse(s,"ot");
 
     parse_string(s,"PT",pool_type,2);
+    pool_type[2] = '\0';
     parse_sharers(s,"SC",buf,BUFSIZE);
     printf("poolid=%lu[%s] uuid=%llx.%llx, shared-by:%s: "
            "pgp=%llu(max=%llu) obj=%llu(%llu) "
-- 
1.7.1


[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] xen-tmem-list-parse: fix ugly parse output

[Dan Magenheimer]

Ping?  (Sorry if I am being too impatient... I just saw
a tmem hypervisor fix go in and it reminded me to look
for this tools fix.)

> -----Original Message-----
> From: Dan Magenheimer
> Sent: Thursday, November 01, 2012 10:43 AM
> To: Ian Campbell; Ian Jackson
> Cc: xen-devel@lists.xen.org
> Subject: [PATCH] xen-tmem-list-parse: fix ugly parse output
> 
> Hmmm... It appears I never posted the corrected version of this patch
> so it never made it upstream.  See:
> 
> http://lists.xen.org/archives/html/xen-devel/2011-11/msg00587.html
> http://lists.xen.org/archives/html/xen-devel/2011-11/msg02145.html
> 
> It would be good if this very minor fix was also applied to 4.2
> (and, if possible, 4.1).
> 
> Thanks,
> Dan
> 
> ===================
> 
> The program xen-tmem-list-parse parses the output of xm/xl tmem-list
> into human-readable format.  A missing NULL terminator sometimes
> causes garbage to be spewed where the two-letter pool type
> should be output.
> 
> Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
> ---
>  tools/misc/xen-tmem-list-parse.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/misc/xen-tmem-list-parse.c b/tools/misc/xen-tmem-list-parse.c
> index 977e4d3..f32b107 100644
> --- a/tools/misc/xen-tmem-list-parse.c
> +++ b/tools/misc/xen-tmem-list-parse.c
> @@ -243,6 +243,7 @@ void parse_pool(char *s)
>      unsigned long long flush_objs = parse(s,"ot");
> 
>      parse_string(s,"PT",pool_type,2);
> +    pool_type[2] = '\0';
>      if (pool_type[1] == 'S')
>          return; /* no need to repeat print data for shared pools */
>      printf("domid%lu,id%lu[%s]:pgp=%llu(max=%llu) obj=%llu(%llu) "
> @@ -286,6 +287,7 @@ void parse_shared_pool(char *s)
>      unsigned long long flush_objs = parse(s,"ot");
> 
>      parse_string(s,"PT",pool_type,2);
> +    pool_type[2] = '\0';
>      parse_sharers(s,"SC",buf,BUFSIZE);
>      printf("poolid=%lu[%s] uuid=%llx.%llx, shared-by:%s: "
>             "pgp=%llu(max=%llu) obj=%llu(%llu) "
> --
> 1.7.1

Re: [PATCH] xen-tmem-list-parse: fix ugly parse output

[Ian Campbell]

On Wed, 2012-11-14 at 00:08 +0000, Dan Magenheimer wrote:

> > Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com> and applied, sorry for
the delay.

> > ---
> >  tools/misc/xen-tmem-list-parse.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/tools/misc/xen-tmem-list-parse.c b/tools/misc/xen-tmem-list-parse.c
> > index 977e4d3..f32b107 100644
> > --- a/tools/misc/xen-tmem-list-parse.c
> > +++ b/tools/misc/xen-tmem-list-parse.c
> > @@ -243,6 +243,7 @@ void parse_pool(char *s)
> >      unsigned long long flush_objs = parse(s,"ot");
> > 
> >      parse_string(s,"PT",pool_type,2);
> > +    pool_type[2] = '\0';
> >      if (pool_type[1] == 'S')
> >          return; /* no need to repeat print data for shared pools */
> >      printf("domid%lu,id%lu[%s]:pgp=%llu(max=%llu) obj=%llu(%llu) "
> > @@ -286,6 +287,7 @@ void parse_shared_pool(char *s)
> >      unsigned long long flush_objs = parse(s,"ot");
> > 
> >      parse_string(s,"PT",pool_type,2);
> > +    pool_type[2] = '\0';
> >      parse_sharers(s,"SC",buf,BUFSIZE);
> >      printf("poolid=%lu[%s] uuid=%llx.%llx, shared-by:%s: "
> >             "pgp=%llu(max=%llu) obj=%llu(%llu) "
> > --
> > 1.7.1

Re: [PATCH] xen-tmem-list-parse: fix ugly parse output

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH] xen-tmem-list-parse: fix ugly parse output"):
> On Wed, 2012-11-14 at 00:08 +0000, Dan Magenheimer wrote:
> 
> > > Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com> and applied, sorry for
> the delay.

I'm happy to backport this to 4.2.  Since none of this is tested by
the autotests and the change is self-contained, there is no point
waiting to see if it survives in -unstable and I have done so right
away.

As for 4.1, I think that's really very much in the deep freeze and
receiving essential bug/compatibility/security fixes only so I would
be inclined to punt on this patch.

I have another comment though:

  The program xen-tmem-list-parse parses the output of xm/xl tmem-list
  into human-readable format.  A missing NULL terminator sometimes
  causes garbage to be spewed where the two-letter pool type
  should be output.

Would it not be much better if  xl tmem-list  produced a sane format
to start with ?  I haven't seen what the old and new formats are like
but in general I think  xl foo-list  should produce information useful
to humans.

In xl for the future we should be providing json output for things
which need to be machine-readable.  This may need a transition plan of
course.

Ian.

Re: [PATCH] xen-tmem-list-parse: fix ugly parse output

[Dan Magenheimer]

> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Subject: Re: [PATCH] xen-tmem-list-parse: fix ugly parse output
> 
> Ian Campbell writes ("Re: [PATCH] xen-tmem-list-parse: fix ugly parse output"):
> > On Wed, 2012-11-14 at 00:08 +0000, Dan Magenheimer wrote:
> >
> > > > Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
> >
> > Acked-by: Ian Campbell <ian.campbell@citrix.com> and applied, sorry for
> > the delay.
> 
> I'm happy to backport this to 4.2.  Since none of this is tested by
> the autotests and the change is self-contained, there is no point
> waiting to see if it survives in -unstable and I have done so right
> away.
> 
> As for 4.1, I think that's really very much in the deep freeze and
> receiving essential bug/compatibility/security fixes only so I would
> be inclined to punt on this patch.
> 
> I have another comment though:
> 
>   The program xen-tmem-list-parse parses the output of xm/xl tmem-list
>   into human-readable format.  A missing NULL terminator sometimes
>   causes garbage to be spewed where the two-letter pool type
>   should be output.
> 
> Would it not be much better if  xl tmem-list  produced a sane format
> to start with ?  I haven't seen what the old and new formats are like
> but in general I think  xl foo-list  should produce information useful
> to humans.
> 
> In xl for the future we should be providing json output for things
> which need to be machine-readable.  This may need a transition plan of
> course.

The real consumer of the tmem-list is a toolstack.  It contains a wealth
of information that might be useful to automatic balancing tools
(for balancing across multiple machines, not so much VMs within a single
machine).  So a long-winded-hard-to-parse-but-human-readable format
didn't seem wise.  Also, because it was not clear at the time (and
still isn't) which of the listed data would be useful, or if more
would be even more useful, I chose a format where more or less
data could be provided forwards- and backwards-compatible across
multiple generations of tools parsing the data.

BUT, the data is also useful for debugging and for experimenters
trying tmem to observe what is going on (I often used
"watch -d 'xm tmem-list | xen-tmem-list-parse'"),
so I hacked up a parsing program to make it human readable.

That's just the history... I am very open to a different approach
as long as it meets the needs described above.  Someone with
a different set of skills than I will need to take it on though.

Thanks,
Dan