restore: "We seem to be looping" can get lost

[chrysn]

when btrfs restore is run with output redirection (which i'd assume not to be uncommon due to the amount of output), the "We seem to be looping" message can get lost in the buffers. (the operator would be confused by the lack of progress, but not see any indication of a question being asked in the logs of stdout or stderr; i only found the issue because i straced the presumably hanging process).

a fflush(stdout) after the again: jump mark in cmds-restore.c should do the trick. it might be worth considering to write the messages (the question and the loop question) to stderr instead, which afaik should be unbuffered anyway.

[lakeman]

Blocking for user input during a restore is a bad idea anyway IMHO. Files with a lot of churn like logs and databases can easily hit the current loop counter limit.
Obviously we should be defensive against any kind of broken filesystem, but can we actually detect the kinds of looping problems we need to deal with?
Or at least provide a command line flag to suppress the prompt?
Plus I believe that there is another bug here as we are assuming that stdin is not being buffered. Leading the user to add a line-feed to the input, which is assumed to mean STOP for the next prompt.

[tomato42]

if the checksums are correct, why report it at all?

[lakeman]

A hypothetical software bug could have created a valid circular loop in the filesystem?

[tomato42]

unlike the quite common hardware failure?

if software bug really is a possibility, provide a switch for a last resort attempt, but if everything works exactly as designed on the software side, there's no point in bothering the user

[lakeman]

btrfs restore is the last resort, when everything else has failed. So it should be as defensive as possible to anything that any version of btrfs, past or future, might have done to the on disk structure.

[tomato42]

is the documentation outdated then?

https://btrfs.wiki.kernel.org/index.php/Btrfsck

• btrfs restore will help you copy data off a broken btrfs filesystem. See its page: Restore
• btrfs check --repair, aka btrfsck is your last option if the ones above have not worked.

[unquietwiki]

This issue still exists in 5.1. *static enum loop_response ask_to_continue(const char file) in https://github.com/kdave/btrfs-progs/blob/master/cmds-restore.c ; probably need to add a static int as a boolean to add a 4th option.