Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1324194 > unrolled thread

[PATCH v2] nbd: Issue a uevent when disconnecting

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2] nbd: Issue a uevent when disconnecting Wouter Verhelst <w@uter.be> - 2016-02-02 16:30 +0100
    Re: [PATCH v2] nbd: Issue a uevent when disconnecting Markus Pargmann <mpa@pengutronix.de> - 2016-02-05 10:10 +0100
      Re: [PATCH v2] nbd: Issue a uevent when disconnecting Wouter Verhelst <w@uter.be> - 2016-02-05 10:50 +0100
    [PATCH] nbd: Create size change events for userspace Markus Pargmann <mpa@pengutronix.de> - 2016-02-05 10:10 +0100

#1324194 — [PATCH v2] nbd: Issue a uevent when disconnecting

There already is a uevent by default when closing a device upon connect
of the device. However, the same isn't true on disconnect.

This makes it hard for userspace to keep track of whether a device is
connected, since we are notified when the connection is created, but not
when it is removed again.

Explicitly issue a "change" uevent to remedy.

[v2: incorporate feedback from udev maintainer and make sure that we
issue a uevent upon connect as well as disconnect, rather than connect
only]

Signed-off-by: Wouter Verhelst <w@uter.be>
---
 drivers/block/nbd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index e4c5cc1..04d9563 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -417,6 +417,8 @@ static int nbd_thread_recv(struct nbd_device *nbd)
 	spin_unlock_irqrestore(&nbd->tasks_lock, flags);
 
 	ret = device_create_file(disk_to_dev(nbd->disk), &pid_attr);
+	kobject_uevent(&disk_to_dev(nbd->disk)->kobj, KOBJ_CHANGE);
+
 	if (ret) {
 		dev_err(disk_to_dev(nbd->disk), "device_create_file failed!\n");
 
@@ -438,6 +440,7 @@ static int nbd_thread_recv(struct nbd_device *nbd)
 	}
 
 	device_remove_file(disk_to_dev(nbd->disk), &pid_attr);
+	kobject_uevent(&disk_to_dev(nbd->disk)->kobj, KOBJ_CHANGE);
 
 	spin_lock_irqsave(&nbd->tasks_lock, flags);
 	nbd->task_recv = NULL;
-- 
2.7.0

[toc] | [next] | [standalone]

#1327544

[Multipart message — attachments visible in raw view] — view raw

Hi Wouter,

On Tuesday, February 02, 2016 04:27:00 PM Wouter Verhelst wrote:
> There already is a uevent by default when closing a device upon connect
> of the device. However, the same isn't true on disconnect.
> 
> This makes it hard for userspace to keep track of whether a device is
> connected, since we are notified when the connection is created, but not
> when it is removed again.
> 
> Explicitly issue a "change" uevent to remedy.
> 
> [v2: incorporate feedback from udev maintainer and make sure that we
> issue a uevent upon connect as well as disconnect, rather than connect
> only]

The systemd people had the same feedback. Basically the device is not
marked as ready until a uevent is received. My idea was to use the size
property consistently. If the device is connected the size is correctly
changed and a uevent is created. On disconnect the size is set to 0 with
another uevent.

Does this work for you as well? I will send the patch as reply.

Thanks,

Markus

> 
> Signed-off-by: Wouter Verhelst <w@uter.be>
> ---
>  drivers/block/nbd.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index e4c5cc1..04d9563 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -417,6 +417,8 @@ static int nbd_thread_recv(struct nbd_device *nbd)
>  	spin_unlock_irqrestore(&nbd->tasks_lock, flags);
>  
>  	ret = device_create_file(disk_to_dev(nbd->disk), &pid_attr);
> +	kobject_uevent(&disk_to_dev(nbd->disk)->kobj, KOBJ_CHANGE);
> +
>  	if (ret) {
>  		dev_err(disk_to_dev(nbd->disk), "device_create_file failed!\n");
>  
> @@ -438,6 +440,7 @@ static int nbd_thread_recv(struct nbd_device *nbd)
>  	}
>  
>  	device_remove_file(disk_to_dev(nbd->disk), &pid_attr);
> +	kobject_uevent(&disk_to_dev(nbd->disk)->kobj, KOBJ_CHANGE);
>  
>  	spin_lock_irqsave(&nbd->tasks_lock, flags);
>  	nbd->task_recv = NULL;
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[toc] | [prev] | [next] | [standalone]

#1327573

Hi Markus,

On Fri, Feb 05, 2016 at 10:03:41AM +0100, Markus Pargmann wrote:
> Hi Wouter,
> 
> On Tuesday, February 02, 2016 04:27:00 PM Wouter Verhelst wrote:
> > There already is a uevent by default when closing a device upon connect
> > of the device. However, the same isn't true on disconnect.
> > 
> > This makes it hard for userspace to keep track of whether a device is
> > connected, since we are notified when the connection is created, but not
> > when it is removed again.
> > 
> > Explicitly issue a "change" uevent to remedy.
> > 
> > [v2: incorporate feedback from udev maintainer and make sure that we
> > issue a uevent upon connect as well as disconnect, rather than connect
> > only]
> 
> The systemd people had the same feedback.

Well, the systemd people and the udev people are the same people these
days :-)

> Basically the device is not marked as ready until a uevent is received. My
> idea was to use the size property consistently. If the device is connected
> the size is correctly changed and a uevent is created. On disconnect the size
> is set to 0 with another uevent.
> 
> Does this work for you as well? I will send the patch as reply.

Sure. What matters most is that the uevent is done; how it's done, less
so.

I should add that this was meant to go with systemd #2422. I wasn't
aware of #358, or I would've talked to you more :-)

-- 
It is easy to love a country that is famous for chocolate and beer

  -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26

[toc] | [prev] | [next] | [standalone]

#1327546 — [PATCH] nbd: Create size change events for userspace

The userspace needs to know when nbd devices are ready for use.
Currently no events are created for the userspace which doesn't work for
systemd.

See the discussion here: https://github.com/systemd/systemd/pull/358

This patch uses a central point to setup the nbd-internal sizes. A ioctl
to set a size does not lead to a visible size change. The size of the
block device will be kept at 0 until nbd is connected. As soon as it
connects, the size will be changed to the real value and a uevent is
created. When disconnecting, the blockdevice is set to 0 size and
another uevent is generated.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---

 drivers/block/nbd.c | 80 +++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 59 insertions(+), 21 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 4c5d94146aa3..f3682882822a 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -100,6 +100,11 @@ static inline struct device *nbd_to_dev(struct nbd_device *nbd)
 	return disk_to_dev(nbd->disk);
 }
 
+static bool nbd_is_connected(struct nbd_device *nbd)
+{
+	return !!nbd->task_recv;
+}
+
 static const char *nbdcmd_to_ascii(int cmd)
 {
 	switch (cmd) {
@@ -112,6 +117,43 @@ static const char *nbdcmd_to_ascii(int cmd)
 	return "invalid";
 }
 
+static int nbd_size_clear(struct nbd_device *nbd, struct block_device *bdev)
+{
+	bdev->bd_inode->i_size = 0;
+	set_blocksize(bdev, 0);
+	set_capacity(nbd->disk, 0);
+	kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE);
+
+	return 0;
+}
+
+static int nbd_size_update(struct nbd_device *nbd, struct block_device *bdev)
+{
+	int ret;
+
+	if (!nbd_is_connected(nbd))
+		return 0;
+
+	ret = set_blocksize(bdev, nbd->blksize);
+	if (ret)
+		return ret;
+
+	bdev->bd_inode->i_size = nbd->bytesize;
+	set_capacity(nbd->disk, nbd->bytesize >> 9);
+	kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE);
+
+	return 0;
+}
+
+static int nbd_size_set(struct nbd_device *nbd, struct block_device *bdev,
+			int blocksize, int nr_blocks)
+{
+	nbd->blksize = blocksize;
+	nbd->bytesize = blocksize * nr_blocks;
+
+	return nbd_size_update(nbd, bdev);
+}
+
 static void nbd_end_request(struct nbd_device *nbd, struct request *req)
 {
 	int error = req->errors ? -EIO : 0;
@@ -401,7 +443,7 @@ static struct device_attribute pid_attr = {
 	.show = pid_show,
 };
 
-static int nbd_thread_recv(struct nbd_device *nbd)
+static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev)
 {
 	struct request *req;
 	int ret;
@@ -421,6 +463,8 @@ static int nbd_thread_recv(struct nbd_device *nbd)
 		return ret;
 	}
 
+	nbd_size_update(nbd, bdev);
+
 	while (1) {
 		req = nbd_read_stat(nbd);
 		if (IS_ERR(req)) {
@@ -431,6 +475,8 @@ static int nbd_thread_recv(struct nbd_device *nbd)
 		nbd_end_request(nbd, req);
 	}
 
+	nbd_size_clear(nbd, bdev);
+
 	device_remove_file(disk_to_dev(nbd->disk), &pid_attr);
 
 	nbd->task_recv = NULL;
@@ -707,20 +753,19 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		return err;
 	}
 
-	case NBD_SET_BLKSIZE:
-		nbd->blksize = arg;
-		nbd->bytesize &= ~(nbd->blksize-1);
-		bdev->bd_inode->i_size = nbd->bytesize;
-		set_blocksize(bdev, nbd->blksize);
-		set_capacity(nbd->disk, nbd->bytesize >> 9);
-		return 0;
+	case NBD_SET_BLKSIZE: {
+		loff_t bsize = nbd->bytesize;
+		do_div(bsize, arg);
+
+		return nbd_size_set(nbd, bdev, arg, bsize);
+	}
 
 	case NBD_SET_SIZE:
-		nbd->bytesize = arg & ~(nbd->blksize-1);
-		bdev->bd_inode->i_size = nbd->bytesize;
-		set_blocksize(bdev, nbd->blksize);
-		set_capacity(nbd->disk, nbd->bytesize >> 9);
-		return 0;
+		return nbd_size_set(nbd, bdev, nbd->blksize,
+				    arg / nbd->blksize);
+
+	case NBD_SET_SIZE_BLOCKS:
+		return nbd_size_set(nbd, bdev, nbd->blksize, arg);
 
 	case NBD_SET_TIMEOUT:
 		nbd->xmit_timeout = arg * HZ;
@@ -736,13 +781,6 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		nbd->flags = arg;
 		return 0;
 
-	case NBD_SET_SIZE_BLOCKS:
-		nbd->bytesize = ((u64) arg) * nbd->blksize;
-		bdev->bd_inode->i_size = nbd->bytesize;
-		set_blocksize(bdev, nbd->blksize);
-		set_capacity(nbd->disk, nbd->bytesize >> 9);
-		return 0;
-
 	case NBD_DO_IT: {
 		struct task_struct *thread;
 		int error;
@@ -764,7 +802,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		}
 
 		nbd_dev_dbg_init(nbd);
-		error = nbd_thread_recv(nbd);
+		error = nbd_thread_recv(nbd, bdev);
 		nbd_dev_dbg_close(nbd);
 		kthread_stop(thread);
 
-- 
2.7.0.rc3

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web