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

[Mihir Nanavati <mihirn@xxxxxxxxx>]

On Fri, Dec 10, 2010 at 1:07 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx> wrote:

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);

Thanks, that looks much cleaner.
 

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

Sure, we could do it, but I'm not too sure what other modes we could have for opening, let alone ones that might be used simultaneously in a bit field ;)
 

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.

Sure, will do.
 

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

Could do, but I'd rather we get the interface finalized first ;)  Is there anything one specially needs to take into consideration when replacing them in the bindings?

~M
 

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