Re: [PATCH 2/2] blktests/033: add test to cover gendisk leak

[Ming Lei <ming.lei@redhat.com>]

On Thu, Feb 16, 2023 at 08:29:29AM +0000, Shinichiro Kawasaki wrote:
> 
> 
> 
> I suggest the commit title:
> 
>     block/033: add test to cover gendisk leak

OK.

> 
> On Feb 16, 2023 / 11:01, Ming Lei wrote:
> > So far only sync ublk removal is supported, and the device's
> > last reference is dropped in gendisk's ->free_disk(), so it
> > can be used to test gendisk leak issue.
> > 
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> >  common/ublk         | 32 ++++++++++++++++++++++++++++++++
> >  tests/block/033     | 33 +++++++++++++++++++++++++++++++++
> >  tests/block/033.out |  2 ++
> >  3 files changed, 67 insertions(+)
> >  create mode 100644 common/ublk
> >  create mode 100755 tests/block/033
> >  create mode 100644 tests/block/033.out
> > 
> > diff --git a/common/ublk b/common/ublk
> > new file mode 100644
> > index 0000000..66b3a58
> > --- /dev/null
> > +++ b/common/ublk
> > @@ -0,0 +1,32 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-3.0+
> > +# Copyright (C) 2023 Ming Lei
> > +#
> > +# null_blk helper functions.
> 
> I think you meant s/null_blk/ublk/

Sure, :-)

> 
> > +
> > +. common/shellcheck
> > +
> > +_have_ublk() {
> > +	_have_driver ublk_drv
> 
> The _init_ublk() below looks assuming the ublk_drv modules is loadable. If so,
> the live above should be:
> 
>     _have_module ublk_drv

OK

> 
> Of note is that some of the blkteste users run tests with all drivers as
> built-in modules. So it is the better that the new test case can run with
> built-in ublk_drv, if possible. (Or it can be a left work.)

The test doesn't care if it is builtin or module, and I think the choice
should be in hand of user.


Thanks, 
Ming