[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] xen-blkback: Switch to closed state after releasing the backing device



On Fri, Sep 07, 2018 at 12:20:26PM +0200, Valentin Vidic wrote:
> On Fri, Sep 07, 2018 at 09:54:55AM +0200, Roger Pau Monné wrote:
> > Then I'm afraid you will have to look into the vbd_free/create fix.
> 
> Yes, here is a first draft for that idea, let me know if you see some
> problems there:

Thanks!

> --- xenbus.c.orig       2018-09-07 12:11:57.798071485 +0200
> +++ xenbus.c    2018-09-07 12:14:23.536077992 +0200
> @@ -758,6 +759,7 @@
>                              enum xenbus_state frontend_state)
>  {
>         struct backend_info *be = dev_get_drvdata(&dev->dev);
> +       struct block_device *bdev;
>         int err;
>  
>         pr_debug("%s %p %s\n", __func__, dev, 
> xenbus_strstate(frontend_state));
> @@ -772,6 +774,22 @@
>  
>         case XenbusStateInitialised:
>         case XenbusStateConnected:
> +               if (!be->blkif->vbd.bdev) {
> +                       printk("blkdev_get");
> +                       bdev = blkdev_get_by_dev(be->blkif->vbd.pdevice,
> +                                                be->blkif->vbd.readonly ?
> +                                                FMODE_READ : FMODE_WRITE, 
> NULL);
> +
> +                       if (IS_ERR(bdev)) {
> +                               pr_warn("frontend_changed: device %08x could 
> not be opened\n",
> +                                       be->blkif->vbd.pdevice);
> +                               break;
> +                       }
> +
> +                       printk("blkdev_get good");
> +                       be->blkif->vbd.bdev = bdev;
> +               }
> +

I would prefer if you could avoid open-coding this here, and instead
use xen_vbd_create or similar. I would also prefer that the call to
xen_vbd_create in backend_changed was removed and we had a single call
to xen_vbd_create that's used for both initial device connection and
reconnection.

Also, I think this could cause issues if for some reason the frontend
switches to state 'Connected' before hotplug scripts have run, in
which case you would try to open an unexpected device because pdevice
won't be correctly set.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.