Re: [Xen-devel] [PATCH 3/3]: xl: allow long listing of Domain 0'

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2010-08-09 at 18:40 +0100, Gianni Tedesco wrote:
> On Mon, 2010-08-09 at 18:36 +0100, Stefano Stabellini wrote:
> > On Mon, 9 Aug 2010, Gianni Tedesco (3P) wrote:
> > > On Mon, 2010-08-09 at 17:11 +0100, Stefano Stabellini wrote:
> > > > On Mon, 9 Aug 2010, Andre Przywara wrote:
> > > > > Hi,
> > > > > 
> > > > > currently xl list aborts when one tries to list -l Domain 0 (either 
> > > > > explicitly or by listing all domains):
> > > > > # xl list -l 0
> > > > > Neither kernel nor bootloader specified
> > > > > 
> > > > > Ignore this error message (which is invalid for Dom0). I haven't 
> > > > > found 
> > > > > an obvious way to check for Dom0 before printing this message, so I 
> > > > > simply removed the exit() call here.
> > > > > 
> > > > 
> > > > I would rather skip dom0 in the list_domains_details loop, I'll apply a
> > > > patch that does that.
> > > 
> > > FWIW I think that's the wrong fix. The config parser code ought not be
> > > the place to check for such things and should be handled in libxl (or
> > > perhaps elsewhere in xl) with reasonable error message. Seems like a
> > > work-around rather than a fix to not print dom0 info.
> > > 
> > > 
> > 
> > I agree, that's why I added a check on dom0 in list_domains_details.
> > The alternative would be to add a check on domid == 0 in
> > libxl_userdata_retrieve, but considering that libxl_userdata_retrieve is
> > supposed to be a generic libxl function, I preferred
> > list_domains_details.
> 
> What I mean is that the kernel vs. bootloader check only affects domain
> create path right? Therefore the check ought to be deep in that code and
> dom0 not skipped in the list_domains_details().

As the one who added the check in question to parse_config_data I agree
that it would be better moved into the relevant caller(s).

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel