zfs: let each ZFS provider carry the libsolaris.so manifest entry

[gburd]

Naming a concrete ZFS provider in a module list, rather than the zfs placeholder, silently produces an image with no /usr/lib/fs/libsolaris.so.

Cause

modules/open_zfs/module.py and modules/bsd_zfs/module.py both declare provides = ['zfs'], and scripts/osv/modules/resolve.py registers a provided name as an alias for the providing module:

if hasattr(module, 'provides'):
    for name in getattr(module, 'provides'):
        if(_modules.get(name)):
            raise Exception(...)
        else:
            _modules[name] = module

So resolving open_zfs first registers zfs -> open_zfs. A later require('zfs') finds that key already present and returns early, so modules/zfs/module.py is never imported. That matters because the zfs placeholder is the only one of the two that owns a usr.manifest, and that manifest holds the one line every ZFS image needs:

/usr/lib/fs/libsolaris.so: libsolaris.so

The build still succeeds, which is what makes this unpleasant. Nothing is reported missing; the image just lacks the ZFS library. With fs=ramfs, where scripts/build passes usr.manifest as the bootfs manifest, the guest then dies at startup with Failed to preload ZFS library. Powering off.

Verification

On a clean clone of this repository's master, counting libsolaris.so lines in the generated build/release.x64/usr.manifest, and reading the resolver's own Importing ... lines to confirm the shadowing:

module list	libsolaris.so lines	modules/zfs/module.py imported?
image=native-example,zfs (placeholder named)	1	yes
image=native-example,open_zfs (provider named)	0	no
image=native-example,open_zfs + this change	1	no, but the entry is present

Fix

Give each provider its own copy of the one manifest line it needs, and remove it from the placeholder. The placeholder cannot keep a copy: it require()s the provider, so both appear as distinct resolved modules and generate_manifests would append the line twice. Verified by executing the resolver for every combination (zfs, open_zfs, zfs,zfs-tools, open_zfs,zfs,zfs-tools): exactly one line in each. See the correction comment below for the measured table, including the two-line result that my first version produced.

This deliberately does not touch the resolver. Making a provides alias also load the aliased module's manifest would change module-resolution semantics for every provider using the mechanism, including the java placeholders, to fix something that only shows up here because modules/zfs is the only placeholder that owns a manifest. Two small manifests are the smaller and safer fix.

The same shadowing affects bsd_zfs, so it gets the same entry.

[gburd]

Correcting my own verification table in the description above: it was wrong, and the patch as first pushed had a defect.

I claimed the placeholder path "still yields exactly one line". It yields two. The reasoning in the description ("a provider and the placeholder resolve to the same module object") is also wrong: the placeholder require()s the provider, so both appear as distinct entries in the resolved module set, and generate_manifests appends the manifest of each. Measured by executing the resolver rather than reading it:

module list "zfs"                  -> modules=['bsd_zfs', 'zfs']
    from bsd_zfs -> 1
    from zfs     -> 1
    TOTAL libsolaris.so lines: 2

Not fatal, since mkbootfs would add the same file twice, but the table was published as evidence and it did not hold.

Fixed by keeping the entry on the providers and removing it from the placeholder. Re-verified by executing the resolver for every combination:

module list	before	placeholder + providers	with this fix
zfs	1	2	1
open_zfs	0	1	1
zfs,zfs-tools	1	2	1
open_zfs,zfs,zfs-tools	0	1	1

The underlying bug is unchanged and still reproduces on pristine master: naming a provider directly registers the zfs alias, so a later require('zfs') returns early at scripts/osv/modules/resolve.py:141-146, the placeholder is never imported, and its manifest entry is lost.

Corrected branch: pr/zfs-provider-manifest-v2. I will repoint this PR to it rather than leave the defective table standing.

[gburd]

The repoint promised in the correction above has now landed

My correction comment said the fix was to keep the manifest entry on the
providers and remove it from the placeholder, and that I would repoint this
PR to pr/zfs-provider-manifest-v2. That repoint had not actually happened, so
until now the head still shipped modules/zfs/usr.manifest and produced the
two-line result this thread describes as the defect. The description and the code
disagreed, which is the worse of the two states.

The head branch has been fast-forwarded to 3f0c4579f (a strict fast-forward of
afcf5a7de, no force-push, the original commit untouched). That commit is the
one-hunk removal of modules/zfs/usr.manifest.

Re-verified by executing the resolver, not by reading it, at three revisions with
one harness (counting libsolaris.so lines in the manifests
generate_manifests() would append for each resolved module):

module list	master 0e34e4dc2	previous head afcf5a7de	this head 3f0c4579f
zfs	1	2	1
open_zfs	0	1	1
zfs,zfs-tools	1	2	1
open_zfs,zfs,zfs-tools	0	1	1

The middle column reproduces the defect on demand, so the check is load-bearing
rather than vacuously passing. The resolver trace also still shows the underlying
cause unchanged on master: for open_zfs, the imported modules are
['open_zfs'] only, i.e. modules/zfs/module.py is never imported and its
manifest entry is lost.

No change to the fix itself, the commit message or the described behaviour. Only
the head now matches what the thread says.

[gburd]

The retracted reasoning was still in the commit message; it is now gone

A description is editable forever. A commit message is what gets merged into
git log permanently, so a retraction has to land in all three places
separately: the body, the code, and the commit message. On this PR it landed
in the first two and not the third.

Concretely, the head carried two commits with the same subject:

afcf5a7de  zfs: let each ZFS provider carry the libsolaris.so manifest entry
3f0c4579f  zfs: let each ZFS provider carry the libsolaris.so manifest entry

and afcf5a7de's message asserted the thing this thread already retracted:

The placeholder keeps its copy, so the supported path is unchanged and the
entry is not duplicated: a provider and the placeholder resolve to the same
module object, so the manifest is contributed once.

That reasoning is wrong (the placeholder require()s the provider, so both
appear as distinct resolved modules and the line is appended twice), and
3f0c4579f is the one-hunk removal of modules/zfs/usr.manifest that fixes it.
Because 3f0c4579f was a fast-forward of afcf5a7de, merging this PR would have
put the false claim into git log next to the commit that disproves it.

Fixed by squashing to a single commit whose message states only what the code
does. Head is now c462b0cab, one commit, and the message carries the
three-revision resolver table rather than the retracted "placeholder keeps its
copy" reasoning.

The change is message-only. The resulting tree is byte-identical to the previous
head:

git rev-parse c462b0cab^{tree}  ->  5c473005500a906ac5601fb051b2b75db941648d
git rev-parse 3f0c4579f^{tree}  ->  5c473005500a906ac5601fb051b2b75db941648d

so nothing about the fix or its verification changed, and
git merge-tree --write-tree --messages upstream/master c462b0cab still exits 0.
The previous tip is preserved as the tag pre-audit13-zfs-provider-manifest-3f0c4579f
in case the earlier history is wanted.