On Thu 24-03-22 13:20:41, Jan Kara wrote:
> On Thu 24-03-22 08:51:07, Christoph Hellwig wrote:
> > The bdev parameter to ->ioctl contains the block device that the ioctl
> > is called on, which can be the partition. But the code in nbd_ioctl
> > that uses it really wants the whole device for things like the bd_openers
> > check. Switch to not pass the bdev along and always use nbd->disk->part0
> > instead.
> >
> > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
>
> Looks good. Feel free to add:
>
> Reviewed-by: Jan Kara <jack@xxxxxxx>
Hum, thinking about this some more...
> > -static void nbd_clear_sock_ioctl(struct nbd_device *nbd,
> > - struct block_device *bdev)
> > +static void nbd_clear_sock_ioctl(struct nbd_device *nbd)
> > {
> > sock_shutdown(nbd);
> > - __invalidate_device(bdev, true);
> > - nbd_bdev_reset(bdev);
> > + __invalidate_device(nbd->disk->part0, true);
> > + nbd_bdev_reset(nbd);
Should't we call __invalidate_device() for the partition bdev here? Because
if the NBD device has partitions, filesystem will be mounted on this
partition and we want to invalidate it. Similarly the partition buffer
cache is different from the buffer cache of the whole device and we should
invalidate the partition one. In fact in cases like this I think we need
to invalidate all the partitions and filesystems that are there on this
disk so neither the old, nor the new code looks quite correct to me. Am I
missing something?
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
