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

[Roger Pau Monné]

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@xxxxxxxxxxxxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxx>
>      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.