makefs: Record inode for all entries

[emaste]

Summary

cd9660 images with Rock Ridge extensions record inode numbers in the File Serial Number in PX records (as of http://svnweb.freebsd.org/changeset/base/260041). Previously makefs in mtree mode left many of these as 0. Some tools (like bsdtar) consider files with identical PX File Serial Numbers to be hard links.

This didn't affect the kernel's cd9660(4); it does not correctly support hard links (PR19782).

PR:             284795

There is an existing reproducible builds issue with PX File Serial Numbers (PR285027), not addressed by this change.

[emaste]

This didn't affect the kernel's cd9660(4); it does not correctly support hard links (PR19782).

cd9660(4) ignores this field, and it does not correctly support hard links (PR19782).

[brooks]

Seems plausible. I do wonder what happens if your input spans filesystems and there are collisions, but that would be a large, existing problem.

[jrtc27]

My recollection is that paths not being present shouldn’t matter, at least for link types that don’t have contents not reflected in the metalog already (e.g. directories or symlinks), yet this will spit out warnings.

You say it left “many” as 0; in what cases does it end up as non-zero?

[emaste]

You say it left “many” as 0; in what cases does it end up as non-zero?

Should probably be "most" are 0. The mtree code currently assigns st_ino here:

/*
 * Check for hardlinks. If the contents key is used, then the check
 * will only trigger if the contents file is a link even if it is used
 * by more than one file
 */
if (sb.st_nlink > 1) {
        fsinode *curino;

        st->st_ino = sb.st_ino;
        st->st_dev = sb.st_dev;
        curino = link_check(node->inode);
        if (curino != NULL) {
                free(node->inode);
                node->inode = curino;
                node->inode->nlink++;
                /* Reset st since node->inode has been updated. */
                st = &node->inode->st;
        }
}

My recollection is that paths not being present shouldn’t matter, at least for link types that don’t have contents not reflected in the metalog already (e.g. directories or symlinks), yet this will spit out warnings.

Shouldn't matter, although it is a difference in behaviour between mtree and tree walk modes that I'd like to avoid. There's only single new warning shown by this code which I've recorded in PR285052.

[emaste]

I took a look at making the output reproducible, and we can address this issue with that change as well -- D49141.

Brooks does raise a good point. I think I'd like to:

1. Move forward with D49141
2. Abandon this change
3. (Later) move D49141's source->target inode mapping into core makefs and use (fs, inode) as the key

[brooks]

In D49134#1121137, @emaste wrote:

I took a look at making the output reproducible, and we can address this issue with that change as well -- D49141.

Brooks does raise a good point. I think I'd like to:

1. Move forward with D49141
2. Abandon this change
3. (Later) move D49141's source->target inode mapping into core makefs and use (fs, inode) as the key

This sounds like a good plan. Over all, leaking local inode numbers seems hard to exploit, but also like something we shouldn't do.

[emaste]

D49141 is committed now and will also solve this issue.