| Message ID | 20190529201606.14903-3-mchristi@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nbd: block/dev size changes | expand |
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
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 >
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
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;
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(-)