Xen devel - Oct 2013 - [PATCH] xl: skip zero-size config data in list_domains_details

libxl_userdata_retrieve returns 0 when there''s no stored config file
for
a domain, so we cannot rely on that to continue processing.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/xl_cmdimpl.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index a8261be..542a971 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -3062,7 +3062,7 @@ static void list_domains_details(const libxl_dominfo
*info, int nb_domain)
         if (info[i].domid == 0)
             continue;
         rc = libxl_userdata_retrieve(ctx, info[i].domid, "xl",
&data, &len);
-        if (rc)
+        if (rc || len == 0)
             continue;
         CHK_ERRNO(asprintf(&config_source, "<domid %d
data>", info[i].domid));
         libxl_domain_config_init(&d_config);
-- 
1.7.10.4

Wei Liu writes ("[PATCH] xl: skip zero-size config data in
list_domains_details"):
> libxl_userdata_retrieve returns 0 when there''s no stored config
file for
> a domain, so we cannot rely on that to continue processing.
How could a zero-sized config userdata ever arise ?

I haven''t tried it but presumably, at the moment, continuing causes
some kind of other errors later (complaints about missing config
entries perhaps) ?

Ian.

On Tue, 2013-10-22 at 10:17 +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH] xl: skip zero-size config data in
list_domains_details"):
> > libxl_userdata_retrieve returns 0 when there''s no stored
config file for
> > a domain, so we cannot rely on that to continue processing.
> 
> How could a zero-sized config userdata ever arise ?
Dom0 or perhaps a domain started by a different toolstack (even one
using libxl)? It''d be nice if xl list would display what it could for
these.

Ian.

On Tue, Oct 22, 2013 at 10:17:00AM +0100, Ian Jackson
wrote:
> Wei Liu writes ("[PATCH] xl: skip zero-size config data in
list_domains_details"):
> > libxl_userdata_retrieve returns 0 when there''s no stored
config file for
> > a domain, so we cannot rely on that to continue processing.
> 
> How could a zero-sized config userdata ever arise ?
> 
When Xen cannot clean up a domain after its death, it remains visible in
Xen in -p-sd state. Xl list -l will still try to retrieve the config
file, which has been deleted at that time.
> I haven''t tried it but presumably, at the moment, continuing
causes
> some kind of other errors later (complaints about missing config
> entries perhaps) ?
> 
Yes, this leads to ''Domain name must be specified'' because the
config
data is NULL, aborting ''xl list -l''.

Wei.
> Ian.

On Tue, Oct 22, 2013 at 10:27:39AM +0100, Wei Liu wrote:
> On Tue, Oct 22, 2013 at 10:17:00AM +0100, Ian Jackson wrote:
> > Wei Liu writes ("[PATCH] xl: skip zero-size config data in
list_domains_details"):
> > > libxl_userdata_retrieve returns 0 when there''s no stored
config file for
> > > a domain, so we cannot rely on that to continue processing.
> > 
> > How could a zero-sized config userdata ever arise ?
> > 
> 
> When Xen cannot clean up a domain after its death, it remains visible in
> Xen in -p-sd state. Xl list -l will still try to retrieve the config
> file, which has been deleted at that time.
> 
> > I haven''t tried it but presumably, at the moment, continuing
causes
> > some kind of other errors later (complaints about missing config
> > entries perhaps) ?
> > 
> 
> Yes, this leads to ''Domain name must be specified''
because the config
> data is NULL, aborting ''xl list -l''.
> 
Just to make clear, ''Domain name must be specified''
doesn''t cause the
abortion of ''xl list -l'', it''s the call to generate
json output that
aborts ''xl list -l''.

Wei.
> Wei.
> 
> > Ian.

Wei Liu writes ("Re: [PATCH] xl: skip zero-size config data in
list_domains_details"):
> When Xen cannot clean up a domain after its death, it remains visible in
> Xen in -p-sd state. Xl list -l will still try to retrieve the config
> file, which has been deleted at that time.
...
> Yes, this leads to ''Domain name must be specified''
because the config
> data is NULL, aborting ''xl list -l''.
Aha.  Right.  I have just looked at the context (primarily, 8cde53d0)
and you''re entirely right with this change.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Ian.

On Tue, 2013-10-22 at 16:52 +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH] xl: skip zero-size config data in
list_domains_details"):
> > When Xen cannot clean up a domain after its death, it remains visible
in
> > Xen in -p-sd state. Xl list -l will still try to retrieve the config
> > file, which has been deleted at that time.
> ...
> > Yes, this leads to ''Domain name must be specified''
because the config
> > data is NULL, aborting ''xl list -l''.
> 
> Aha.  Right.  I have just looked at the context (primarily, 8cde53d0)
> and you''re entirely right with this change.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
I think I saw a conversation this morning where you decided you had been
mistaken somewhere, I think here (IIRC the suggestions was that xl list
and xl list -l should contain the same set of domains).

For the avoidance of confusion perhaps you should apply this and/or
whatever other patch was under discussion...

Ian.