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

Ming Lei
2022-12-07T02:33:04Z

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