Re: [PATCH 0/9][btrfs-progs] Remove unused dirstream variable

[David Sterba <dsterba@suse.cz>]

On Sat, Dec 30, 2023 at 12:20:54PM +0100, Goffredo Baroncelli wrote:
> On 14/12/2023 17.17, David Sterba wrote:
> > On Sat, Dec 09, 2023 at 07:53:20PM +0100, Goffredo Baroncelli wrote:
> >> From: Goffredo Baroncelli <kreijack@inwind.it>
> >>
> >> For historical reason, the helpers [btrfs_]open_[file_or_]dir() work with
> >> directory returning the 'fd' and a 'dirstream' variable returned by
> >> opendir(3).
> >>
> >> If the path is a file, the 'fd' is computed from open(2) and
> >> dirstream is set to NULL.
> >> If the path is a directory, first the directory is opened by opendir(3), then
> >> the 'fd' is computed using dirfd(3).
> >> However the 'dirstream' returned by opendir(3) is left open until 'fd'
> >> is not needed anymore.
> >>
> >> In near every case the 'dirstream' variable is not used. Only 'fd' is
> >> used.
> > 
> > As I'm reading dirfd manual page, dirfd returns the internal file
> > descriptor of the dirstream and it gets closed after call to closedir().
> > This means if we pass a directory and want a file descriptor then its
> > lifetime matches the correspoinding DIR.
> > 
> >> A call to close_file_or_dir() freed both 'fd' and 'dirstream'.
> >>
> >> Aim of this patch set is to getrid of this complexity; when the path of
> >> a directory is passed, the fd is get directly using open(path, O_RDONLY):
> >> so we don't need to use readdir(3) and to maintain the not used variable
> >> 'dirstream'.
> > 
> > Does this work the same way as with the dirstream?
> > 
> 
> Hi David, are you interested in this patch ? I think that it is a
> great simplification.

Sorry, I got distracted by other things.

The patchset does not apply cleanly on 6.7 so I've used 6.6.3 as the
base for testing. There's one failure in the cli tests, result can be
seen here https://github.com/kdave/btrfs-progs/actions/runs/7813042695/job/21311365019

====== RUN MUSTFAIL /home/runner/work/btrfs-progs/btrfs-progs/btrfs filesystem resize 1:-128M /home/runner/work/btrfs-progs/btrfs-progs/tests/test.img
ERROR: not a btrfs filesystem: /home/runner/work/btrfs-progs/btrfs-progs/tests/test.img
failed (expected): /home/runner/work/btrfs-progs/btrfs-progs/btrfs filesystem resize 1:-128M /home/runner/work/btrfs-progs/btrfs-progs/tests/test.img
no expected error message in the output 2
test failed for case 003-fi-resize-args

I think it's related to the changes to dirstream, however I can't
reproduce it locally, only on the github actions CI.

Unlike other commands in the test, this one is called on an image and it
must fail to prevent accidentaly resizing underlying filesystem.