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
> > > <m.tretter@xxxxxxxxxxxxxx> 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 <m.tretter@xxxxxxxxxxxxxx>
> > >>>> ---
> > >>>> 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
