Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Andreas Dilger <adilger@dilger.ca>
Message-Id: <605DCBDE-A388-4B98-BF5A-38773F15E3F4@dilger.ca>
Content-Type: multipart/signed;
 boundary="Apple-Mail=_CEB716EB-3B38-4573-8174-C14C57FB2842";
 protocol="application/pgp-signature"; micalg=pgp-sha256
Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\))
Subject: Re: [PATCH 1/1] mke2fs: the -d option can now handle tarball input
Date:   Thu, 20 Jul 2023 17:53:56 -0600
In-Reply-To: <20230711235354.GE11476@frogsfrogsfrogs>
Cc:     Johannes Schauer Marin Rodrigues <josch@mister-muffin.de>,
        Ext4 Developers List <linux-ext4@vger.kernel.org>
To:     "Darrick J. Wong" <djwong@kernel.org>
References: <20230620121641.469078-1-josch@mister-muffin.de>
 <20230620121641.469078-2-josch@mister-muffin.de>
 <20230630155128.GA11419@frogsfrogsfrogs>
 <168836303674.2483784.4947178089926484601@localhost>
 <20230711235354.GE11476@frogsfrogsfrogs>
Precedence: bulk


--Apple-Mail=_CEB716EB-3B38-4573-8174-C14C57FB2842
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii

On Jul 11, 2023, at 5:53 PM, Darrick J. Wong <djwong@kernel.org> wrote:
>=20
> On Mon, Jul 03, 2023 at 07:43:56AM +0200, Johannes Schauer Marin =
Rodrigues wrote:
>>=20
>> Quoting Darrick J. Wong (2023-06-30 17:51:28)
>>> On Tue, Jun 20, 2023 at 02:16:41PM +0200, Johannes Schauer Marin =
Rodrigues wrote:
>>>> If archive.h is available during compilation, enable mke2fs to read =
a
>>>> tarball as input. Since libarchive.so.13 is opened with dlopen,
>>>> libarchive is not a hard library dependency of the resulting =
binary.
>>>=20
>>> I can't say I'm in favor of adding build dependencies to e2fsprogs,
>>> since the point of -d taking a directory arg was to *avoid* having =
to
>>> understand anything other than posix(ish) directory tree walking =
APIs.
>>=20
>> this is why the build dependency is optional.
>=20
> As Ted said elsewhere, the big question is (a) do we really want
> e2fsprogs depending on libarchive at all, and (b) is libarchive's API
> stable enough that you'll maintain it for us?  Merging this patch *is*
> adding to the complexity of what most distros consider to be critical
> system utility.

FWIW, I've been looking at using ext4 filesystem images as random-access
archive files that can be directly mounted and used, rather than having
application workflows untar many small files into the filesystem, read
them for a short time, and then delete them again.  That is especially
important for workflows like AI/ML or genomics that are only reading a
subset of the files on each pass.

Storing all of the small files into an ext4 image would allow it to be
loopback mounted and accessed directly without the added write/unlink
overhead for each job.

Being able to import a tarball (or zipfile, or whatever libarchive =
allows)
directly into an ext4 image would be super convenient for this, so I'd
be in favor of including this functionality into mke2fs.  I hadn't even
though of this aspect of the workflow, but it would certainly simplify
things.

>> It should be perfectly possible to build e2fsprogs without libarchive
>> as well. I copied the pattern that was already implemented for =
libmagic
>> which is also not a hard dependency but gets dlopened-ed at runtime.
>> If this mechanism is fine for libmagic it should be fine for others, =
no?

IMHO, yes, if the code is sufficiently isolated and doesn't cause much
ongoing maintenance effort.

Having just looked through the patch, I think it could use some cleanup.
Basic code style issues:
- wrapping lines at 80-columns
- avoid use of C++ comments in the code
- split large highly-indented code blocks into helper functions
- consistent indentation for continued lines
- consistent one blank line between functions
- consistent one blank line after variable declarations

In terms of code structure, refactoring it to put libarchive handling
in a separate file  (e.g. mke2fs-archive.c or similar) would also make
the maintenance easier, since it can be added/removed from the build
more easily, and (if necessary) removed from the tree if it is no longer
working.

Then have only a couple of small function calls in the main mke2fs.c
code that are accessing the libarchive functionality if it is built-in,
or being no-ops (or just printing the error message) if libarchive is
unavailable.

Cheers, Andreas


--Apple-Mail=_CEB716EB-3B38-4573-8174-C14C57FB2842
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename=signature.asc
Content-Type: application/pgp-signature;
	name=signature.asc
Content-Description: Message signed with OpenPGP

-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org

iQIzBAEBCAAdFiEEDb73u6ZejP5ZMprvcqXauRfMH+AFAmS5yRQACgkQcqXauRfM
H+CIkA/8DtcnQiyLoESrbVulUPYbXq5Wy+W9/rwxCQ/bXWhwfvSy3JnHBsQDzF7a
EWtkLXsLrtU7yTTyAgq2ON4afUQPQUdehqmmVvJ96OTgNmNc+Wg0mM5dUzYzVDdk
amCws6Lh285LtGDSWeqcGMAvM8DQ3Kso2jx/hHIGBAZrhFwBJXFrN92wqEcbqLLc
NxlWbLfzf+qNugm35sajRLdiqZ4fMVD3efTswQ8qLO4/ZrPT4LqawnI2/RN8XpHY
lmnidc7sGBiDVgJPtJRDqn/ki7Fy+t2qL/ZrbicMBOfkrMrAcKLnc+vMBLBjrOTX
ugFEMctDU2tqFO3D//Vj/vpaUgE40vue3T/fUTJXtUBUM26K3ai0xm4Q4NAEzhbm
oG4ElX5uqFD/4gR+/7rxr4Ouqffb3ES0ltH3ytlrP8xoPtdCNiPsGmz/BtYpF40I
HMSsiV9jsz3CbqtXAwiNRzf9qqzfECQX8VRjY40t3CzWraQ68u3wHppuAclnkVDO
Zs2uLqQtJicWUOy4q+sNUnctTjamu2NO8ypHvPj2DsqXNpZMq5d/p80hFgATQgsQ
tb4+SrbDDgL3tO+Vz7i68fII58edj/ihYiAIFcsyWq+MuXleAL9qHQPBvUZ/WIhH
GlHPr47wRwv36RtSkGZ16wGvJP6BZdQrJBPGhKhRElkYwtLlPjg=
=0Bf1
-----END PGP SIGNATURE-----

--Apple-Mail=_CEB716EB-3B38-4573-8174-C14C57FB2842--