Re: [Xen-devel] [PATCH] libxl: Use xenbus to communicate with xenstore if the socket fails

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Fri, 2010-12-10 at 06:52 +0000, Mihir Nanavati wrote:
> I've made the changes in libxenstore as you recommended - xs_daemon|
> domain_open{_readonly} all alias to xs_open. 
> 
> 
> Also, in the case of opening read-only, if it fails using sockets, it
> opens /proc/xen/xenbus with O_RDONLY.

Thanks! I think xs_open should be simplified to:
        if (ro)
                xsh = get_handle(xs_daemon_socket_ro(), ro);
        else
                xsh = get_handle(xs_daemon_socket(), ro);
        
        if (!xsh)
                xsh = get_handle(xs_domain_dev(), ro);

For future flexibility should we consider passing a flags argument and
defining "XS_OPEN_READONLY 1<<0" instead of having an ro argument?

It's probably worth adding xs_close (with xs_daemon_close as an alias)
for API symmetry as well as adding a comment to the header marking the
aliases as deprecated.

I don't suppose you feel like running sed over the tree to convert the
in tree users, do you ;-)

Ian.

> 
> 
> ~M
> 
> On Thu, Dec 9, 2010 at 3:39 AM, Ian Campbell
> <Ian.Campbell@xxxxxxxxxxxxx> wrote:
>         
>         On Thu, 2010-12-09 at 10:53 +0000, Vincent Hanquez wrote:
>         > On 09/12/10 10:21, Ian Campbell wrote:
>         > >>   I think there was at least 1 other practical
>         differences, but it
>         > >> seems to have eluded me.
>         >
>         > Thinking more about it, it might be related to timeout,
>         and/or it might
>         > be: If xenstored has decided to not answer you, your thread
>         is dead in
>         > the water, because you end up stuck in kernel land forever
>         (wait not
>         > interruptible aka D).
>         >
>         > (It might have been related to historical reasons, however
>         it could
>         > still be happening in the unlikely event of xenstored dying)
>         
>         
>         Sounds plausible.
>         
>         The current driver seems to use wait_event_interruptible and
>         attempt to
>         do something sane looking with O_NONBLOCK on read but on write
>         looks
>         like it may end up waiting for a reply forever if xenstored
>         has gone
>         away, there's even a "FIXME avoid synchronous wait for
>         response" in the
>         code.
>         
>         Ian.
>         
> 
> 



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