#2953 PR merged
: New 990_verify_empty_rootfs.sh to verify untouched ROOTFS_DIR at the end of 'prep'¶
Labels: enhancement
, fixed / solved / done
jsmeix opened issue at 2023-03-06 14:32:¶
-
Type: Enhancement
-
Impact: Normal
-
Reference to related issue (URL):
https://github.com/rear/rear/issues/2951 -
How was this pull request tested?
Only a quick test by me:
https://github.com/rear/rear/pull/2953#issuecomment-1456253142 -
Brief description of the changes in this pull request:
New prep/default/990_verify_empty_rootfs.sh
to verify that at the end of the 'prep' stage
the ReaR recovery system area (i.e. ROOTFS_DIR) is still empty.
jsmeix commented at 2023-03-06 14:35:¶
My quick test with
OUTPUT=ISO
BACKUP=NETFS
# usr/sbin/rear -D mkrescue
Relax-and-Recover 2.7 / Git
Running rear mkrescue (PID 2532 date 2023-03-06 15:34:15)
Command line options: usr/sbin/rear -D mkrescue
Using log file: /root/rear.github.master/var/log/rear/rear-linux-h9wr.log
Using build area: /var/tmp/rear.eW6J7hByPMIzjI0
Running 'init' stage ======================
Running workflow mkrescue on the normal/original system
Running 'prep' stage ======================
Using '/usr/bin/xorrisofs' to create ISO filesystem images
Adding backup directory mountpoint 'fs:/other' to EXCLUDE_RECREATE
Using autodetected kernel '/boot/vmlinuz-5.14.21-150400.24.46-default' as kernel in the recovery system
Modified ReaR recovery system area after 'prep' stage (/var/tmp/rear.eW6J7hByPMIzjI0/rootfs not empty)
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var/lib
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var/lib/nfs
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var/lib/nfs/sm
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var/lib/nfs/sm.bak
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var/lib/nfs/v4recovery
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var/lib/nfs/rpc_pipefs
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var/lib/nfs/rpc_pipefs/nfsd
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var/lib/nfs/rpc_pipefs/gssd
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var/lib/nfs/rpc_pipefs/gssd/clntXX
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var/lib/nfs/rpc_pipefs/statd
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var/lib/nfs/rpc_pipefs/mount
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var/lib/nfs/rpc_pipefs/cache
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var/lib/nfs/rpc_pipefs/nfs
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var/lib/nfs/rpc_pipefs/lockd
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var/lib/nfs/rpc_pipefs/portmap
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var/lib/nfs/rpc_pipefs/nfsd4_cb
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/var/lib/nfs/nfsdcltrack
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/etc
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/etc/rear
/var/tmp/rear.eW6J7hByPMIzjI0/rootfs/etc/rear/rescue.conf
Running 'layout/save' stage ======================
Creating disk layout
...
pcahyna commented at 2023-03-06 14:39:¶
I think this needs to be done only if DEBUG is set, at least until the actual problems are solved. Otherwise it will scare users. (I mean specifically the `LogPrintError statement.)
pcahyna commented at 2023-03-06 14:41:¶
And maybe give it 999 (to make clear that it should run at the very end)?
jsmeix commented at 2023-03-06 15:02:¶
I avoid ultimate range boundary numbers like 001 and 999
because there might something appear in the future
that needs to be done at the very very beginning
or at the very very end.
jsmeix commented at 2023-03-06 15:05:¶
How it looks now on the terminal for me:
# usr/sbin/rear -D mkrescue
Relax-and-Recover 2.7 / Git
Running rear mkrescue (PID 5940 date 2023-03-06 15:57:21)
Command line options: usr/sbin/rear -D mkrescue
Using log file: /root/rear.github.master/var/log/rear/rear-linux-h9wr.log
Using build area: /var/tmp/rear.8xy4IML8zGhWONS
Running 'init' stage ======================
Running workflow mkrescue on the normal/original system
Running 'prep' stage ======================
Using '/usr/bin/xorrisofs' to create ISO filesystem images
Adding backup directory mountpoint 'fs:/other' to EXCLUDE_RECREATE
Using autodetected kernel '/boot/vmlinuz-5.14.21-150400.24.46-default' as kernel in the recovery system
Modified ReaR recovery system area after 'prep' stage (/var/tmp/rear.8xy4IML8zGhWONS/rootfs not empty)
Running 'layout/save' stage ======================
Creating disk layout
...
and with only verbose mode
# usr/sbin/rear -v mkrescue
Relax-and-Recover 2.7 / Git
Running rear mkrescue (PID 12405 date 2023-03-06 16:05:01)
Using log file: /root/rear.github.master/var/log/rear/rear-linux-h9wr.log
Running workflow mkrescue on the normal/original system
Using autodetected kernel '/boot/vmlinuz-5.14.21-150400.24.46-default' as kernel in the recovery system
Creating disk layout
...
pcahyna commented at 2023-03-06 15:44:¶
One small note: would it make sense to refer to prep/README
also in
the error message?
jsmeix commented at 2023-03-07 08:50:¶
Currently this is only an initial first step.
As long as the actual problems are not solved
only a normal info message is shown in debug modes
that must not scare users who use our current
work in progress GitHub master code.
jsmeix commented at 2023-03-07 08:50:¶
I would like to merge it in its current state
today afternoon unless there are objections.
schlomo commented at 2023-03-07 09:10:¶
No objections, please go ahead.
About user who use ReaR from Git master: I actually would like show the info to exactly those users. They surely don't expect a stable product (our last release is stable) and will have to accept that our master is also changing and evolving, and that things are changing.
With that, I believe it to be much more important to create strong visibility rather than hiding this.
I actually plan to simply move the code from prep to rescue that modifies the rescue image, andI believe that the risk of doing so is manageable.
jsmeix commented at 2023-03-07 13:59:¶
I close this one as "fixed/solved/done" because
the current intent of this pull request is done, cf.
https://github.com/rear/rear/pull/2953#issuecomment-1457779482
Further enhancements after the actual problems got solved
will happen via separated subsequent pull requests as needed.
pcahyna commented at 2023-03-07 16:39:¶
No objections, please go ahead.
About user who use ReaR from Git master: I actually would like show the info to exactly those users. They surely don't expect a stable product (our last release is stable) and will have to accept that our master is also changing and evolving, and that things are changing.
With that, I believe it to be much more important to create strong visibility rather than hiding this.
I actually disagree with this reasoning. If I am using ReaR master branch from GitHub, and I see a new error message like this, I will assume it is a regression. But it is not a regression, we just started to detect a problematic condition that has existed for a long time. So the message is confusing.
IOW: why do we, developers want to create strong visibility for problems that are detected by internal checks? Because we want users to report them. But in this case, we already know about the problem, so reporting it will be useless (and will only waste our and the reporter's time).
I agree with making the problem visible, but only after solving it for known problematic cases, so that "typical" user of the development code will not encounter this unless the problem is reintroduced.
jsmeix commented at 2023-03-08 07:58:¶
I had it in my mind all the time
but I forgot to tell it here:
After the actual problems got solved
it will become a BugError() when
at the end of the 'prep' stage ROOTFS_DIR is modified
(i.e. when ROOTFS_DIR is not still empty)
because that is a bug in our ReaR code.
This issue does not belong to normal users because
there is nothing what a user could normally do when
at the end of the 'prep' stage ROOTFS_DIR is modified
(except fixing the ReaR scripts but this is not what we expect
that users should normally do for us - which does of course
not mean that we do not appreciate bugfix contributions from users).
jsmeix commented at 2023-03-08 08:40:¶
@pcahyna
I agree with your
https://github.com/rear/rear/pull/2953#issuecomment-1458484921
I think therein you actually meant
... so that "typical" user of the development code
will NOT [instead of "it"] encounter this
unless the problem is reintroduced
pcahyna commented at 2023-03-08 10:02:¶
@jsmeix yes, sorry for making a typo that inverted the meaning of what I wanted to say :-)
[Export of Github issue for rear/rear.]