Re: [PATCH v2 01/34] bdev: open block device as files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 02, 2024 at 07:43:24AM +0100, Christoph Hellwig wrote:
> On Thu, Feb 01, 2024 at 06:08:29PM +0100, Christian Brauner wrote:
> > > > +	/*
> > > > +	 * O_EXCL is one of those flags that the VFS clears once it's done with
> > > > +	 * the operation. So don't raise it here either.
> > > > +	 */
> > > > +	if (mode & BLK_OPEN_NDELAY)
> > > > +		flags |= O_NDELAY;
> > > 
> > > O_EXCL isn't dealt with in this helper at all.
> > 
> > Yeah, on purpose was my point bc we can just rely on @holder and passing
> > _EXCL without holder is invalid. But I could add it.
> 
> Ok.  I found it weird to have the comment next to BLK_OPEN_NDELAY
> as it looked like it sneaked through.  Especially as BLK_OPEN_EXCL
> has literally nothing to do with O_EXCL at all as the latter is a
> namespace operation flag.  So even if the comment was intentional
> I think we're probably better off without it.
> 
> > Yes, I had considered that and it would work but there's the issue that
> > we need to figure out how to handle BLK_OPEN_RESTRICT_WRITES. It has no
> > corresponding O_* flag that would let us indicate this.
> 
> Oh, indeed.
> 
> >
> > So I had
> > considered:
> > 
> > 1/ Expose bdev_file_open_excl() so callers don't need to pass any
> >    specific flags. Nearly all filesystems would effectively use this
> >    helper as sb_open_mode() adds it implicitly. That would have the
> >    side-effect of introducing another open helper ofc; possibly two if
> >    we take _by_dev() and _by_path() into account.
> > 
> > 2/ Abuse an O_* flag to mean BLK_OPEN_RESTRICT_WRITES. For example,
> >    O_TRUNC or O_NOCTTY which is pretty yucky.
> > 
> > 3/ Introduce an internal O_* flag which is also ugly. Vomitorious and my
> >    co-maintainers would likely chop off my hands so I can't go near a
> >    computer again.
> > 
> > 3/ Make O_EXCL when passed together with bdev_file_open_by_*() always
> >    imply BLK_OPEN_RESTRICT_WRITES.
> > 
> > The 3/ option would probably be the cleanest one and I think that all
> > filesystems now pass at least a holder and holder ops so this _should_
> > work.
> 
> 2 and 3 sound pretty horrible.  3 would work and look clean for the
> block side, but O_ flags are mess so I wouldn't go there.

My numbering is obviously wrong btw. That last point should obviously be 4/

> 
> Maybe  variant of 1 that allows for a non-exclusive open and clearly
> marks that?
> 
> Or just leave it as-is for now and look into that later.

Ok.




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux