On Tue, 17 Apr 2018 11:15:43 +0200, Michael Tretter wrote: > Hi Josef, > > On Sat, 14 Apr 2018 01:10:27 +0000, Josef Bacik wrote: > > Yeah sorry I screwed that up again. I’m wondering if we can just > > drop this altogether and leave the zero setting in the config put > > that we already have. > > I'm not sure if I understand you correctly. The nbd_config_put() > function does not touch the bdev at all. How should config put take > care of setting the size to 0? > > Dropping the bd_set_size to 0 from nbd_start_device_ioctl should be > fine, because usually reset takes care of that. However, if there is > an opener during nbd_bdev_reset(), the size won't be changed to 0 > either. This would still be the same behavior as with my patch.
Ping. Michael > > Michael > > > > Does doing that fix it for you? Thanks, > > > > Josef > > > > Sent from my iPhone > > > > > On Apr 13, 2018, at 10:26 AM, Michael Tretter > > > <[email protected]> wrote: > > >> On Fri, 16 Mar 2018 09:36:45 -0700, Jens Axboe wrote: > > >>> On 3/15/18 3:00 AM, Michael Tretter wrote: > > >>>> On Wed, 28 Feb 2018 10:54:56 +0100, Michael Tretter wrote: > > >>>> If a file system is mounted on the nbd during a disconnect, > > >>>> resetting the size to 0, might change the block size and destroy > > >>>> the buffer_head mappings. This will cause a infinite loop when > > >>>> the file system looks for the buffer_heads for flushing. > > >>>> > > >>>> Only set the file size to 0, if we are the only opener of the > > >>>> nbd. > > >>>> > > >>>> Signed-off-by: Michael Tretter <[email protected]> > > >>>> --- > > >>>> drivers/block/nbd.c | 3 ++- > > >>>> 1 file changed, 2 insertions(+), 1 deletion(-) > > >>>> > > >>>> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > > >>>> index 5f2a4240a204..0d1dbb60430b 100644 > > >>>> --- a/drivers/block/nbd.c > > >>>> +++ b/drivers/block/nbd.c > > >>>> @@ -1118,7 +1118,8 @@ static int nbd_start_device_ioctl(struct > > >>>> nbd_device *nbd, struct block_device *b if (ret) > > >>>> sock_shutdown(nbd); > > >>>> mutex_lock(&nbd->config_lock); > > >>>> - bd_set_size(bdev, 0); > > >>>> + if (bdev->bd_openers <= 1) > > >>>> + bd_set_size(bdev, 0); > > >>>> /* user requested, ignore socket errors */ > > >>>> if (test_bit(NBD_DISCONNECT_REQUESTED, > > >>>> &config->runtime_flags)) ret = 0; > > >>> > > >>> Are there any comments on this patch? > > >> > > >> Josef? > > >> > > > > > > Maybe some history helps for reviewing the patch: > > > > > > Commit abbbdf12497d ("nbd: replace kill_bdev() with > > > __invalidate_device()") already fixed this issue once by changing > > > nbd_size_clear() to only set the size to 0 if the device is only > > > opened once. The line was moved several times during the rework for > > > the netlink interface and the check for the number of openers was > > > dropped, reintroducing the issue. > > > > > > Michael
