2022-06-22 14:06:46

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH] nfs/blocklayout: refactor block device opening

Deduplicate the helpers to open a device node by passing a name
prefix argument and using the same helper for both kinds of paths.

Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/nfs/blocklayout/dev.c | 42 +++++++++++-----------------------------
1 file changed, 11 insertions(+), 31 deletions(-)

diff --git a/fs/nfs/blocklayout/dev.c b/fs/nfs/blocklayout/dev.c
index 5e56da748b2ab..fea5f8821da5e 100644
--- a/fs/nfs/blocklayout/dev.c
+++ b/fs/nfs/blocklayout/dev.c
@@ -301,18 +301,14 @@ bl_validate_designator(struct pnfs_block_volume *v)
}
}

-/*
- * Try to open the udev path for the WWN. At least on Debian the udev
- * by-id path will always point to the dm-multipath device if one exists.
- */
static struct block_device *
-bl_open_udev_path(struct pnfs_block_volume *v)
+bl_open_path(struct pnfs_block_volume *v, const char *prefix)
{
struct block_device *bdev;
const char *devname;

- devname = kasprintf(GFP_KERNEL, "/dev/disk/by-id/wwn-0x%*phN",
- v->scsi.designator_len, v->scsi.designator);
+ devname = kasprintf(GFP_KERNEL, "/dev/disk/by-id/%s%*phN",
+ prefix, v->scsi.designator_len, v->scsi.designator);
if (!devname)
return ERR_PTR(-ENOMEM);

@@ -326,28 +322,6 @@ bl_open_udev_path(struct pnfs_block_volume *v)
return bdev;
}

-/*
- * Try to open the RH/Fedora specific dm-mpath udev path for this WWN, as the
- * wwn- links will only point to the first discovered SCSI device there.
- */
-static struct block_device *
-bl_open_dm_mpath_udev_path(struct pnfs_block_volume *v)
-{
- struct block_device *bdev;
- const char *devname;
-
- devname = kasprintf(GFP_KERNEL,
- "/dev/disk/by-id/dm-uuid-mpath-%d%*phN",
- v->scsi.designator_type,
- v->scsi.designator_len, v->scsi.designator);
- if (!devname)
- return ERR_PTR(-ENOMEM);
-
- bdev = blkdev_get_by_path(devname, FMODE_READ | FMODE_WRITE, NULL);
- kfree(devname);
- return bdev;
-}
-
static int
bl_parse_scsi(struct nfs_server *server, struct pnfs_block_dev *d,
struct pnfs_block_volume *volumes, int idx, gfp_t gfp_mask)
@@ -360,9 +334,15 @@ bl_parse_scsi(struct nfs_server *server, struct pnfs_block_dev *d,
if (!bl_validate_designator(v))
return -EINVAL;

- bdev = bl_open_dm_mpath_udev_path(v);
+ /*
+ * Try to open the RH/Fedora specific dm-mpath udev path first, as the
+ * wwn- links will only point to the first discovered SCSI device there.
+ * On other distributions like Debian, the default SCSI by-id path will
+ * point to the dm-multipath device if one exists.
+ */
+ bdev = bl_open_path(v, "dm-uuid-mpath-0x");
if (IS_ERR(bdev))
- bdev = bl_open_udev_path(v);
+ bdev = bl_open_path(v, "wwn-0x");
if (IS_ERR(bdev))
return PTR_ERR(bdev);
d->bdev = bdev;
--
2.30.2


2022-07-26 19:25:36

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] nfs/blocklayout: refactor block device opening

ping?

On Wed, Jun 22, 2022 at 03:58:22PM +0200, Christoph Hellwig wrote:
> Deduplicate the helpers to open a device node by passing a name
> prefix argument and using the same helper for both kinds of paths.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> fs/nfs/blocklayout/dev.c | 42 +++++++++++-----------------------------
> 1 file changed, 11 insertions(+), 31 deletions(-)
>
> diff --git a/fs/nfs/blocklayout/dev.c b/fs/nfs/blocklayout/dev.c
> index 5e56da748b2ab..fea5f8821da5e 100644
> --- a/fs/nfs/blocklayout/dev.c
> +++ b/fs/nfs/blocklayout/dev.c
> @@ -301,18 +301,14 @@ bl_validate_designator(struct pnfs_block_volume *v)
> }
> }
>
> -/*
> - * Try to open the udev path for the WWN. At least on Debian the udev
> - * by-id path will always point to the dm-multipath device if one exists.
> - */
> static struct block_device *
> -bl_open_udev_path(struct pnfs_block_volume *v)
> +bl_open_path(struct pnfs_block_volume *v, const char *prefix)
> {
> struct block_device *bdev;
> const char *devname;
>
> - devname = kasprintf(GFP_KERNEL, "/dev/disk/by-id/wwn-0x%*phN",
> - v->scsi.designator_len, v->scsi.designator);
> + devname = kasprintf(GFP_KERNEL, "/dev/disk/by-id/%s%*phN",
> + prefix, v->scsi.designator_len, v->scsi.designator);
> if (!devname)
> return ERR_PTR(-ENOMEM);
>
> @@ -326,28 +322,6 @@ bl_open_udev_path(struct pnfs_block_volume *v)
> return bdev;
> }
>
> -/*
> - * Try to open the RH/Fedora specific dm-mpath udev path for this WWN, as the
> - * wwn- links will only point to the first discovered SCSI device there.
> - */
> -static struct block_device *
> -bl_open_dm_mpath_udev_path(struct pnfs_block_volume *v)
> -{
> - struct block_device *bdev;
> - const char *devname;
> -
> - devname = kasprintf(GFP_KERNEL,
> - "/dev/disk/by-id/dm-uuid-mpath-%d%*phN",
> - v->scsi.designator_type,
> - v->scsi.designator_len, v->scsi.designator);
> - if (!devname)
> - return ERR_PTR(-ENOMEM);
> -
> - bdev = blkdev_get_by_path(devname, FMODE_READ | FMODE_WRITE, NULL);
> - kfree(devname);
> - return bdev;
> -}
> -
> static int
> bl_parse_scsi(struct nfs_server *server, struct pnfs_block_dev *d,
> struct pnfs_block_volume *volumes, int idx, gfp_t gfp_mask)
> @@ -360,9 +334,15 @@ bl_parse_scsi(struct nfs_server *server, struct pnfs_block_dev *d,
> if (!bl_validate_designator(v))
> return -EINVAL;
>
> - bdev = bl_open_dm_mpath_udev_path(v);
> + /*
> + * Try to open the RH/Fedora specific dm-mpath udev path first, as the
> + * wwn- links will only point to the first discovered SCSI device there.
> + * On other distributions like Debian, the default SCSI by-id path will
> + * point to the dm-multipath device if one exists.
> + */
> + bdev = bl_open_path(v, "dm-uuid-mpath-0x");
> if (IS_ERR(bdev))
> - bdev = bl_open_udev_path(v);
> + bdev = bl_open_path(v, "wwn-0x");
> if (IS_ERR(bdev))
> return PTR_ERR(bdev);
> d->bdev = bdev;
> --
> 2.30.2
---end quoted text---

2022-07-27 22:20:37

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH] nfs/blocklayout: refactor block device opening

On Tue, 2022-07-26 at 21:21 +0200, Christoph Hellwig wrote:
> ping?

Queued for the next merge window, and should already be appearing in
linux-next.

>
> On Wed, Jun 22, 2022 at 03:58:22PM +0200, Christoph Hellwig wrote:
> > Deduplicate the helpers to open a device node by passing a name
> > prefix argument and using the same helper for both kinds of paths.
> >
> > Signed-off-by: Christoph Hellwig <[email protected]>
> > ---
> >  fs/nfs/blocklayout/dev.c | 42 +++++++++++-------------------------
> > ----
> >  1 file changed, 11 insertions(+), 31 deletions(-)
> >
> > diff --git a/fs/nfs/blocklayout/dev.c b/fs/nfs/blocklayout/dev.c
> > index 5e56da748b2ab..fea5f8821da5e 100644
> > --- a/fs/nfs/blocklayout/dev.c
> > +++ b/fs/nfs/blocklayout/dev.c
> > @@ -301,18 +301,14 @@ bl_validate_designator(struct
> > pnfs_block_volume *v)
> >         }
> >  }
> >  
> > -/*
> > - * Try to open the udev path for the WWN.  At least on Debian the
> > udev
> > - * by-id path will always point to the dm-multipath device if one
> > exists.
> > - */
> >  static struct block_device *
> > -bl_open_udev_path(struct pnfs_block_volume *v)
> > +bl_open_path(struct pnfs_block_volume *v, const char *prefix)
> >  {
> >         struct block_device *bdev;
> >         const char *devname;
> >  
> > -       devname = kasprintf(GFP_KERNEL, "/dev/disk/by-id/wwn-
> > 0x%*phN",
> > -                               v->scsi.designator_len, v-
> > >scsi.designator);
> > +       devname = kasprintf(GFP_KERNEL, "/dev/disk/by-id/%s%*phN",
> > +                       prefix, v->scsi.designator_len, v-
> > >scsi.designator);
> >         if (!devname)
> >                 return ERR_PTR(-ENOMEM);
> >  
> > @@ -326,28 +322,6 @@ bl_open_udev_path(struct pnfs_block_volume *v)
> >         return bdev;
> >  }
> >  
> > -/*
> > - * Try to open the RH/Fedora specific dm-mpath udev path for this
> > WWN, as the
> > - * wwn- links will only point to the first discovered SCSI device
> > there.
> > - */
> > -static struct block_device *
> > -bl_open_dm_mpath_udev_path(struct pnfs_block_volume *v)
> > -{
> > -       struct block_device *bdev;
> > -       const char *devname;
> > -
> > -       devname = kasprintf(GFP_KERNEL,
> > -                       "/dev/disk/by-id/dm-uuid-mpath-%d%*phN",
> > -                       v->scsi.designator_type,
> > -                       v->scsi.designator_len, v-
> > >scsi.designator);
> > -       if (!devname)
> > -               return ERR_PTR(-ENOMEM);
> > -
> > -       bdev = blkdev_get_by_path(devname, FMODE_READ |
> > FMODE_WRITE, NULL);
> > -       kfree(devname);
> > -       return bdev;
> > -}
> > -
> >  static int
> >  bl_parse_scsi(struct nfs_server *server, struct pnfs_block_dev *d,
> >                 struct pnfs_block_volume *volumes, int idx, gfp_t
> > gfp_mask)
> > @@ -360,9 +334,15 @@ bl_parse_scsi(struct nfs_server *server,
> > struct pnfs_block_dev *d,
> >         if (!bl_validate_designator(v))
> >                 return -EINVAL;
> >  
> > -       bdev = bl_open_dm_mpath_udev_path(v);
> > +       /*
> > +        * Try to open the RH/Fedora specific dm-mpath udev path
> > first, as the
> > +        * wwn- links will only point to the first discovered SCSI
> > device there.
> > +        * On other distributions like Debian, the default SCSI by-
> > id path will
> > +        * point to the dm-multipath device if one exists.
> > +        */
> > +       bdev = bl_open_path(v, "dm-uuid-mpath-0x");
> >         if (IS_ERR(bdev))
> > -               bdev = bl_open_udev_path(v);
> > +               bdev = bl_open_path(v, "wwn-0x");
> >         if (IS_ERR(bdev))
> >                 return PTR_ERR(bdev);
> >         d->bdev = bdev;
> > --
> > 2.30.2
> ---end quoted text---

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]