| Message ID | 20201028072434.1922108-1-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nbd: don't update block size after device is started | expand |
Tested on Ubuntu 20.04.1 LTS with kernel 5.10.0-rc1+, it works fine. Tested-by: lining <lining2020x@163.com> 在 2020/10/28 15:24, Ming Lei 写道: > Mounted NBD device can be resized, one use case is rbd-nbd. > > Fix the issue by setting up default block size, then not touch it > in nbd_size_update() any more. This kind of usage is aligned with loop > which has same use case too. > > Reported-by: lining <lining2020x@163.com> > Cc: Josef Bacik <josef@toxicpanda.com> > Cc: Jan Kara <jack@suse.cz> > Signed-off-by: Ming Lei <ming.lei@redhat.com> > --- > drivers/block/nbd.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > index 3c9485acdd81..e13ce0f75f80 100644 > --- a/drivers/block/nbd.c > +++ b/drivers/block/nbd.c > @@ -296,7 +296,7 @@ static void nbd_size_clear(struct nbd_device *nbd) > } > } > > -static void nbd_size_update(struct nbd_device *nbd) > +static void nbd_size_update(struct nbd_device *nbd, bool start) > { > struct nbd_config *config = nbd->config; > struct block_device *bdev = bdget_disk(nbd->disk, 0); > @@ -313,7 +313,8 @@ static void nbd_size_update(struct nbd_device *nbd) > if (bdev) { > if (bdev->bd_disk) { > bd_set_nr_sectors(bdev, nr_sectors); > - set_blocksize(bdev, config->blksize); > + if (start) > + set_blocksize(bdev, config->blksize); > } else > set_bit(GD_NEED_PART_SCAN, &nbd->disk->state); > bdput(bdev); > @@ -328,7 +329,7 @@ static void nbd_size_set(struct nbd_device *nbd, loff_t blocksize, > config->blksize = blocksize; > config->bytesize = blocksize * nr_blocks; > if (nbd->task_recv != NULL) > - nbd_size_update(nbd); > + nbd_size_update(nbd, false); > } > > static void nbd_complete_rq(struct request *req) > @@ -1308,7 +1309,7 @@ static int nbd_start_device(struct nbd_device *nbd) > args->index = i; > queue_work(nbd->recv_workq, &args->work); > } > - nbd_size_update(nbd); > + nbd_size_update(nbd, true); > return error; > } > >
On 10/28/20 1:24 AM, Ming Lei wrote: > Mounted NBD device can be resized, one use case is rbd-nbd. > > Fix the issue by setting up default block size, then not touch it > in nbd_size_update() any more. This kind of usage is aligned with loop > which has same use case too. Applied, thanks.
On Wed, Oct 28, 2020 at 03:24:34PM +0800, Ming Lei wrote: > Mounted NBD device can be resized, one use case is rbd-nbd. > > Fix the issue by setting up default block size, then not touch it > in nbd_size_update() any more. This kind of usage is aligned with loop > which has same use case too. I think the only reasonable fix here is to remove the set_blocksize call entirely. The concept of block size set by it is a file system construct and nbd has not business setting it at all.
On Thu, Oct 29, 2020 at 07:51:49AM +0000, Christoph Hellwig wrote: > On Wed, Oct 28, 2020 at 03:24:34PM +0800, Ming Lei wrote: > > Mounted NBD device can be resized, one use case is rbd-nbd. > > > > Fix the issue by setting up default block size, then not touch it > > in nbd_size_update() any more. This kind of usage is aligned with loop > > which has same use case too. > > I think the only reasonable fix here is to remove the set_blocksize > call entirely. The concept of block size set by it is a file system > construct and nbd has not business setting it at all. I think the idea is reasonable, we have several drivers(loop, nbd, zram, pktcdvd, bcache) which call into set_blocksize(). Also ioctl(BLKBSZSET) which is used by 'blockdev --setbsz', still not understand which kind of use case is served for. Thanks, Ming
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 3c9485acdd81..e13ce0f75f80 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -296,7 +296,7 @@ static void nbd_size_clear(struct nbd_device *nbd) } } -static void nbd_size_update(struct nbd_device *nbd) +static void nbd_size_update(struct nbd_device *nbd, bool start) { struct nbd_config *config = nbd->config; struct block_device *bdev = bdget_disk(nbd->disk, 0); @@ -313,7 +313,8 @@ static void nbd_size_update(struct nbd_device *nbd) if (bdev) { if (bdev->bd_disk) { bd_set_nr_sectors(bdev, nr_sectors); - set_blocksize(bdev, config->blksize); + if (start) + set_blocksize(bdev, config->blksize); } else set_bit(GD_NEED_PART_SCAN, &nbd->disk->state); bdput(bdev); @@ -328,7 +329,7 @@ static void nbd_size_set(struct nbd_device *nbd, loff_t blocksize, config->blksize = blocksize; config->bytesize = blocksize * nr_blocks; if (nbd->task_recv != NULL) - nbd_size_update(nbd); + nbd_size_update(nbd, false); } static void nbd_complete_rq(struct request *req) @@ -1308,7 +1309,7 @@ static int nbd_start_device(struct nbd_device *nbd) args->index = i; queue_work(nbd->recv_workq, &args->work); } - nbd_size_update(nbd); + nbd_size_update(nbd, true); return error; }
Mounted NBD device can be resized, one use case is rbd-nbd. Fix the issue by setting up default block size, then not touch it in nbd_size_update() any more. This kind of usage is aligned with loop which has same use case too. Reported-by: lining <lining2020x@163.com> Cc: Josef Bacik <josef@toxicpanda.com> Cc: Jan Kara <jack@suse.cz> Signed-off-by: Ming Lei <ming.lei@redhat.com> --- drivers/block/nbd.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)