Re: [PATCH v2 16/16] xen-blkback: Inform userspace that device has been opened

Roger Pau Monné
2023-06-06T09:15:50Z

On Tue, May 30, 2023 at 04:31:16PM -0400, Demi Marie Obenour wrote:
> Set "opened" to "0" before the hotplug script is called. Once the
> device node has been opened, set "opened" to "1".
>
> "opened" is used exclusively by userspace. It serves two purposes:
>
> 1. It tells userspace that the diskseq Xenstore entry is supported.
>
> 2. It tells userspace that it can wait for "opened" to be set to 1.
> Once "opened" is 1, blkback has a reference to the device, so
> userspace doesn't need to keep one.
>
> Together, these changes allow userspace to use block devices with
> delete-on-close behavior, such as loop devices with the autoclear flag
> set or device-mapper devices with the deferred-remove flag set.

There was some work in the past to allow reloading blkback as a
module, it's clear that using delete-on-close won't work if attempting
to reload blkback.

Isn't there some existing way to check whether a device is opened?
(stat syscall maybe?).

I would like to avoid adding more xenstore blkback state if such
information can be fetched from other methods.

> Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> ---
> drivers/block/xen-blkback/xenbus.c | 35 ++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
> index 9c3eb148fbd802c74e626c3d7bcd69dcb09bd921..519a78aa9073d1faa1dce5c1b36e95ae58da534b 100644
> --- a/drivers/block/xen-blkback/xenbus.c
> +++ b/drivers/block/xen-blkback/xenbus.c
> @@ -3,6 +3,20 @@
> Copyright (C) 2005 Rusty Russell <rusty@rustcorp.com.au>
> Copyright (C) 2005 XenSource Ltd
>
> +In addition to the Xenstore nodes required by the Xen block device
> +specification, this implementation of blkback uses a new Xenstore
> +node: "opened". blkback sets "opened" to "0" before the hotplug script
> +is called. Once the device node has been opened, blkback sets "opened"
> +to "1".
> +
> +"opened" is read exclusively by userspace. It serves two purposes:
> +
> +1. It tells userspace that diskseq@major:minor syntax for "physical-device" is
> + supported.
> +
> +2. It tells userspace that it can wait for "opened" to be set to 1 after writing
> + "physical-device". Once "opened" is 1, blkback has a reference to the
> + device, so userspace doesn't need to keep one.
>
> */
>
> @@ -699,6 +713,14 @@ static int xen_blkbk_probe(struct xenbus_device *dev,
> if (err)
> pr_warn("%s write out 'max-ring-page-order' failed\n", __func__);
>
> + /*
> + * This informs userspace that the "opened" node will be set to "1" when
> + * the device has been opened successfully.
> + */
> + err = xenbus_write(XBT_NIL, dev->nodename, "opened", "0");
> + if (err)
> + goto fail;
> +

You would need to set "opened" before registering the xenstore backend
watch AFAICT, or else it could be racy.

Thanks, Roger.