On Mon, Jul 18, 2022 at 02:22:44PM -0400, Sweet Tea Dorminy wrote: > > > At each log entry, we want to assert a > > somewhat complicated invariant: > > > > If verity has not yet started: an orphan indicates that verity has > > started. > > If verity has started: mount should handle the orphan and blow away > > verity data: expect 0 merkle items after mounting the snapshot dev. If > > we can measure the file, verity has finished. > > If verity has finished: the orphan should be gone, so mount should not > > blow away merkle items. Expect the same number of merkle items before > > and after mounting the snapshot dev. > > I was a bit confused by the mix of invariants and state transition > conditions, and think this would be somewhat clearer to split into 'state > transition' and 'state invariant' sections, perhaps as follows: > > There are three possible states for a given point in the log: initially > verity has not yet started; then verity has started but not finished; and > finally verity has finished. The log must proceed through these states in > order: verity starts when an orphan item is added; and > verity has finished when, post-mount, the verity tool can measure the file. > > Each state has its own invariant for testing: > - If verity has not yet started: no verity items exist. > - If verity has started: mount should handle the orphan and blow away > verity data: expect 0 merkle items after mounting. > - If verity has finished: the orphan should be gone and mount should not > blow away merkle items. Expect the same number of merkle items before > and after mounting. Thank you so much for untangling the state transitions from the invariant at each state, that makes it much much clearer. > > > > > Note that this relies on grepping btrfs inspect-internal dump-tree. > > Until btrfs-progs has the ability to print the new Merkle items, they > > will show up as UNKNOWN.36/37. > > I think that progs versions 5.17+ include print verity items [1] but I don't > know how tests deal with version-dependent output... > > [1] https://github.com/kdave/btrfs-progs/commit/c4947248580c20869e75e8e61fb9b5e020053b3c Good point! I think we should just target the new one? > > > +replay_log_prog=$here/src/log-writes/replay-log > > +num_entries=$($replay_log_prog --log $LOGWRITES_DEV --num-entries) > > +entry=$($replay_log_prog --log $LOGWRITES_DEV --replay $replay_dev > > --find --end-mark mkfs | cut -d@ -f1) > > +$replay_log_prog --log $LOGWRITES_DEV --replay $replay_dev --limit > > $entry || \ > > + _fail "failed to replay to start entry $entry" > > +let entry+=1 > > + > > +# state = 0: verity hasn't started > > +# state = 1: verity underway > > +# state = 2: verity done > > +state=0 > > +while [ $entry -lt $num_entries ]; > > +do > > + $replay_log_prog --limit 1 --log $LOGWRITES_DEV --replay $replay_dev > > --start $entry || \ > > + _fail "failed to take replay step at entry: $entry" > > + > > + $LVM_PROG lvcreate -s -L 4M -n $snapname $vgname/$lvname > > >>$seqres.full 2>&1 || \ > > + _fail "Failed to create snapshot" > > + $UDEV_SETTLE_PROG >>$seqres.full 2>&1 > > + > > + orphan=$(count_item $snap_dev ORPHAN) > > + if [ $state -eq 0 ]; then > > + [ $orphan -gt 0 ] && state=1 > > + fi > This being in an if is inconsistent with the state transitions a few lines > down; it would be nice to be consistent, though I don't have a preference > about which way. Oh yeah, I am a bit inconsistent. I'll try to make it more uniform. > > + > > + pre_mount=$(count_item $snap_dev UNKNOWN.3[67]) > > + _mount $snap_dev $SCRATCH_MNT || _fail "mount failed at entry $entry" > > + fsverity measure $SCRATCH_MNT/fsv >>$seqres.full 2>&1 > > + measured=$? > > + umount $SCRATCH_MNT > > + [ $state -eq 1 ] && [ $measured -eq 0 ] && state=2 > > + [ $state -eq 2 ] && ([ $measured -eq 0 ] || _fail "verity done, but > > measurement failed at entry $entry") > > + post_mount=$(count_item $snap_dev UNKNOWN.3[67]) > > + > > + echo "entry: $entry, state: $state, orphan: $orphan, pre_mount: > > $pre_mount, post_mount: $post_mount" >> $seqres.full > > + > > + if [ $state -eq 1 ]; then > > + [ $post_mount -eq 0 ] || \ > > + _fail "mount failed to clear under-construction merkle items pre: > > $pre_mount, post: $post_mount at entry $entry"; > > + fi > > + if [ $state -eq 2 ]; then > > + [ $pre_mount -gt 0 ] || \ > > + _fail "expected to have verity items before mount at entry $entry" > > + [ $pre_mount -eq $post_mount ] || \ > > + _fail "mount cleared merkle items after verity was enabled > > $pre_mount vs $post_mount at entry $entry"; > > + fi > > + > > + let entry+=1 > > + $LVM_PROG lvremove $vgname/$snapname -y >>$seqres.full > > +done > > I'm not understanding the snapshot part. It seems like most tests using > log-writes do `_log_writes_replay_log_range $cur $SCRATCH_DEV >> > $seqres.full` to start each iteration; and then it seems like this test can > check the item counts before and after a _scratch_mount/_scratch_umount > cycle and get the same results. (And, if that worked, the test wouldn't > need its own _cleanup() and its own lv management, I think?) But I'm > probably missing something. IIRC, the purpose of the snapshots is that the mount/unmount cycle is destructive in the middle of the operation. If the orphan is present, we'll blow up all the verity items, so if we did it on the device we were replaying onto, it would leave it in a messed up state as we kept replaying. So we snapshot at each entry and mount/unmount that to check the invariants. I think I might be able to switch to the helper functions for advancing the log from FUA to FUA instead of by 1 entry each time, though. That might make the test a bit faster :)
|