From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH V3] libxenstore: filter watch events in libxenstore when we unwatch Date: Fri, 14 Dec 2012 14:54:13 +0000 Message-ID: <50CB3D95.9000000@citrix.com> References: <53cbcf7907b54963906d7b5fa5ad40b4fff37886.1355399450.git.julien.grall@citrix.com> <20682.8150.410558.581149@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20682.8150.410558.581149@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Jackson Cc: Ian Campbell , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org 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