Re: [PATCH V3] libxenstore: filter watch events in libxenstore when we unwatch

[Julien Grall <julien.grall@citrix.com>]

On 12/13/2012 06:35 PM, Ian Jackson wrote:

> Julien Grall writes ("[PATCH V3] libxenstore: filter watch events in libxenstore when we unwatch"):
>> XenStore puts in queued watch events via a thread and notifies the user.
>> Sometimes xs_unwatch is called before all related message is read. The use
>> case is non-threaded libevent, we have two event A and B:
>>     - Event A will destroy something and call xs_unwatch;
>>     - Event B is used to notify that a node has changed in XenStore.
>> As the event is called one by one, event A can be handled before event B.
>> So on next xs_watch_read the user could retrieve an unwatch token and
>> a segfault occured if the token store the pointer of the structure
>> (ie: "backend:0xcafe").
>>
>> To avoid problem with previous application using libXenStore, this behaviour
>> will only be enabled if XS_UNWATCH_SAFE is give to xs_open.
> 
> Sorry I didn't reply to your previous email on this subject.
> 
> I think this is a reasonable approach but the option name needs to be
> more descriptive and the documentation a bit better.
> 
> XS_UNWATCH_FILTER ?  XS_WATCH_TOKENS_UNIQUE ?


I think XS_UNWATCH_FILTER is better.

> 
> As for the documentation:


I think it's too restrictive for upstream. xs_unwatch could only filter
following the token and the subpath. I modified your documentation
proposal below to take into account this solution. What do you think?

>  /*
>   * Setting XS_UNWATCH_FILTER arranges that after xs_unwatch, no
>   * related watch events will be delivered via xs_read_watch.  But

    * this relies on the couple token, subpath is unique.

>   *
>   *   XS_UNWATCH_FILTER clear           XS_UNWATCH_FILTER set
>   *
>   *    Even after xs_unwatch, "stale"    After xs_unwatch returns, no
>   *    instances of the watch event      watch events with the same
>   *    may be delivered.                 token and with the same subpath

    *                                      will be delivered.
    *

    *    A path and a subpath can be       The application must avoid to

    *    register with the same token.     register a path (/foo/) and
    *                                      a subpath (/foo/bar) with the
    *                                      same path until a successful
    *                                      xs_unwatch for the first
    *                                      watch has returned.

>   *                                      
>   */
> 
> With this specification it is not necessary to check the path when
> filtering out the stale events.  Which is just as well because:
>

>> +		if (l_token && !strcmp(token, l_token)
>> +			/* Use strncmp because we can have a watch fired on sub-directory */
>> +			&& l_path && !strncmp(path, l_path, strlen(path))) {
> 
> This isn't correct: the subpath relation is not the same as the prefix
> relation.


I will fix the test by using xs_path_is_subpath.

Thanks,

Julien