Re: [Xen-devel] [PATCH] libxenstore: filter watch events in libxenstore when we unwatch

[Julien Grall <julien.grall@xxxxxxxxx>]

On Fri, Sep 21, 2012 at 5:28 PM, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx> wrote:
> Julien Grall writes ("Re: [PATCH] libxenstore: filter watch events in 
> libxenstore when we unwatch"):
>> On 09/21/2012 04:45 PM, Ian Jackson wrote:
>> > The num_strings thing is obsolete.  There will always be two strings.
>> > Also xs_count_strings walks the array.
>>
>> >> +  /* Clear the pipe token if there are no more pending watches. */
>> >> +  if (list_empty(&h->watch_list) && (h->watch_pipe[0] != -1)) {
>> >> +          while (read(h->watch_pipe[0], &c, 1) != 1)
>> >> +                  continue;
>> >> +  }
>> >
>> > I'm not convinced this is necessary.  Don't callers already need to
>> > cope with potential spurious signallings of the watch pipe ?
>>
>> I base my code on xs_read_watch. As I understand xs_read_watch, it will
>> wait on a condition until the list is not empty.
>> So if the list is empty and not the pipe, an event can occur and block
>> the application (with xs_read_watch).
>
> xs_read_watch can only be correctly be used if you are determined to
> wait, right there, until a particular watch turns up.
>
> If you are using xs_fileno, you must do as the comment says:
>  * Callers should, after xs_fileno has become readable, repeatedly
>  * call xs_check_watch until it returns NULL and sets errno to EAGAIN.
>  * (If the fd became readable, xs_check_watch is allowed to make it no
>  * longer show up as readable even if future calls to xs_check_watch
>  * will return more watch events.)

The function xs_check_watch was recently introduced in Xen (see your
commit on december 2011).
Most of application, for instance QEMU, still use xs_read_watch.
So if xs_unwatch doesn't empty the pipe, most of this applications
potentially stall indefinitly.

IMHO, this piece of code is really important.

Sincerely yours,

-- 
Grall Julien

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