Re: [PATCH V2] xfs_repair: fix progress reporting

[Donald Douwsma <ddouwsma@redhat.com>]

On 19/05/2020 22:53, Eric Sandeen wrote:
> On 5/19/20 2:03 AM, Donald Douwsma wrote:
>>
>> On 19/05/2020 11:29, Eric Sandeen wrote:
>>> The Fixes: commit tried to avoid a segfault in case the progress timer
>>> went off before the first message type had been set up, but this
>>> had the net effect of short-circuiting the pthread start routine,
>>> and so the timer didn't get set up at all and we lost all fine-grained
>>> progress reporting.
>>>
>>> The initial problem occurred when log zeroing took more time than the
>>> timer interval.
>>>
>>> So, make a new log zeroing progress item and initialize it when we first
>>> set up the timer thread, to be sure that if the timer goes off while we
>>> are still zeroing the log, it will be initialized and correct.
>>>
>>> (We can't offer fine-grained status on log zeroing, so it'll go from
>>> zero to $LOGBLOCKS with nothing in between, but it's unlikely that log
>>> zeroing will take so long that this really matters.)
>>>
>>> Reported-by: Leonardo Vaz <lvaz@redhat.com>
>>> Fixes: 7f2d6b811755 ("xfs_repair: avoid segfault if reporting progre...")
>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>> ---
>> I've been looking at this myself, got stuck writing an xfstest, which this
>> passes, though the fix I was trying missed at least one of the formatters
>> that this fixes, and the log zeroing is a nice touch.
>>
>> Reviewed-by: Donald Douwsma <ddouwsma@redhat.com>
> 
> Hm, serves you right for writing a validation test.  ;)  sorry :(
> 

My fix ended up being a bit... primitive, so all good!

> But:  "the fix I was trying missed at least one of the formatters
> that this fixes" - which one is that?  I didn't think I fixed any existing
> thing.

Actually its probably my test that's a bit busted, I wanted to catch the
all of the formatting variations, but some are time sensitive, so its hit
second vs seconds and my filter didn't cope.


Ran: xfs/516
Failures: xfs/516
Failed 1 of 1 tests

[root@rhel7 xfstests-dev]# diff -u /root/Devel/upstream/xfstests-dev/tests/xfs/516.out /root/Devel/upstream/xfstests-dev/results//xfs/516.out.bad
--- /root/Devel/upstream/xfstests-dev/tests/xfs/516.out	2020-05-19 15:49:58.736465391 +1000
+++ /root/Devel/upstream/xfstests-dev/results//xfs/516.out.bad	2020-05-19 16:30:45.257220692 +1000
@@ -2,6 +2,7 @@
 Format and populate
 Introduce a dmdelay
 Run repair
+	- #:#:#: Phase #: #% done - estimated remaining time # minutes, # second
 	- #:#:#: Phase #: #% done - estimated remaining time # minutes, # seconds
 	- #:#:#: Phase #: elapsed time # second - processed # inodes per minute
 	- #:#:#: Phase #: elapsed time # seconds - processed # inodes per minute
@@ -13,3 +14,4 @@
         - #:#:#: scanning filesystem freespace - # of # allocation groups done
         - #:#:#: setting up duplicate extent list - # of # allocation groups done
         - #:#:#: verify and correct link counts - # of # allocation groups done
+        - #:#:#: zeroing log - # of # blocks done