Re: [Xen-devel] [PATCH] xl: Check for dependencies in xl

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2011-01-13 at 16:57 +0000, Kamala Narasimhan wrote: 
> If I fail to start xencommons before starting a vm, I get a non
> obvious "failed to free memory for the domain" error!  This patch
> checks for dependencies when xl is invoked.
> 
> Note:  Currently I am checking for xenstore daemon only.  More checks
> could be added if necessary.  Also, I am performing the check
> irrespective of the kind of command being invoked through xl.
> Example. xl info, list etc.  doesn't require that xenstored be
> running.  But expecting xenstored be running before invoking the
> toolstack seem like a reasonable expectation in general.  Please
> consider this for inclusion or feel free to tweak it as necessary.

Having just tripped over this on a new install where xenstored wasn't
started at the default runlevel I like this idea.

> Signed-off-by: Kamala Narasimhan <kamala.narasimhan@xxxxxxxxxx>
> 
> Kamala
> 
> diff -r ce208811f540 tools/libxl/xl.c
> --- a/tools/libxl/xl.c  Thu Jan 13 01:26:44 2011 +0000
> +++ b/tools/libxl/xl.c  Thu Jan 13 11:26:35 2011 -0500
> @@ -74,6 +74,27 @@ static void parse_global_config(const ch
> 
>      xlu_cfg_destroy(config);
>  }
> +
> +static int check_dependencies(void)
> +{
> +    struct xs_handle *xsh;
> +
> +    xsh = xs_daemon_open();

This function is deprecated, please use xs_open. In addition this would
fail if you are running xenstored in a different domain to xl (e.g. a
stub-xenstored-dom).

Unfortunately the problem with opening a connection to xenstored in a
way which can cope with a remote xenstored is that it can block
indefinitely if the other end is not available.

One idea might be to simply check for the xenstored pidfile instead.
IIRC the prototypes of stub-xenstored also dropped a "domidfile" which
could also be checked in that case.

Otherwise I suspect the correct fix might involve a fix to
the /proc/xen/xenbus kernel driver so it doesn't block in this way or
has a timeout etc.

Perhaps this is more a theoretical concern, since stubdom xenstored
never really progressed past the prototype stage. It would be nice not
to add more things which would need fixing up for that case though.

> @@ -103,6 +124,9 @@ int main(int argc, char **argv)
>          exit(1);
>      }
>      opterr = 0;
> +
> +    if ( !check_dependencies() )
> +        exit(1);
> 
>      logger = xtl_createlogger_stdiostream(stderr, minmsglevel,  0);
>      if (!logger) exit(1);

The next thing after this xtl_createlogger_stdiostream(...) is a call to
libxl_ctx_init which will try and open the xenstore in the same way and
has error logging already so perhaps we should improve on that instead
of adding a separate check?

Ian.


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