Message-ID: <171584261210.153302.13800498533021477492@localhost>
Date: Thu, 16 May 2024 08:56:52 +0200
From: Johannes Schauer Marin Rodrigues <josch@...ter-muffin.de>
To: Theodore Ts'o <tytso@....edu>
Cc: linux-ext4@...r.kernel.org
Subject: Re: created ext4 disk image differs depending on the underlying filesystem

Hi Ted,

Quoting Theodore Ts'o (2024-05-11 23:11:11)
> On Sat, May 11, 2024 at 07:34:42AM +0200, Johannes Schauer Marin Rodrigues wrote:
> >  2. allow resetting fs->super->s_kbytes_written to zero. This patch worked for
> >     me:
> > 
> > Would you be happy about a patch for (2.)? If yes, I can send something over
> > once I find some time. :)
> > 
> 
> I'm currently going back and forth about whether we should just (a)
> unconditionally set s_kbytes_writes to zero before we write out the
> superblock, or (b) whether we add a new extended operation, or (c) do
> a hack where if SOURCE_DATE_EPOCH is set, use that as implied "set
> s_kbytes_written to zero since the user is probably caring about a
> reproducible file system".
> 
> (c) is a bit hacky, but it's the most convenient for users, and adding
> Yet Another extended operation.

when changing defaults in my own software I try to think about if and if yes
how users will be able to change that default to get back to how it was before
because xkcd #1172 is very real. Your options a) and c) do not give users a way
to tell mke2fs that yes, they *do* want their filesystem to record how much was
written to it during its creation. Even somebody who uses SOURCE_DATE_EPOCH to
get reproducible output might have this need. Now you can argue "but users who
want this will be very very rare" and i will not disagree but then i think the
need to set s_kbytes_writes to zero because i want reproducible images when
creating an ext4 file system image on top of 9p fs is also very, very rare. And
then I look at option b) which is not nice but isn't it okay to have a
cumbersome option for people in very niche situations?

> Related to this is the design question about whether SOURCE_DATE_EPOCH should
> imply using a fixed value for s_uuid and s_hash_seed.  Again, it's a little
> weird to overload SOURCE_DATE_EPOCH to setting the uuid and hash_seed to some
> fixed value, which might be a time-based UUID with the ethernet address set
> to all zeroes, or some other fixed value.  But it's a pretty good proxy of
> what the user wants, and if this is this is the default, the user can always
> override it via an extended option if they really want something different.

Beware that generating a fitting uuid can quickly become not so fun anymore if
you want to follow the relevant RFC instead of "just making something up". I've
had a talk with the reproducible builds people about this issue as I was
looking for prior art on how to turn a SOURCE_DATE_EPOCH into a predictable
uuid and I was told that the proper way would be to first generate a version 5
uuid using a DNS name you control and then use that uuid as the namespace for
another uuid together with SOURCE_DATE_EPOCH. It was then discussed whether the
reproducible builds team should formalize a method to turn a SOURCE_DATE_EPOCH
into a uuid and document it and how that should be done but it seems to be
tricky to do it right if one wants to follow the relevant RFCs to the letter.

> If it weren't for the fact that I'm considering have SOURCE_DATE_EPOCH
> provide default values for s_uuid and s_hash_seed, I'd be tempted to just
> unconditionally set the s_kbytes_written to zero.
> 
> I'm curious what your opinions might be on this, as someone who might
> want to use this feature.

Not only "might want to use this" but "actively using it":

https://tracker.debian.org/news/1529763/accepted-mmdebstrap-150-1-source-into-unstable/

As the mmdebstrap upstream author, I would have no problem with mke2fs setting
s_uuid and s_hash_seed to some reproducible value by default. As you said, any
user who doesn't like this can always run mke2fs manually and because
mmdebstrap writes tarballs to stdout, adding custom mke2fs options is really
easy:

mmdebstrap | mke2fs -d - -U $(uuidgen) -E hash_seed=$(uuidgen) ...

That being said, I'm not aware of anybody else requiring bit-by-bit
reproducible ext4 images. I never got bit-by-bit reproducible output to work
when using an unpacked filesystem directory as the source for mke2fs. But I'm
at MiniDebConf Berlin this week and just yesterday I met somebody from the
Debian Cloud team who said that they are interested in this functionality. I
shall make them aware of this thread today and maybe they have some further
input.

> > As an end-user I am very interested in keeping the functionality of mke2fs
> > which keeps track of which parts are actually sparse and which ones are
> > not.  This functionality can be used with tools like "bmaptool" (a more
> > clever dd) to only copy those parts of the image to the flash drive which
> > are actually supposed to contain data.
> If the file system where the image is created supports either the FIEMAP
> ioctl or fallocate SEEK_HOLE, then "bmaptool create" can figure out which
> parts of the file is sparse, so we don't need to make any changes to
> e2fsprogs.  If the file system doesn't support FIEMAP or SEEK_HOLE, one could
> imagine that bmaptool could figure out which parts of the file could be
> sparse simply by looking for blocks that are all zeroes.  This is basically
> what "cp --sparse=always" or what the attached make-sparse.c file does to
> determine where the holes could be.
> 
> Yes, I could imagine adding a new io_manager much like test_io and
> undo_io which tracked which blocks had been written, and then would
> write out a BMAP file.  However, the vast majority of constructed file
> systems are quite small, so simply reading all of the blocks to
> determine which blocks were all zeroes ala cp --sparse=always isn't
> going to invole all that much overhead.  And I'd argue the right thing
> to do would be to teach bmaptool how to do what cp --sparse=always so
> that the same interface regardless of whether bmaptool is running on a
> modern file system that supports FIEMAP or SEEK_HOLE, or some legacy
> file system like FAT16 or FAT32.

Thank you for your make-sparse.c. I was wondering whether it is really as
simple as finding all 1024 byte blocks that are all zeros and then skipping
them with lseek, creating a "hole". I imagined that maybe there were some
potential issues of the sort that when my filesystem is part of a disk image
together with a partition table, then maybe due to at what offset the
filesystem is stored on that image or what expectations ext4 or other
filesystems on that image have, there could be situations when it really *is*
necessary to write 1024 consecutive zeroes to my flash drive. At the time that
I was pondering about this, I used fallocate --dig-holes to turn a disk image I
had into a sparse one and this made fsck report problems in the filesystem that
it was not able to fix. I didn't investigate this further.

My need to "dig holes" did not come from the underlying filesystem being a dumb
one like fat or 9p but because it turns out that copying an ext4 image onto a
disk image with an offset while preserving its sparse-ness is not something dd
(or any similar utility I found) was able to do. So at the end of this mail is
the program I am now using to copy the output of mke2fs (which is sparse) onto
my disk image while preserving all the holes created by mke2fs exactly as
mke2fs decided they should be placed. After flashing this to a SD-Card which I
filled with random bytes before, the resulting system booted fine and fsck did
not report any issues. Assuming that the underlying filesystem is smart, I
imagine that simply preserving the holes is the safer option than digging new
ones.

Thanks!

cheers, josch




#define _GNU_SOURCE
#define _LARGEFILE64_SOURCE
#include <sys/types.h>
#include <fcntl.h>
#include <unistd.h>
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>


int main(int argc, char *argv[]) {
    if (argc != 3 && argc != 4) {
        fprintf(stderr, "Usage: %s infile outfile [offset]\n", argv[0]);
        exit(EXIT_FAILURE);
    }
    int infd = open(argv[1], O_RDONLY);
    if (infd == -1) {
        perror("open");
        exit(EXIT_FAILURE);
    }
    off64_t inlength = lseek64(infd, 0, SEEK_END);
    if (inlength == -1) {
        perror("lseek64");
        exit(EXIT_FAILURE);
    }
    int outfd = open(argv[2], O_CREAT | O_WRONLY);
    if (outfd == -1) {
        perror("open");
        exit(EXIT_FAILURE);
    }
    off64_t outlength = lseek64(outfd, 0, SEEK_END);
    if (outlength == -1) {
        perror("lseek64");
        exit(EXIT_FAILURE);
    }
    long long offset = 0;
    if (argc == 4) {
        offset = strtoll(argv[3], NULL, 10);
        if (errno != 0) {
            perror("strtoll");
            exit(EXIT_FAILURE);
        }
    }
    off64_t curr = 0;
    while (true) {
        off64_t data = lseek64(infd, curr, SEEK_DATA);
        if (data == -1) {
            break;
        }
        off64_t hole = lseek64(infd, data, SEEK_HOLE);
        if (hole == -1) {
            hole = inlength;
        }
        off64_t off_out = data + offset;
        ssize_t ret = copy_file_range(infd, &data, outfd, &off_out, hole - data, 0);
        if (ret == -1) {
            perror("copy_file_range");
            exit(EXIT_FAILURE);
        }
        curr = hole;
    }
    if (outlength < inlength + offset) {
        int ret = ftruncate(outfd, inlength + offset);
        if (ret == -1) {
            perror("ftruncate");
            exit(EXIT_FAILURE);
        }
    }
    close(infd);
    close(outfd);
    exit(EXIT_SUCCESS);
}
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists