Re: [PATCH v4 2/4] tools: split out xenstored starting form xencommons

[Wei Liu <wei.liu2@citrix.com>]

On Tue, Aug 02, 2016 at 01:20:40PM +0200, Juergen Gross wrote:
> On 02/08/16 13:13, Wei Liu wrote:
> > On Tue, Aug 02, 2016 at 12:39:26PM +0200, Juergen Gross wrote:
> >> In order to prepare starting a xenstore domain split out the starting
> >> of the xenstore daemon from the xencommons script into a dedicated
> >> launch-xenstore script.
> >>
> >> Correct one error: don't remove old tdb files in background, as this
> >> could lead to very subtle races.
> >>
> > 
> > Sorry to only notice this now -- I suppose this shouldn't be in commit
> > message because it is a changelog for different revision of your patch,
> > or you need to describe what the race is if there is really such issue
> > -- and that's a potential backport candidate.
> 
> -		rm -f "$XENSTORED_ROOTDIR"/tdb* &>/dev/null
> ...
> +	rm -f "$XENSTORED_ROOTDIR"/tdb* 2>/dev/null
> 
> Please note the "&" in the removed line. And the description just tells
> you that: the file shouldn't be removed in the background. How else
> would you describe it?
> 
> And the race should be obvious: removing a file in the background which
> is going to be used by the next command is racy.
> 
> > As far as I can tell this is pure code motion and shouldn't have any
> > change in behaviour. Hence this line in commit message makes me feel a
> > bit confused.
> 
> Still confused?

Ah, the 2> vs &> thing is really subtle. I missed that, sorry.

I think you need to separate that one change out so that we can backport
it to stable releases to avoid the race.

Wei.

> 
> 
> Juergen

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