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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
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.
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.
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
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.
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.
Ian Jackson
2013-Apr-11 12:46 UTC
Re: [PATCH] allow xendomains to work for xl list -l [and 1 more messages]
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.
Ian Campbell
2013-Apr-11 13:10 UTC
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> Ian.
Ian Jackson
2013-Apr-11 15:56 UTC
Re: [PATCH] allow xendomains to work for xl list -l [and 1 more messages]
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.
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
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
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.
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.