[PATCH] allow xendomains to work for xl list -l

[PATCH] allow xendomains to work for xl list -l

[M A Young]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 645 bytes --]

I have discovered a few problems when using the 
tools/hotplug/Linux/init.d/xendomains startup script if you use xl instead 
of xm.
>From xen 4.2 onwards xl list -l gives a JSON format output containing no 
spaces or line feeds, but the xendomains script expects the older format 
(of xl in xen 4.1 and xm) of one key-value pair per line.
This patch adds a new line after each comma in the output of xl list -l 
before processing it further, allows there to be not to be a space between 
the key and value format used by xl list -l and accepts the "Xen saved 
domain" as a valid header for a saved xen image if xl is being used.

 	Michael Young

[-- Attachment #2: Type: TEXT/PLAIN, Size: 2482 bytes --]

Allow xendomains script to work with xl list -l

This patch inserts a line feed after each comma in the output from
xl list -l, allows there not to be a space between "name" and "domid" keys
and their value, and accepts the "Xen saved domain" value as a valid header
for a saved xen image if xl is being used.

Signed-off-by: Michael Young <m.a.young@durham.ac.uk>

--- xen-4.2.2/tools/hotplug/Linux/init.d/xendomains.orig	2013-03-21 17:55:42.000000000 +0000
+++ xen-4.2.2/tools/hotplug/Linux/init.d/xendomains	2013-04-09 00:09:22.784700367 +0100
@@ -212,9 +212,9 @@
     elif [[ "$1" =~ '(domid' ]]; then
         id=$(echo $1 | sed -e 's/^.*(domid \(.*\))$/\1/')
     elif [[ "$1" =~ '"name":' ]]; then
-        name=$(echo $1 | sed -e 's/^.*"name": "\(.*\)",$/\1/')
+        name=$(echo $1 | sed -e 's/^.*"name": *"\(.*\)",$/\1/')
     elif [[ "$1" =~ '"domid":' ]]; then
-        id=$(echo $1 | sed -e 's/^.*"domid": \(.*\),$/\1/')
+        id=$(echo $1 | sed -e 's/^.*"domid": *\(.*\),$/\1/')
     fi
 
     [ -n "$name" -a -n "$id" ] && return 0 || return 1
@@ -233,7 +233,7 @@
 		RC=0
 		;;
 	esac
-    done < <($CMD list -l | grep $LIST_GREP)
+    done < <($CMD list -l | sed -e "s/,/,\n/g" | grep $LIST_GREP)
     return $RC
 }
 
@@ -255,7 +255,7 @@
         for dom in $XENDOMAINS_SAVE/*; do
             if [ -f $dom ] ; then
                 HEADER=`head -c 16 $dom | head -n 1 2> /dev/null`
-                if [ $HEADER = "LinuxGuestRecord" ]; then
+                if [ "$HEADER" = "LinuxGuestRecord" ] || [ "$CMD" = "xl" -a "$HEADER" = "Xen saved domain" ]; then
                     echo -n " ${dom##*/}"
                     XMR=`$CMD restore $dom 2>&1 1>/dev/null`
                     #$CMD restore $dom
@@ -315,7 +315,7 @@
 	if test "$state" != "-b---d" -a "$state" != "-----d"; then
 	    return 1;
 	fi
-    done < <($CMD list -l | grep $LIST_GREP)
+    done < <($CMD list -l | sed -e "s/,/,\n/g" | grep $LIST_GREP)
     return 0
 }
 
@@ -446,7 +446,7 @@
 	    fi
 	    kill $WDOG_PID >/dev/null 2>&1
 	fi
-    done < <($CMD list -l | grep $LIST_GREP)
+    done < <($CMD list -l | sed -e "s/,/,\n/g" | grep $LIST_GREP)
 
     # NB. this shuts down ALL Xen domains (politely), not just the ones in
     # AUTODIR/*
@@ -483,7 +483,7 @@
 		return 0
 		;;
 	esac
-    done < <($CMD list -l | grep $LIST_GREP)
+    done < <($CMD list -l | sed -e "s/,/,\n/g" | grep $LIST_GREP)
     return 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] allow xendomains to work for xl list -l

[Ian Campbell]

On Tue, 2013-04-09 at 21:05 +0100, M A Young wrote:
> From xen 4.2 onwards xl list -l gives a JSON format output containing no 
> spaces or line feeds, but the xendomains script expects the older format 
> (of xl in xen 4.1 and xm) of one key-value pair per line.

Hrm, I'm not sure this change was intentional and I don't recall a patch
which did this on purpose. Ideally xl list would remain somewhat human
readable even if it is also machine readable.

I wonder if this is yajl v1 vs v2 specific? For v1 libxl_yajl_gen_alloc
creates a yajl_gen_config with beautify = 1 and passes it to
yajl_gen_alloc.

For v2 however yajl_gen_alloc doesn't take such an option. It looks like
we are instead supposed to call yajl_gen_config with yajl_gen_beautify.
We probably also want to set yajl_gen_indent_string to "    " (although
that might be the default from my reading).

I don't have a yajl2 test system handy -- could you try that though?

[...]
>  and accepts the "Xen saved 
> domain" as a valid header for a saved xen image if xl is being used.

This bit sounds independently useful too. ISTR someone else sending a
similar patch but it fell through the cracks for some reason which I
don't remember and I can't find it now.

Ian.

Re: [PATCH] allow xendomains to work for xl list -l

[Ian Campbell]

On Wed, 2013-04-10 at 14:00 +0100, Ian Campbell wrote:

> This bit sounds independently useful too. ISTR someone else sending a
> similar patch but it fell through the cracks for some reason which I
> don't remember and I can't find it now.
http://lists.xen.org/archives/html/xen-users/2013-01/msg00099.html
through
http://lists.xen.org/archives/html/xen-users/2013-01/msg00144.html

I've just pinged him to find out what happened to the resend.

Ian.

Re: [PATCH] allow xendomains to work for xl list -l

[M A Young]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1207 bytes --]

On Wed, 10 Apr 2013, Ian Campbell wrote:

> On Tue, 2013-04-09 at 21:05 +0100, M A Young wrote:
>> From xen 4.2 onwards xl list -l gives a JSON format output containing no
>> spaces or line feeds, but the xendomains script expects the older format
>> (of xl in xen 4.1 and xm) of one key-value pair per line.
>
> Hrm, I'm not sure this change was intentional and I don't recall a patch
> which did this on purpose. Ideally xl list would remain somewhat human
> readable even if it is also machine readable.
>
> I wonder if this is yajl v1 vs v2 specific? For v1 libxl_yajl_gen_alloc
> creates a yajl_gen_config with beautify = 1 and passes it to
> yajl_gen_alloc.
>
> For v2 however yajl_gen_alloc doesn't take such an option. It looks like
> we are instead supposed to call yajl_gen_config with yajl_gen_beautify.
> We probably also want to set yajl_gen_indent_string to "    " (although
> that might be the default from my reading).
>
> I don't have a yajl2 test system handy -- could you try that though?

Yes, setting yajl_gen_beautify (as in the attached patch) gets the 
xendomains script working again. I didn't try setting the spaces though 
that does indeed seem to be the default.

 	Michael Young

[-- Attachment #2: Type: TEXT/PLAIN, Size: 462 bytes --]

--- xen-4.2.2/tools/libxl/libxl_json.h.orig	2013-03-21 17:55:42.000000000 +0000
+++ xen-4.2.2/tools/libxl/libxl_json.h	2013-04-10 22:14:15.938459238 +0100
@@ -54,7 +54,10 @@
 
 static inline yajl_gen libxl_yajl_gen_alloc(const yajl_alloc_funcs *allocFuncs)
 {
-    return yajl_gen_alloc(allocFuncs);
+    yajl_gen g;
+    g = yajl_gen_alloc(allocFuncs);
+    yajl_gen_config(g, yajl_gen_beautify, 1);
+    return g;
 }
 
 #else /* !HAVE_YAJL_V2 */

[-- 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] allow xendomains to work for xl list -l

[Ian Campbell]

On Thu, 2013-04-11 at 00:15 +0100, M A Young wrote:
> Yes, setting yajl_gen_beautify (as in the attached patch) gets the 
> xendomains script working again. I didn't try setting the spaces though 
> that does indeed seem to be the default.

Cool. I think you need to check for errors after
	g = yajl_gen_alloc(allocFuncs);	
though.
Ian.

Re: [PATCH] allow xendomains to work for xl list -l

[M A Young]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 616 bytes --]

On Thu, 11 Apr 2013, Ian Campbell wrote:

> On Thu, 2013-04-11 at 00:15 +0100, M A Young wrote:
>> Yes, setting yajl_gen_beautify (as in the attached patch) gets the
>> xendomains script working again. I didn't try setting the spaces though
>> that does indeed seem to be the default.
>
> Cool. I think you need to check for errors after
> 	g = yajl_gen_alloc(allocFuncs);
> though.

This version of the patch checks g isn't NULL before trying to set 
yajl_gen_beautify . I decided not to test whether yajl_gen_beautify is set 
successfully since unbeautified output is probably better than nothing.

 	Michael Young

[-- Attachment #2: Type: TEXT/PLAIN, Size: 652 bytes --]

xl list -l should produce readable output when built with yajl2 so
it is compatible with the xendomains script.

Signed-off-by: Michael Young <m.a.young@durham.ac.uk>

--- xen-4.2.2/tools/libxl/libxl_json.h.orig	2013-03-21 17:55:42.000000000 +0000
+++ xen-4.2.2/tools/libxl/libxl_json.h	2013-04-10 22:14:15.938459238 +0100
@@ -54,7 +54,11 @@
 
 static inline yajl_gen libxl_yajl_gen_alloc(const yajl_alloc_funcs *allocFuncs)
 {
-    return yajl_gen_alloc(allocFuncs);
+    yajl_gen g;
+    g = yajl_gen_alloc(allocFuncs);
+    if (g)
+        yajl_gen_config(g, yajl_gen_beautify, 1);
+    return g;
 }
 
 #else /* !HAVE_YAJL_V2 */

[-- 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] allow xendomains to work for xl list -l

[Ian Campbell]

On Fri, 2013-04-12 at 00:02 +0100, M A Young wrote:
> On Thu, 11 Apr 2013, Ian Campbell wrote:
> 
> > On Thu, 2013-04-11 at 00:15 +0100, M A Young wrote:
> >> Yes, setting yajl_gen_beautify (as in the attached patch) gets the
> >> xendomains script working again. I didn't try setting the spaces though
> >> that does indeed seem to be the default.
> >
> > Cool. I think you need to check for errors after
> > 	g = yajl_gen_alloc(allocFuncs);
> > though.
> 
> This version of the patch checks g isn't NULL before trying to set 
> yajl_gen_beautify . I decided not to test whether yajl_gen_beautify is set 
> successfully since unbeautified output is probably better than nothing.

Agreed. Acked + applied.

Your attachment had DOS line endings for some reason...

Ian.

Re: [PATCH] allow xendomains to work for xl list -l [and 1 more messages]

[Ian Jackson]

M A Young writes ("Re: [PATCH] allow xendomains to work for xl list -l"):
> Yes, setting yajl_gen_beautify (as in the attached patch) gets the 
> xendomains script working again. I didn't try setting the spaces though 
> that does indeed seem to be the default.

Ah, great.  Thanks.  I intend to apply this patch, probably tomorrow
to give anyone a chance to comment.

Ian.

Re: [PATCH] allow xendomains to work for xl list -l [and 1 more messages]

[Ian Campbell]

On Thu, 2013-04-11 at 13:46 +0100, Ian Jackson wrote:
> M A Young writes ("Re: [PATCH] allow xendomains to work for xl list -l"):
> > Yes, setting yajl_gen_beautify (as in the attached patch) gets the 
> > xendomains script working again. I didn't try setting the spaces though 
> > that does indeed seem to be the default.
> 
> Ah, great.  Thanks.  I intend to apply this patch, probably tomorrow
> to give anyone a chance to comment.

I did comment already...
<1365667034.27868.121.camel@zakaz.uk.xensource.com>

Ian.

Re: [PATCH] allow xendomains to work for xl list -l [and 1 more messages]

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH] allow xendomains to work for xl list -l [and 1 more messages]"):
> On Thu, 2013-04-11 at 13:46 +0100, Ian Jackson wrote:
> > M A Young writes ("Re: [PATCH] allow xendomains to work for xl list -l"):
> > > Yes, setting yajl_gen_beautify (as in the attached patch) gets the 
> > > xendomains script working again. I didn't try setting the spaces though 
> > > that does indeed seem to be the default.
> > 
> > Ah, great.  Thanks.  I intend to apply this patch, probably tomorrow
> > to give anyone a chance to comment.
> 
> I did comment already...
> <1365667034.27868.121.camel@zakaz.uk.xensource.com>

Oh yes, I missed thtat.

Thanks,
Ian.

Re: [PATCH] allow xendomains to work for xl list -l

[M A Young]

On Wed, 10 Apr 2013, Ian Campbell wrote:

>>  and accepts the "Xen saved
>> domain" as a valid header for a saved xen image if xl is being used.
>
> This bit sounds independently useful too. ISTR someone else sending a
> similar patch but it fell through the cracks for some reason which I
> don't remember and I can't find it now.

I have thought about this further, and since xm only reads files with the 
"LinuxGuestRecord" header and xl only reads files with the "Xen saved 
domain" header should we be trying to use whichever of xm or xl matches 
the header (assuming it works) regardless of which one we chose at the 
start of the xendomains script.

 	Michael Young

Re: [PATCH] allow xendomains to work for xl list -l

[Ian Campbell]

On Thu, 2013-04-11 at 23:06 +0100, M A Young wrote:
> On Wed, 10 Apr 2013, Ian Campbell wrote:
> 
> >>  and accepts the "Xen saved
> >> domain" as a valid header for a saved xen image if xl is being used.
> >
> > This bit sounds independently useful too. ISTR someone else sending a
> > similar patch but it fell through the cracks for some reason which I
> > don't remember and I can't find it now.
> 
> I have thought about this further, and since xm only reads files with the 
> "LinuxGuestRecord" header and xl only reads files with the "Xen saved 
> domain" header should we be trying to use whichever of xm or xl matches 
> the header (assuming it works) regardless of which one we chose at the 
> start of the xendomains script.

I don't think so, if xend is running then using xl is not
recommended/supported and vice versa. So if xemdomains things one is in
use it shouldn't use the other.

You could perhaps make xendomains print a suitable message but I suppose
xm and xl will both fail on each others format anyway, although perhaps
not with a very useful message?

I'd be happy to see an xl patch which detects xm format save files and
prints something informative, and even happier if there was some
conversion routine (which might be non-trivial though, I haven't looked)

Ian.

Re: [PATCH] allow xendomains to work for xl list -l

[Ian Jackson]

M A Young writes ("[PATCH] allow xendomains to work for xl list -l"):
> I have discovered a few problems when using the 
> tools/hotplug/Linux/init.d/xendomains startup script if you use xl instead 
> of xm.
> >From xen 4.2 onwards xl list -l gives a JSON format output containing no 
> spaces or line feeds, but the xendomains script expects the older format 
> (of xl in xen 4.1 and xm) of one key-value pair per line.
> This patch adds a new line after each comma in the output of xl list -l 
> before processing it further, allows there to be not to be a space between 
> the key and value format used by xl list -l and accepts the "Xen saved 
> domain" as a valid header for a saved xen image if xl is being used.
...
> This patch inserts a line feed after each comma in the output from
> xl list -l, allows there not to be a space between "name" and "domid" keys
> and their value, and accepts the "Xen saved domain" value as a valid header
> for a saved xen image if xl is being used.

Roger, do you have an opinion about this ?  Without testing it myself,
I'm inclined to accept the patch.

This does illuminate the fact that the JSON output is not the most
convenient thing for a shell to deal with.

Ian.