| Message ID | 20240619173929.177818-8-cel@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Snapshot of fixes for SCSI PR key registration | expand |
On Wed, Jun 19, 2024 at 01:39:32PM -0400, cel@kernel.org wrote: > if (IS_ERR(bdev_file)) { > - pr_warn("pNFS: failed to open device %s (%ld)\n", > + dprintk("failed to open device %s (%ld)\n", > devname, PTR_ERR(bdev_file)); I'd just drop this one entirely. Otherwise this looks good: Reviewed-by: Christoph Hellwig <hch@lst.de>
On 19 Jun 2024, at 13:39, cel@kernel.org wrote: > From: Chuck Lever <chuck.lever@oracle.com> > > Since f931d8374cad ("nfs/blocklayout: refactor block device > opening"), an error is reported when no multi-path device is > found. But this isn't a fatal error if the subsequent device open > is successful. On systems without multi-path devices, this message > always appears whether there is a problem or not. > > Instead, generate less system journal noise by reporting an error > only when both open attempts fail. The new error message is more > actionable since it indicates that there is a real configuration > issue to be addressed. > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > --- > fs/nfs/blocklayout/dev.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/fs/nfs/blocklayout/dev.c b/fs/nfs/blocklayout/dev.c > index b3828e5ee079..356bc967fb5d 100644 > --- a/fs/nfs/blocklayout/dev.c > +++ b/fs/nfs/blocklayout/dev.c > @@ -318,7 +318,7 @@ bl_open_path(struct pnfs_block_volume *v, const char *prefix) > bdev_file = bdev_file_open_by_path(devname, BLK_OPEN_READ | BLK_OPEN_WRITE, > NULL, NULL); > if (IS_ERR(bdev_file)) { > - pr_warn("pNFS: failed to open device %s (%ld)\n", > + dprintk("failed to open device %s (%ld)\n", > devname, PTR_ERR(bdev_file)); > } > > @@ -348,8 +348,11 @@ bl_parse_scsi(struct nfs_server *server, struct pnfs_block_dev *d, > bdev_file = bl_open_path(v, "dm-uuid-mpath-0x"); > if (IS_ERR(bdev_file)) > bdev_file = bl_open_path(v, "wwn-0x"); > - if (IS_ERR(bdev_file)) > + if (IS_ERR(bdev_file)) { > + pr_err("pNFS: no device found for volume %*phN\n", pr_warn is more appropriate here because there can be clients mounting that that don't have block devices for the filesystem. That's not necessarily a configuration problem. Ben > + v->scsi.designator_len, v->scsi.designator); > return PTR_ERR(bdev_file); > + } > d->bdev_file = bdev_file; > bdev = file_bdev(bdev_file); > > -- > 2.45.1
On Thu, Jun 20, 2024 at 08:17:12AM -0400, Benjamin Coddington wrote: > > - if (IS_ERR(bdev_file)) > > + if (IS_ERR(bdev_file)) { > > + pr_err("pNFS: no device found for volume %*phN\n", > > pr_warn is more appropriate here because there can be clients mounting that > that don't have block devices for the filesystem. That's not necessarily a > configuration problem. Yes, agreed.
On Thu, Jun 20, 2024 at 06:36:05AM +0200, Christoph Hellwig wrote: > On Wed, Jun 19, 2024 at 01:39:32PM -0400, cel@kernel.org wrote: > > if (IS_ERR(bdev_file)) { > > - pr_warn("pNFS: failed to open device %s (%ld)\n", > > + dprintk("failed to open device %s (%ld)\n", > > devname, PTR_ERR(bdev_file)); > > I'd just drop this one entirely. Actually I'd like to keep this dprintk because it displays the whole path to the device. That has diagnostic value, IMO.
diff --git a/fs/nfs/blocklayout/dev.c b/fs/nfs/blocklayout/dev.c index b3828e5ee079..356bc967fb5d 100644 --- a/fs/nfs/blocklayout/dev.c +++ b/fs/nfs/blocklayout/dev.c @@ -318,7 +318,7 @@ bl_open_path(struct pnfs_block_volume *v, const char *prefix) bdev_file = bdev_file_open_by_path(devname, BLK_OPEN_READ | BLK_OPEN_WRITE, NULL, NULL); if (IS_ERR(bdev_file)) { - pr_warn("pNFS: failed to open device %s (%ld)\n", + dprintk("failed to open device %s (%ld)\n", devname, PTR_ERR(bdev_file)); } @@ -348,8 +348,11 @@ bl_parse_scsi(struct nfs_server *server, struct pnfs_block_dev *d, bdev_file = bl_open_path(v, "dm-uuid-mpath-0x"); if (IS_ERR(bdev_file)) bdev_file = bl_open_path(v, "wwn-0x"); - if (IS_ERR(bdev_file)) + if (IS_ERR(bdev_file)) { + pr_err("pNFS: no device found for volume %*phN\n", + v->scsi.designator_len, v->scsi.designator); return PTR_ERR(bdev_file); + } d->bdev_file = bdev_file; bdev = file_bdev(bdev_file);