* [PATCH 0/2] nbd: block/dev size changes @ 2019-05-29 20:16 Mike Christie 2019-05-29 20:16 ` [PATCH 1/2] nbd: fix crash when the blksize is zero v2 Mike Christie 2019-05-29 20:16 ` [PATCH 2/2] nbd: add netlink reconfigure resize support v3 Mike Christie 0 siblings, 2 replies; 8+ messages in thread From: Mike Christie @ 2019-05-29 20:16 UTC (permalink / raw) To: josef, linux-block This is just Xiubo's zero blksize patch from here: https://marc.info/?l=linux-kernel&m=155893597828674&w=2 and my resize patch. Xiubo's zero blksize patch and my resize patch conflicted, so I fixed the issue mentioned in the link so his patch does "goto out" instead of returning while holding resources, and rebuilt my patch over his. I also fixed a issue in my resize patch so we only call nbd_size_set if a value has changed. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] nbd: fix crash when the blksize is zero v2 2019-05-29 20:16 [PATCH 0/2] nbd: block/dev size changes Mike Christie @ 2019-05-29 20:16 ` Mike Christie 2019-05-30 3:19 ` Xiubo Li 2019-06-13 16:56 ` Josef Bacik 2019-05-29 20:16 ` [PATCH 2/2] nbd: add netlink reconfigure resize support v3 Mike Christie 1 sibling, 2 replies; 8+ messages in thread From: Mike Christie @ 2019-05-29 20:16 UTC (permalink / raw) To: josef, linux-block; +Cc: Xiubo Li, Mike Christie From: Xiubo Li <xiubli@redhat.com> This will allow the blksize to be set zero and then use 1024 as default. Signed-off-by: Xiubo Li <xiubli@redhat.com> [fix to use goto out instead of return in genl_connect] Signed-off-by: Mike Christie <mchristi@redhat.com> --- V2: - Fix error handling in genl_connect. drivers/block/nbd.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 053958a8a2ba..236253fbf455 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -135,6 +135,8 @@ static struct dentry *nbd_dbg_dir; #define NBD_MAGIC 0x68797548 +#define NBD_DEF_BLKSIZE 1024 + static unsigned int nbds_max = 16; static int max_part = 16; static struct workqueue_struct *recv_workqueue; @@ -1237,6 +1239,14 @@ static void nbd_clear_sock_ioctl(struct nbd_device *nbd, nbd_config_put(nbd); } +static bool nbd_is_valid_blksize(unsigned long blksize) +{ + if (!blksize || !is_power_of_2(blksize) || blksize < 512 || + blksize > PAGE_SIZE) + return false; + return true; +} + /* Must be called with config_lock held */ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, unsigned int cmd, unsigned long arg) @@ -1252,8 +1262,9 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, case NBD_SET_SOCK: return nbd_add_socket(nbd, arg, false); case NBD_SET_BLKSIZE: - if (!arg || !is_power_of_2(arg) || arg < 512 || - arg > PAGE_SIZE) + if (!arg) + arg = NBD_DEF_BLKSIZE; + if (!nbd_is_valid_blksize(arg)) return -EINVAL; nbd_size_set(nbd, arg, div_s64(config->bytesize, arg)); @@ -1333,7 +1344,7 @@ static struct nbd_config *nbd_alloc_config(void) atomic_set(&config->recv_threads, 0); init_waitqueue_head(&config->recv_wq); init_waitqueue_head(&config->conn_wait); - config->blksize = 1024; + config->blksize = NBD_DEF_BLKSIZE; atomic_set(&config->live_connections, 0); try_module_get(THIS_MODULE); return config; @@ -1769,6 +1780,12 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info) if (info->attrs[NBD_ATTR_BLOCK_SIZE_BYTES]) { u64 bsize = nla_get_u64(info->attrs[NBD_ATTR_BLOCK_SIZE_BYTES]); + if (!bsize) + bsize = NBD_DEF_BLKSIZE; + if (!nbd_is_valid_blksize(bsize)) { + ret = -EINVAL; + goto out; + } nbd_size_set(nbd, bsize, div64_u64(config->bytesize, bsize)); } if (info->attrs[NBD_ATTR_TIMEOUT]) { -- 2.21.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] nbd: fix crash when the blksize is zero v2 2019-05-29 20:16 ` [PATCH 1/2] nbd: fix crash when the blksize is zero v2 Mike Christie @ 2019-05-30 3:19 ` Xiubo Li 2019-06-13 16:56 ` Josef Bacik 1 sibling, 0 replies; 8+ messages in thread From: Xiubo Li @ 2019-05-30 3:19 UTC (permalink / raw) To: Mike Christie, josef, linux-block On 2019/5/30 4:16, Mike Christie wrote: > From: Xiubo Li <xiubli@redhat.com> > > This will allow the blksize to be set zero and then use 1024 as > default. > > Signed-off-by: Xiubo Li <xiubli@redhat.com> > [fix to use goto out instead of return in genl_connect] > Signed-off-by: Mike Christie <mchristi@redhat.com> > --- > > V2: > - Fix error handling in genl_connect. This change looks good to me. Thanks. BRs Xiubo > drivers/block/nbd.c | 23 ++++++++++++++++++++--- > 1 file changed, 20 insertions(+), 3 deletions(-) > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > index 053958a8a2ba..236253fbf455 100644 > --- a/drivers/block/nbd.c > +++ b/drivers/block/nbd.c > @@ -135,6 +135,8 @@ static struct dentry *nbd_dbg_dir; > > #define NBD_MAGIC 0x68797548 > > +#define NBD_DEF_BLKSIZE 1024 > + > static unsigned int nbds_max = 16; > static int max_part = 16; > static struct workqueue_struct *recv_workqueue; > @@ -1237,6 +1239,14 @@ static void nbd_clear_sock_ioctl(struct nbd_device *nbd, > nbd_config_put(nbd); > } > > +static bool nbd_is_valid_blksize(unsigned long blksize) > +{ > + if (!blksize || !is_power_of_2(blksize) || blksize < 512 || > + blksize > PAGE_SIZE) > + return false; > + return true; > +} > + > /* Must be called with config_lock held */ > static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, > unsigned int cmd, unsigned long arg) > @@ -1252,8 +1262,9 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, > case NBD_SET_SOCK: > return nbd_add_socket(nbd, arg, false); > case NBD_SET_BLKSIZE: > - if (!arg || !is_power_of_2(arg) || arg < 512 || > - arg > PAGE_SIZE) > + if (!arg) > + arg = NBD_DEF_BLKSIZE; > + if (!nbd_is_valid_blksize(arg)) > return -EINVAL; > nbd_size_set(nbd, arg, > div_s64(config->bytesize, arg)); > @@ -1333,7 +1344,7 @@ static struct nbd_config *nbd_alloc_config(void) > atomic_set(&config->recv_threads, 0); > init_waitqueue_head(&config->recv_wq); > init_waitqueue_head(&config->conn_wait); > - config->blksize = 1024; > + config->blksize = NBD_DEF_BLKSIZE; > atomic_set(&config->live_connections, 0); > try_module_get(THIS_MODULE); > return config; > @@ -1769,6 +1780,12 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info) > if (info->attrs[NBD_ATTR_BLOCK_SIZE_BYTES]) { > u64 bsize = > nla_get_u64(info->attrs[NBD_ATTR_BLOCK_SIZE_BYTES]); > + if (!bsize) > + bsize = NBD_DEF_BLKSIZE; > + if (!nbd_is_valid_blksize(bsize)) { > + ret = -EINVAL; > + goto out; > + } > nbd_size_set(nbd, bsize, div64_u64(config->bytesize, bsize)); > } > if (info->attrs[NBD_ATTR_TIMEOUT]) { ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] nbd: fix crash when the blksize is zero v2 2019-05-29 20:16 ` [PATCH 1/2] nbd: fix crash when the blksize is zero v2 Mike Christie 2019-05-30 3:19 ` Xiubo Li @ 2019-06-13 16:56 ` Josef Bacik 1 sibling, 0 replies; 8+ messages in thread From: Josef Bacik @ 2019-06-13 16:56 UTC (permalink / raw) To: Mike Christie; +Cc: josef, linux-block, Xiubo Li On Wed, May 29, 2019 at 03:16:05PM -0500, Mike Christie wrote: > From: Xiubo Li <xiubli@redhat.com> > > This will allow the blksize to be set zero and then use 1024 as > default. > > Signed-off-by: Xiubo Li <xiubli@redhat.com> > [fix to use goto out instead of return in genl_connect] > Signed-off-by: Mike Christie <mchristi@redhat.com> > --- Sorry I missed this second go around Reviewed-by: Josef Bacik <josef@toxicpanda.com> Thanks, Josef ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] nbd: add netlink reconfigure resize support v3 2019-05-29 20:16 [PATCH 0/2] nbd: block/dev size changes Mike Christie 2019-05-29 20:16 ` [PATCH 1/2] nbd: fix crash when the blksize is zero v2 Mike Christie @ 2019-05-29 20:16 ` Mike Christie 2019-06-13 17:01 ` Josef Bacik 1 sibling, 1 reply; 8+ messages in thread From: Mike Christie @ 2019-05-29 20:16 UTC (permalink / raw) To: josef, linux-block; +Cc: Mike Christie If the device is setup with ioctl we can resize the device after the initial setup, but if the device is setup with netlink we cannot use the resize related ioctls and there is no netlink reconfigure size ATTR handling code. This patch adds netlink reconfigure resize support to match the ioctl interface. Signed-off-by: Mike Christie <mchristi@redhat.com> --- V3; - If the device size or block size has not changed do not call nbd_size_set. V2: - Merge reconfig and connect resize related code to helper and avoid multiple nbd_size_set calls. drivers/block/nbd.c | 48 ++++++++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 236253fbf455..9486555e6391 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1685,6 +1685,30 @@ nbd_device_policy[NBD_DEVICE_ATTR_MAX + 1] = { [NBD_DEVICE_CONNECTED] = { .type = NLA_U8 }, }; +static int nbd_genl_size_set(struct genl_info *info, struct nbd_device *nbd) +{ + struct nbd_config *config = nbd->config; + u64 bsize = config->blksize; + u64 bytes = config->bytesize; + + if (info->attrs[NBD_ATTR_SIZE_BYTES]) + bytes = nla_get_u64(info->attrs[NBD_ATTR_SIZE_BYTES]); + + if (info->attrs[NBD_ATTR_BLOCK_SIZE_BYTES]) { + bsize = nla_get_u64(info->attrs[NBD_ATTR_BLOCK_SIZE_BYTES]); + if (!bsize) + bsize = NBD_DEF_BLKSIZE; + if (!nbd_is_valid_blksize(bsize)) { + printk(KERN_ERR "Invalid block size %llu\n", bsize); + return -EINVAL; + } + } + + if (bytes != config->bytesize || bsize != config->blksize) + nbd_size_set(nbd, bsize, div64_u64(bytes, bsize)); + return 0; +} + static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info) { struct nbd_device *nbd = NULL; @@ -1772,22 +1796,10 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info) refcount_set(&nbd->config_refs, 1); set_bit(NBD_BOUND, &config->runtime_flags); - if (info->attrs[NBD_ATTR_SIZE_BYTES]) { - u64 bytes = nla_get_u64(info->attrs[NBD_ATTR_SIZE_BYTES]); - nbd_size_set(nbd, config->blksize, - div64_u64(bytes, config->blksize)); - } - if (info->attrs[NBD_ATTR_BLOCK_SIZE_BYTES]) { - u64 bsize = - nla_get_u64(info->attrs[NBD_ATTR_BLOCK_SIZE_BYTES]); - if (!bsize) - bsize = NBD_DEF_BLKSIZE; - if (!nbd_is_valid_blksize(bsize)) { - ret = -EINVAL; - goto out; - } - nbd_size_set(nbd, bsize, div64_u64(config->bytesize, bsize)); - } + ret = nbd_genl_size_set(info, nbd); + if (ret) + goto out; + if (info->attrs[NBD_ATTR_TIMEOUT]) { u64 timeout = nla_get_u64(info->attrs[NBD_ATTR_TIMEOUT]); nbd->tag_set.timeout = timeout * HZ; @@ -1956,6 +1968,10 @@ static int nbd_genl_reconfigure(struct sk_buff *skb, struct genl_info *info) goto out; } + ret = nbd_genl_size_set(info, nbd); + if (ret) + goto out; + if (info->attrs[NBD_ATTR_TIMEOUT]) { u64 timeout = nla_get_u64(info->attrs[NBD_ATTR_TIMEOUT]); nbd->tag_set.timeout = timeout * HZ; -- 2.21.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] nbd: add netlink reconfigure resize support v3 2019-05-29 20:16 ` [PATCH 2/2] nbd: add netlink reconfigure resize support v3 Mike Christie @ 2019-06-13 17:01 ` Josef Bacik 2019-06-13 17:35 ` Mike Christie 0 siblings, 1 reply; 8+ messages in thread From: Josef Bacik @ 2019-06-13 17:01 UTC (permalink / raw) To: Mike Christie; +Cc: josef, linux-block On Wed, May 29, 2019 at 03:16:06PM -0500, Mike Christie wrote: > If the device is setup with ioctl we can resize the device after the > initial setup, but if the device is setup with netlink we cannot use the > resize related ioctls and there is no netlink reconfigure size ATTR > handling code. > > This patch adds netlink reconfigure resize support to match the ioctl > interface. > > Signed-off-by: Mike Christie <mchristi@redhat.com> Sorry I missed this too, but I think there's a problem with this actually. > --- > > V3; > - If the device size or block size has not changed do not call > nbd_size_set. > > V2: > - Merge reconfig and connect resize related code to helper and avoid > multiple nbd_size_set calls. > > drivers/block/nbd.c | 48 ++++++++++++++++++++++++++++++--------------- > 1 file changed, 32 insertions(+), 16 deletions(-) > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > index 236253fbf455..9486555e6391 100644 > --- a/drivers/block/nbd.c > +++ b/drivers/block/nbd.c > @@ -1685,6 +1685,30 @@ nbd_device_policy[NBD_DEVICE_ATTR_MAX + 1] = { > [NBD_DEVICE_CONNECTED] = { .type = NLA_U8 }, > }; > > +static int nbd_genl_size_set(struct genl_info *info, struct nbd_device *nbd) > +{ > + struct nbd_config *config = nbd->config; > + u64 bsize = config->blksize; > + u64 bytes = config->bytesize; > + > + if (info->attrs[NBD_ATTR_SIZE_BYTES]) > + bytes = nla_get_u64(info->attrs[NBD_ATTR_SIZE_BYTES]); > + > + if (info->attrs[NBD_ATTR_BLOCK_SIZE_BYTES]) { > + bsize = nla_get_u64(info->attrs[NBD_ATTR_BLOCK_SIZE_BYTES]); > + if (!bsize) > + bsize = NBD_DEF_BLKSIZE; > + if (!nbd_is_valid_blksize(bsize)) { > + printk(KERN_ERR "Invalid block size %llu\n", bsize); > + return -EINVAL; > + } > + } > + > + if (bytes != config->bytesize || bsize != config->blksize) > + nbd_size_set(nbd, bsize, div64_u64(bytes, bsize)); This part won't actually update the bdev if there already is one because nbd->task_recv is NULL for netlink related devices. Probably need to fix that to update the bdev unconditionally, and then just see if bdget_disk() returns NULL in nbd_size_update. Also I hate myself for how many size update functions there are. Thanks, Josef ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] nbd: add netlink reconfigure resize support v3 2019-06-13 17:01 ` Josef Bacik @ 2019-06-13 17:35 ` Mike Christie 2019-06-13 17:44 ` Josef Bacik 0 siblings, 1 reply; 8+ messages in thread From: Mike Christie @ 2019-06-13 17:35 UTC (permalink / raw) To: Josef Bacik; +Cc: linux-block On 06/13/2019 12:01 PM, Josef Bacik wrote: > On Wed, May 29, 2019 at 03:16:06PM -0500, Mike Christie wrote: >> If the device is setup with ioctl we can resize the device after the >> initial setup, but if the device is setup with netlink we cannot use the >> resize related ioctls and there is no netlink reconfigure size ATTR >> handling code. >> >> This patch adds netlink reconfigure resize support to match the ioctl >> interface. >> >> Signed-off-by: Mike Christie <mchristi@redhat.com> > > Sorry I missed this too, but I think there's a problem with this actually. > >> --- >> >> V3; >> - If the device size or block size has not changed do not call >> nbd_size_set. >> >> V2: >> - Merge reconfig and connect resize related code to helper and avoid >> multiple nbd_size_set calls. >> >> drivers/block/nbd.c | 48 ++++++++++++++++++++++++++++++--------------- >> 1 file changed, 32 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c >> index 236253fbf455..9486555e6391 100644 >> --- a/drivers/block/nbd.c >> +++ b/drivers/block/nbd.c >> @@ -1685,6 +1685,30 @@ nbd_device_policy[NBD_DEVICE_ATTR_MAX + 1] = { >> [NBD_DEVICE_CONNECTED] = { .type = NLA_U8 }, >> }; >> >> +static int nbd_genl_size_set(struct genl_info *info, struct nbd_device *nbd) >> +{ >> + struct nbd_config *config = nbd->config; >> + u64 bsize = config->blksize; >> + u64 bytes = config->bytesize; >> + >> + if (info->attrs[NBD_ATTR_SIZE_BYTES]) >> + bytes = nla_get_u64(info->attrs[NBD_ATTR_SIZE_BYTES]); >> + >> + if (info->attrs[NBD_ATTR_BLOCK_SIZE_BYTES]) { >> + bsize = nla_get_u64(info->attrs[NBD_ATTR_BLOCK_SIZE_BYTES]); >> + if (!bsize) >> + bsize = NBD_DEF_BLKSIZE; >> + if (!nbd_is_valid_blksize(bsize)) { >> + printk(KERN_ERR "Invalid block size %llu\n", bsize); >> + return -EINVAL; >> + } >> + } >> + >> + if (bytes != config->bytesize || bsize != config->blksize) >> + nbd_size_set(nbd, bsize, div64_u64(bytes, bsize)); > > This part won't actually update the bdev if there already is one because > nbd->task_recv is NULL for netlink related devices. Probably need to fix that I'm not sure I understand this part of the comment. For netlink we do: nbd_genl_connect -> nbd_start_device: nbd_start_device() { ..... nbd->task_recv = current; > to update the bdev unconditionally, and then just see if bdget_disk() returns > NULL in nbd_size_update. Also I hate myself for how many size update functions > there are. Thanks, > > Josef > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] nbd: add netlink reconfigure resize support v3 2019-06-13 17:35 ` Mike Christie @ 2019-06-13 17:44 ` Josef Bacik 0 siblings, 0 replies; 8+ messages in thread From: Josef Bacik @ 2019-06-13 17:44 UTC (permalink / raw) To: Mike Christie; +Cc: Josef Bacik, linux-block On Thu, Jun 13, 2019 at 12:35:27PM -0500, Mike Christie wrote: > On 06/13/2019 12:01 PM, Josef Bacik wrote: > > On Wed, May 29, 2019 at 03:16:06PM -0500, Mike Christie wrote: > >> If the device is setup with ioctl we can resize the device after the > >> initial setup, but if the device is setup with netlink we cannot use the > >> resize related ioctls and there is no netlink reconfigure size ATTR > >> handling code. > >> > >> This patch adds netlink reconfigure resize support to match the ioctl > >> interface. > >> > >> Signed-off-by: Mike Christie <mchristi@redhat.com> > > > > Sorry I missed this too, but I think there's a problem with this actually. > > > >> --- > >> > >> V3; > >> - If the device size or block size has not changed do not call > >> nbd_size_set. > >> > >> V2: > >> - Merge reconfig and connect resize related code to helper and avoid > >> multiple nbd_size_set calls. > >> > >> drivers/block/nbd.c | 48 ++++++++++++++++++++++++++++++--------------- > >> 1 file changed, 32 insertions(+), 16 deletions(-) > >> > >> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > >> index 236253fbf455..9486555e6391 100644 > >> --- a/drivers/block/nbd.c > >> +++ b/drivers/block/nbd.c > >> @@ -1685,6 +1685,30 @@ nbd_device_policy[NBD_DEVICE_ATTR_MAX + 1] = { > >> [NBD_DEVICE_CONNECTED] = { .type = NLA_U8 }, > >> }; > >> > >> +static int nbd_genl_size_set(struct genl_info *info, struct nbd_device *nbd) > >> +{ > >> + struct nbd_config *config = nbd->config; > >> + u64 bsize = config->blksize; > >> + u64 bytes = config->bytesize; > >> + > >> + if (info->attrs[NBD_ATTR_SIZE_BYTES]) > >> + bytes = nla_get_u64(info->attrs[NBD_ATTR_SIZE_BYTES]); > >> + > >> + if (info->attrs[NBD_ATTR_BLOCK_SIZE_BYTES]) { > >> + bsize = nla_get_u64(info->attrs[NBD_ATTR_BLOCK_SIZE_BYTES]); > >> + if (!bsize) > >> + bsize = NBD_DEF_BLKSIZE; > >> + if (!nbd_is_valid_blksize(bsize)) { > >> + printk(KERN_ERR "Invalid block size %llu\n", bsize); > >> + return -EINVAL; > >> + } > >> + } > >> + > >> + if (bytes != config->bytesize || bsize != config->blksize) > >> + nbd_size_set(nbd, bsize, div64_u64(bytes, bsize)); > > > > This part won't actually update the bdev if there already is one because > > nbd->task_recv is NULL for netlink related devices. Probably need to fix that > > I'm not sure I understand this part of the comment. For netlink we do: > > nbd_genl_connect -> nbd_start_device: > > nbd_start_device() > { > ..... > nbd->task_recv = current; > Sorry, I can't read apparently. Reviewed-by: Josef Bacik <josef@toxicpanda.com> Thanks, Josef ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-06-13 17:44 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-29 20:16 [PATCH 0/2] nbd: block/dev size changes Mike Christie 2019-05-29 20:16 ` [PATCH 1/2] nbd: fix crash when the blksize is zero v2 Mike Christie 2019-05-30 3:19 ` Xiubo Li 2019-06-13 16:56 ` Josef Bacik 2019-05-29 20:16 ` [PATCH 2/2] nbd: add netlink reconfigure resize support v3 Mike Christie 2019-06-13 17:01 ` Josef Bacik 2019-06-13 17:35 ` Mike Christie 2019-06-13 17:44 ` Josef Bacik
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.