Re: [PATCH V2 4/6] ublk_drv: add device parameter UBLK_PARAM_TYPE_DEVT

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

On Fri, Nov 25, 2022 at 03:13:58PM +0800, Ziyang Zhang wrote:
> On 2022/11/24 11:04, Ming Lei wrote:
> > Userspace side only knows device ID, but the associated path of ublkc* and
> > ublkb* could be changed by udev, and that depends on userspace's policy, so
> > add parameter of UBLK_PARAM_TYPE_DEVT for retrieving major/minor of the
> > ublkc* and ublkb*, then user may figure out major/minor of the ublk disks
> > he/she owns. With major/minor, it is easy to find the device node path.
> > 
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> >  drivers/block/ublk_drv.c      | 24 +++++++++++++++++++++++-
> >  include/uapi/linux/ublk_cmd.h | 13 +++++++++++++
> >  2 files changed, 36 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > index 9d1578384cba..04a28a2f2e1f 100644
> > --- a/drivers/block/ublk_drv.c
> > +++ b/drivers/block/ublk_drv.c
> > @@ -54,7 +54,8 @@
> >  		| UBLK_F_USER_RECOVERY_REISSUE)
> >  
> >  /* All UBLK_PARAM_TYPE_* should be included here */
> > -#define UBLK_PARAM_TYPE_ALL (UBLK_PARAM_TYPE_BASIC | UBLK_PARAM_TYPE_DISCARD)
> > +#define UBLK_PARAM_TYPE_ALL (UBLK_PARAM_TYPE_BASIC | \
> > +		UBLK_PARAM_TYPE_DISCARD | UBLK_PARAM_TYPE_DEVT)
> >  
> >  struct ublk_rq_data {
> >  	struct llist_node node;
> > @@ -255,6 +256,10 @@ static int ublk_validate_params(const struct ublk_device *ub)
> >  			return -EINVAL;
> >  	}
> >  
> > +	/* dev_t is read-only */
> > +	if (ub->params.types & UBLK_PARAM_TYPE_DEVT)
> > +		WARN_ON_ONCE(1);
> Hi, Ming
> 
> ublk_validate_params() is only called by ublk_ctrl_set_params().
> Why not return -EINVAL here since UBLK_PARAM_TYPE_DEVT is not
> allowed in ublk_ctrl_set_params(). Then the user may know he has
> made a mistake.

Yeah, it is better to return -EINVAL here.

Thanks,
Ming