LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH] Issue a uevent when disconnecting
@ 2016-01-23 16:23 Wouter Verhelst
  2016-02-02 15:27 ` [PATCH v2] nbd: " Wouter Verhelst
  0 siblings, 1 reply; 6+ messages in thread
From: Wouter Verhelst @ 2016-01-23 16:23 UTC (permalink / raw)
  To: Markus Pargmann, open list:NETWORK BLOCK DEVICE (NBD), open list
  Cc: Wouter Verhelst

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.

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

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index e4c5cc1..43dcc12 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -438,6 +438,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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2] nbd: Issue a uevent when disconnecting
  2016-01-23 16:23 [PATCH] Issue a uevent when disconnecting Wouter Verhelst
@ 2016-02-02 15:27 ` Wouter Verhelst
  2016-02-05  9:03   ` Markus Pargmann
  2016-02-05  9:04   ` [PATCH] nbd: Create size change events for userspace Markus Pargmann
  0 siblings, 2 replies; 6+ messages in thread
From: Wouter Verhelst @ 2016-02-02 15:27 UTC (permalink / raw)
  To: Markus Pargmann, nbd-general, lkml

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] nbd: Issue a uevent when disconnecting
  2016-02-02 15:27 ` [PATCH v2] nbd: " Wouter Verhelst
@ 2016-02-05  9:03   ` Markus Pargmann
  2016-02-05  9:42     ` Wouter Verhelst
  2016-02-05  9:04   ` [PATCH] nbd: Create size change events for userspace Markus Pargmann
  1 sibling, 1 reply; 6+ messages in thread
From: Markus Pargmann @ 2016-02-05  9:03 UTC (permalink / raw)
  To: Wouter Verhelst; +Cc: nbd-general, lkml

[-- Attachment #1: Type: text/plain, Size: 2258 bytes --]

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 |

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] nbd: Create size change events for userspace
  2016-02-02 15:27 ` [PATCH v2] nbd: " Wouter Verhelst
  2016-02-05  9:03   ` Markus Pargmann
@ 2016-02-05  9:04   ` Markus Pargmann
  1 sibling, 0 replies; 6+ messages in thread
From: Markus Pargmann @ 2016-02-05  9:04 UTC (permalink / raw)
  To: Wouter Verhelst; +Cc: nbd-general, lkml, Markus Pargmann

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] nbd: Issue a uevent when disconnecting
  2016-02-05  9:03   ` Markus Pargmann
@ 2016-02-05  9:42     ` Wouter Verhelst
  2016-02-12 14:49       ` Markus Pargmann
  0 siblings, 1 reply; 6+ messages in thread
From: Wouter Verhelst @ 2016-02-05  9:42 UTC (permalink / raw)
  To: Markus Pargmann; +Cc: nbd-general, lkml

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] nbd: Issue a uevent when disconnecting
  2016-02-05  9:42     ` Wouter Verhelst
@ 2016-02-12 14:49       ` Markus Pargmann
  0 siblings, 0 replies; 6+ messages in thread
From: Markus Pargmann @ 2016-02-12 14:49 UTC (permalink / raw)
  To: Wouter Verhelst; +Cc: nbd-general, lkml

[-- Attachment #1: Type: text/plain, Size: 2076 bytes --]

Hi Wouter,

On Friday, February 05, 2016 10:42:41 AM Wouter Verhelst wrote:
> 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 :-)

Ah I see, thanks.

I wrote some tests yesterday to ensure that I don't break other things
with the new size calculations. I will send the pull request with this
patch for 4.6.

Best Regards,

Markus

-- 
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 |

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-02-12 14:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-23 16:23 [PATCH] Issue a uevent when disconnecting Wouter Verhelst
2016-02-02 15:27 ` [PATCH v2] nbd: " Wouter Verhelst
2016-02-05  9:03   ` Markus Pargmann
2016-02-05  9:42     ` Wouter Verhelst
2016-02-12 14:49       ` Markus Pargmann
2016-02-05  9:04   ` [PATCH] nbd: Create size change events for userspace Markus Pargmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).