Re: [Xen-devel] [PATCH] drivers: xen-blkfront: only talk_to_blkback() when in XenbusStateInitialising

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Mon, Jun 22, 2015 at 09:56:00AM -0400, Konrad Rzeszutek Wilk wrote:
> From: Bob Liu <bob.liu@xxxxxxxxxx>
> 
> Patch 69b91ede5cab843dcf345c28bd1f4b5a99dacd9b
> "drivers: xen-blkback: delay pending_req allocation to connect_ring"
> exposed an problem that Xen blkfront has. There is a race
> with XenStored and the drivers such that we can see two:
> 
> vbd vbd-268440320: blkfront:blkback_changed to state 2.
> vbd vbd-268440320: blkfront:blkback_changed to state 2.
> vbd vbd-268440320: blkfront:blkback_changed to state 4.
> 
> state changes to XenbusStateInitWait ('2'). The end result is that
> blkback_changed() receives two notify and calls twice setup_blkring().
> 
> While the backend driver may only get the first setup_blkring() which is
> wrong and reads out-dated (or reads them as they are being updated
> with new ring-ref values).
> 
> The end result is that the ring ends up being incorrectly set.

I forgot to mention - this check exists in xen-netfront thought the 
commit for it does not mention the double state changes. Both
Xen PCI frontend and backed are OK. So added this in the commit

"The other drivers in the tree have such checks already in."

> 
> Reported-and-Tested-by: Robert Butera <robert.butera@xxxxxxxxxx>
> Signed-off-by: Bob Liu <bob.liu@xxxxxxxxxx>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> ---
>  drivers/block/xen-blkfront.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index d3c1a95..fc770b7 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -1951,6 +1951,8 @@ static void blkback_changed(struct xenbus_device *dev,
>  
>       switch (backend_state) {
>       case XenbusStateInitWait:
> +             if (dev->state != XenbusStateInitialising)
> +                     break;
>               if (talk_to_blkback(dev, info)) {
>                       kfree(info);
>                       dev_set_drvdata(&dev->dev, NULL);
> -- 
> 2.1.0
> 

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