Re: [Xen-devel] [RFC] Shutdown event script for xl toolstack

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Thanks for your submission; this is coming along but still needs some
work.

Joseph Glanville writes ("Re: [Xen-devel] [RFC] Shutdown event script for xl 
toolstack"):
> On 31 March 2012 05:08, Joseph Glanville
> <joseph.glanville@xxxxxxxxxxxxxx> wrote:
> +        execvpe(arg0,args,envp);

I'm afraid that execvpe doesn't do what you want: it entirely replaces
the environment with the one you specify.  You need to use putenv or
setenv.

> +    if (xlu_cfg_replace_string (config, "shutdown_event_handler",
> +                            &d_config->shutdown_event_handler, 0))
> +        d_config->shutdown_event_handler = NULL;

Weren't we going to have one event handler per type of event ?

> +{
> +    int status;
> +    useconds_t sleep_time = 10, wait_time = 0;
> +    pid_t child_pid, wpid;

I'm afraid this timeout approach is not correct; you would need
something to interrupt the wait, rather than polling like this.  I
think this is impractical.  You should, rather, just not implement a
timeout.

I would advise against reusing exit statuses 0..6.  You should start
your new exit statuses at 50 or something.  This is because many
programs use status 1 to mean "I failed" and you would want to avoid
that being misinterpreted.

Other exit statuses, and deaths due to signals, should be reported
properly as errors using libxl_report_child_exitstatus.

> +    ret = xl_exec(arg0,argv,envp,timeout);

I think calling this function xl_exec is a bit unfortunate.  It
doesn't just exec; it also forks.

> +     if (d_config->shutdown_event_handler)
> +        action = user_shutdown_action(d_config, domid, action,
> +
> event->u.domain_shutdown.shutdown_reason);

Something has linewrapped this.  You should keep to within 75-80
lines.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel