On Sat, Jun 22, 2024 at 07:03:25AM +0200, Christoph Hellwig wrote:
> On Fri, Jun 21, 2024 at 12:22:29PM -0400, cel@xxxxxxxxxx wrote:
> > @@ -367,14 +391,7 @@ bl_parse_scsi(struct nfs_server *server, struct pnfs_block_dev *d,
> > goto out_blkdev_put;
> > }
> >
> > - error = ops->pr_register(file_bdev(d->bdev_file), 0, d->pr_key, true);
> > - if (error) {
> > - pr_err("pNFS: failed to register key for block device %s.",
> > - file_bdev(d->bdev_file)->bd_disk->disk_name);
> > - goto out_blkdev_put;
> > - }
> > -
> > - d->pr_registered = true;
> > + d->pr_register = bl_pr_register_scsi;
>
> I think this will break complex (slice, concat, stripe) volumes,
> as we'll never call ->pr_register for them at all. We'll also need
> a register callback for them, which then calls into underlying
> volume, similar to how bl_parse_deviceid works.
This patch currently adds the pr_reg callback to
bl_find_get_deviceid(), which has no visibility of the volume
hierarchy. Where should the registration be done instead? I'm
missing something.
> That would also
> do away with the need for the d->pr_register callback, we could
> just do the swithc on the volume types which might be more
> efficient. (the same is actually true for the ->map callback,
> but that's a separate cleanup).
--
Chuck Lever
