Add suport for reads from sealed volumes. The main differences are that
catalog index nodes are bigger, and that file extents are kept in a
separate tree.
Writes could in theory be supported as well, as we do in our internal
tool. It would probably be a bit too hacky though, so I'll wait to see
if anybody asks for it.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Define an in-memory omap structure to be shared by all mounts of the
same volume, including both the current transaction and the snapshots.
The on-disk omap is already shared by all of them, so this will make for
saner code.
The patch fixes two existing issues. One is a regression introduced by
39878b3753be ("Preserve omap records in snapshots"), which was causing
regular failures in generic/013 of xfstests. The problem was with the
hacky way in which I told apart container and volume omaps, by setting
sbi->s_vsb_raw to NULL before calling apfs_map_volume_super(); it seems
that the dentry cache or something needs this to be set at all times.
Now that the "latest_snap" field has been moved from the sbi to the omap
struct, we no longer need to handle containers and volumes differently:
container omaps are the same as volume omaps without snapshots.
The other issue is that snapshot mounts only read the location of the
omap root once (on mount), but subsequent writes to the volume will free
this original block and potentially allocate it for something else. I've
never seen this in practice yet, but I would expect the snapshot mount
to start complaining about filesystem corruption. Now that the omap root
node is shared by all mounts of the volume, this problem should be gone.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Attempts to create a second snapshot on a same volume fail, with the
driver reporting corruption. The problem is in apfs_node_locate_key():
we assume that all nodes with fixed key/value length are omap nodes, and
so we work with a wrong value for the length of keys in the snapshots
tree.
These problems would never occur if we just read the key/value lengths
from the root node info footer, and maybe kept them in a btree struct
shared by all the nodes. For now though, just hardcode the correct
length value for trees of this type.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
With the previous patch in place we can now take snapshots, but they
become immediately corrupted when a new transaction begins and changes
the omap records. This happens even if file contents aren't modified,
so it becomes impossible to test this new functionality at all.
So, always preserve omap records that belong to a snapshot. For this
check, the disk layout provides om_most_recent_snap in the object map;
add a matching s_latest_snap field to the in-memory superblock structure
to make it easier to work with.
Bigger changes are needed in apfs_omap_lookup_block(), because we need
to work with an omap record's xid, which has so far mostly been ignored.
Replace apfs_bno_from_query() with a new apfs_omap_map_from_query()
which doesn't discard so much valuable metadata. This change also allows
us to start preserving the flags in the omap record, which may also be
important.
Another problem in apfs_omap_lookup_block() is that we need a way to
tell apart volume omaps from the container omap, which won't be affected
by snapshots. This is all a little hacky, and it would be much cleaner
if I had an in-memory omap struct to pass around instead of the root
nodes. I should look into that in the future.
With this patch in place, my fsck reports no corruption after a single
snapshot is created and the volume is unmounted.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Implement an ioctl that creates a snapshot for the current transaction.
A matching userland tool has already been added to apfsprogs.
Note that, at this point, snapshots won't be handled correctly on later
writes at all. Being able to actually create snapshots for testing is
only the first step in that direction.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
I'm trying to build with "-W" for the first time, and it reports that
apfs_free_query() doesn't use the 'sb' parameter. Get rid of it.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Get rid of the reference counting for in-memory nodes, which is just a
hacky way to avoid accidentally freeing root nodes that are remembered
by the superblock structure. Instead, never delete the root nodes as
part of apfs_free_query() and let the caller decide how to deal with
them.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Rework apfs_node_split() to take advantage of the apfs_object changes
from last patch and avoid the fake buffer head. Now the temporary node
used for the split can be handled like any other node, so do that
instead of using big stack allocations, which are unacceptable in the
kernel.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Add a 'data' field to in-memory objects and point it to the buffer
head's 'b_data'; from now on, always access the data through this field
instead of the bh. To make sure I remembered to clean up all instances,
rename the buffer head in the object from 'bh' to 'o_bh'.
The point is to allow nodes to work with a data block that exists only
in memory, and the next patch will take advantage of this to improve
apfs_node_split(). Since objects that are not nodes will not need this
feature, only adjust the cleanup done in apfs_node_release() for now,
though maybe object cleanups should all be done in one place.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
The record count in a node is a 32-bit number but apfs_node_is_valid()
immediately casts it to an int, so the result may be negative if the
count is unnaturally large. This would bypass the final check in the
function ('records * entry_size <= index_size'), and these sanity checks
are important to prevent out-of-bounds operations, so fix it.
A huge record count in the final check also makes it possible for
multiplication to overflow, so add a check for a rough upper bound
on the record count before actually comparing it to the index size.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Most checksum calculations are already done during transaction commit,
but there are a few exceptions, mostly in the spaceman code. When a
large file is copied, close to 10% of the time is spent in these useless
recalculations. So, defer all of them instead.
Sadly (and confusingly), I don't see any actual performance improvement
in my benchmarks, but I think it's better to leave this in place anyway.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Don't call apfs_sb_bread() to work on blocks that have no valuable data
and will just be overwritten. It's wasteful because the driver must
sleep until the unnecessary read is done. Work instead with a new
apfs_getblk(). The result is a 5% performance improvement when rsyncing
a whole volume.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Update the offset of the current query record key if apfs_node_insert()
expands the table of contents. Otherwise, the function may still find
later on that there's no room for the new record, and return ENOSPC
leaving an inconsistent query. A caller that attempts to refresh such a
query would run into trouble.
This bug was discovered with generic/013 of xfstests.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
The header for apfs_node_split() claims the the query parent will always
be set to NULL on success, but this isn't done correctly if the node is
only defragmented. The problem is that apfs_btree_insert() relies on
this behaviour to decide if a split has been attempted yet, and the
current implementation error will make it loop forever.
So fix apfs_node_split() and make it act as documented.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
I never implemented a way to deal with huge records that take over a
whole node because I thought such a thing wouldn't happen unless xattrs
were used. I was wrong: generic/013 from xfstests regularly aborts
transactions when it tries to create a large inode record right next to
a large (symlink) xattr record.
The difficulty here is that my code always inserts a new record right
after another, in the same node. The node is split and defragmented if
full, but the two records will always be consecutive. If they overflow
the node by themselves, nothing can be done.
So, implement a new insertion mechanism for this special case: just
create a whole new node and put the new record in it. This is probably
bad in terms of disk usage, but it should be rare outside of xfstests.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
After a node gets split, its query will no longer have the parent field
set. If the record we intend to insert is sill too big to fit in the
resulting node half, a second split will be triggered, and the lack of a
parent in which to update the index records will result in null pointer
dereference.
To prevent this, check if a query's parent is gone before a split and,
if that's the case, refresh it. To be safe, also add a check for the
parent inside apfs_node_split().
This is all necessary to fix crashes in generic/013 of xfstests, but it
doesn't fully solve the problems with splits yet. When inserting a
sufficiently large inline xattr, it's possible that a defragmented node
with a single record will still be unable to receive it. Add a check for
this and return -EOPNOTSUPP, to prevent an infinite loop; but I will
need to deal with it for real at some point.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
When a new record key or value is allocated inside the slot left behind
by a larger one that was removed, the free space count gets corrupted.
The problem is that a new entry is added for the room remaining in the
slot, which increases the free space count even though that space was
already counted. Fix this.
After the previous commit ff6e5be159 ("Avoid splitting nodes with a
single record"), this bug started causing failures in generic/001 of
xfstests. This appears to be just a coincidence and not a problem with
that patch.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Generic/011 in xfstests causes a lot of internal fragmentation in the
catalog records. This eventually creates nodes that have a single record
but no free space at all. Splits of this node may be requested, but the
current code doesn't support them (there's even a TODO about it),
because dealing with the resulting temporarily empty nodes would be very
tricky.
For now, avoid the split in these situations and just defragment the
original node. Sadly, this approach won't be enough to deal with the
potentially huge records of inline xattrs: a node holding just one of
those may be legitimately full.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Add support for extent reference tree queries to apfs_delete_node().
The nodes are physical just like those of the object map, so it's a
simple change. It's needed immediately because generic/001 from xfstests
exercises this code path.
One thing to note is that extent reference entries are usually deleted
when an orphan inode gets cleaned up, so the EOPNOTSUPP error code from
this function doesn't reach userland. This made the source of the
problem less obvious, so from now on print an error message to the
console as well.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Attempts to truncate large files are currently failing. The problem is
that a whole new free queue entry gets added for each block in the file,
and the free queue soon runs into the limits of having a single
checkpoint-mapping block. CPM creation will need to be implemented
eventually, but for now just be less wasteful and allow each entry to
cover a range.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Never call apfs_delete_node() for a root node. This is clearly
documented, but I somehow made a mess of it anyway, and it will become a
problem as soon as we start deleting physical extents.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Some compilers complain that bno and oid may be used uninitialized in
apfs_create_node(). This isn't true in practice, because the code path
that triggers the warning will never be run if the function was called
correctly; this is documented by an "ASSERT(false)".
Deal with it anyway, both to avoid the noise during compilation, and to
prevent serious damage if I accidentally misuse this function in the
future.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Our allocation scheme is very naive for now, we just look for the first
available block. We can still allocate large extents this way because we
only write to one file at a time, but the process is interrupted as soon
as a metadata block is needed.
Until I can work on improving allocation, try to avoid this problem by
allocating metadata blocks at the end of the disk.
Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
