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.
