#1359 PR merged: Enhancement what MODULES get included in the reovery system¶
Labels: enhancement, documentation, cleanup,
fixed / solved / done, minor bug
jsmeix opened issue at 2017-05-10 14:57:¶
Implemented support for additional meta values like
MODULES=( 'none' )
MODULES=( 'all' )
MODULES=( 'loaded' )
in build/GNU/Linux/400_copy_modules.sh
what MODULES get included in the recovery system
see
https://github.com/rear/rear/issues/1202
and
https://github.com/rear/rear/issues/1355
jsmeix commented at 2017-05-10 14:59:¶
Do not yet merge it.
This is only a very first attempt mainly FYI
so that you could have a very first look.
For now I only tested that the current default behaviour
did not change for me (on SLES12), nothing else is tested yet.
ubeaut commented at 2017-05-11 05:40:¶
Hi.
I am new to this git stuff and I don't know if this is the correct place
for this comment.
I have noticed that if I create a rescue disk on a headless system (no
keyboard or screen) and then I have to do a restore the usb keyboard
doesn't work. So if I need to enter anything on the screen I can't.
I got around this problem by using a computer without a usb keyboard.
Once I restored the system I plugged a USB keyboard into the restored
system and then created a rescue disk and then I tested that rescue disk
and the usb keyboard worked. I could enter things via keyboard.
So my question is how can I insert the modules for a usb keyboard into
the rescue disk?
Maybe have the usb modules for keyboard loaded by default?
Other than that problem the restore worked well.
Thanks
jsmeix commented at 2017-05-11 09:11:¶
@ubeaut
please use a separated GitHub issue
to report things with USB keyboards.
In general when reporting issues see
https://raw.githubusercontent.com/rear/rear/master/.github/ISSUE_TEMPLATE.md
In particular regarding USB keyboards have a look
at the comments in
https://github.com/rear/rear/pull/1244
Do you use current ReaR GitHub master code?
jsmeix commented at 2017-05-11 09:51:¶
@schlomo
I agree that MODULES=( 'all' ) should mean
all modules that match $KERNEL_VERSION
because no other kernel exists in the recovery system.
Regarding the ResolveModules and ModulesCopyTo
functions: For my very first attempt I do all directly
in the script. Later I will see to what extent it helps
to extract things into separated functions. Then
I may adapt linux-functions.sh appropriately.
Regarding the artificial 'for' loop:
Depending on the particular case I prefer it
over lengthy global 'if elif else fi' or 'case' constructs
which add unnecessary levels of nesting and indentation.
The artificial 'for' loop is my general way to make an
arbitrary code block wherefrom I can jump forward
and out at any place.
It is my way to implement 'goto' as it is normally used
in C to jump straight forward usually to some kind of
"finishing" or "cleanup" code at the bottom, cf.
http://stackoverflow.com/questions/245742/examples-of-good-gotos-in-c-or-c
jsmeix commented at 2017-05-11 11:17:¶
Now modules in EXCLUDE_MODULES are generically removed
regardless if the module name or the module filename is
specified in EXCLUDE_MODULES, cf.
https://github.com/rear/rear/pull/1244#issuecomment-286718122
Now EXCLUDE_MODULES are removed in any case,
also for MODULES=( 'all' ) and MODULES=( 'loaded' )
so that the user has the final power.
schlomo commented at 2017-05-11 12:44:¶
@jsmeix, I think that goto doesn't lead to clean code so that I would
really prefer an if construct here. If the blocks are too long for
your
taste then this is rather a sign that you should extract the block into
a
function instead.
On 11 May 2017 at 13:17, Johannes Meixner notifications@github.com wrote:
Now modules in EXCLUDE_MODULES are generically removed
regardless if the module name or the module filename is
specified in EXCLUDE_MODULES, cf.
#1244 (comment) https://github.com/rear/rear/pull/1244Now EXCLUDE_MODULES are removed in any case,
also for MODULES=( 'all' ) and MODULES=( 'loaded' )
so that the user has the final power.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rear/rear/pull/1359#issuecomment-300759479, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAGMCIzjzlVmPpMyeJZM0i18AqaWxlrWks5r4u5MgaJpZM4NWyh8
.
jsmeix commented at 2017-05-11 13:25:¶
@schlomo
I think we need to agree that we disagree here
because from my point of view that 'goto' usage
leads to cleaner code (of course "from my point of view"
because otherwise I would not implement it this way).
Perhaps there is a more obvious way in bash
how to "jump out and straight forward"?
I think it is a matter of taste who understands
some kind of coding patterns better than others.
jsmeix commented at 2017-05-11 13:55:¶
I noticed that I have in "rear -d -D mkrescue" log
things like (excerpts):
+ source /root/rear/usr/share/rear/rescue/GNU/Linux/260_storage_drivers.sh ... ++ IsInArray Module rbd xen-blkback ... nosy find: ''\''/lib/modules/4.4.21-69-default/kernel/drivers/ide'\'':' No such file or directory pata_sch ...
jsmeix commented at 2017-05-11 14:09:¶
According to
git log -p --follow usr/share/rear/rescue/GNU/Linux/230_storage_and_network_modules.sh
it seems
rescue/GNU/Linux/230_storage_and_network_modules.sh
was falsely fixed via
https://github.com/rear/rear/commit/d6d1ff253a244dde4dbf5e0b5676150d028f1cf7
that reads
dropping find error messages
but implements it by adding '2>&1' that results
..._DRIVERS=( $( find ... 2>&1 | sed ... ) )
which adds the find error messages to the module arrays.
I really appreciate the commment in that commit that tells what
it intent is so that I can now easily fix it (without blind guessing
what might have been actually meant with strange looking code).
jsmeix commented at 2017-05-11 15:06:¶
FYI: This is still work in progress and
under somewhat heavy development
but I think I make good progress towards
more reliable, predicatable, and configurable
kernel modules inclusion in the recovery system.
jsmeix commented at 2017-05-11 15:39:¶
At first glance the current code seems to work too good for me
(on my SLES12 test system) - so that I believe there must be
at least one evil regression hidden somewhere ;-)
On my SLES12 test system "rear mkbackup/mkrescue"
and "rear recover" work for the default and
for MODULES=( 'all_modules' )
and MODULES=( 'loaded_modules' )
and MODULES=( 'no_modules' ).
Even with MODULES=( 'no_modules' ) my recovery system
comes up somewhat successfully to some extent but
e.g. there is no networking because there is no network
driver module (and things like that - as expected).
FYI:
on my SLES12 test system I get
those recovery system ISO sizes
by default: 77M
with MODULES=( 'all_modules' ): 94M
with MODULES=( 'loaded_modules' ): 64M
with MODULES=( 'no_modules' ): 61M
I made the recovery system ISOs
with gzip default compression
cf.
https://github.com/rear/rear/issues/1142
and with FIRMWARE_FILES=( 'No' )
cf.
https://github.com/rear/rear/issues/1216
jsmeix commented at 2017-05-12 12:05:¶
Now I use those more explicit special MODULES values
MODULES=( 'all_modules' )
MODULES=( 'loaded_modules' )
MODULES=( 'no_modules' )
and I adapted my last comment
https://github.com/rear/rear/pull/1359#issuecomment-300829232
accordingly.
jsmeix commented at 2017-05-12 12:46:¶
My current code works so well for me that I will merge it
so that also others can test it and report if there are issues
or even regressions which I will of course fix.
schlomo commented at 2017-05-12 12:56:¶
Please make sure to also test the negative code flows, e.g. if a
module
cannot be found.
On 12 May 2017 at 14:51, Johannes Meixner notifications@github.com wrote:
Merged #1359 https://github.com/rear/rear/pull/1359.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rear/rear/pull/1359#event-1079917314, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AAGMCNP-C7ynBSKEHqkaUneiPBZv1Tbiks5r5FW-gaJpZM4NWyh8
.
jsmeix commented at 2017-05-12 13:09:¶
I had tested MODULES=( 'qqqq' ) and got
in "rear -d -D recover" log
++ echo -e 'Copying kernel modules'
++ for module in '"${MODULES[@]}"'
++ module=qqqq
++ module=qqqq
++ modinfo qqqq
++ continue
++ for module in '"${MODULES[@]}"'
++ module=nfs
++ module=nfs
++ modinfo nfs
+++ modprobe --ignore-install --set-version 4.4.21-69-default --show-depends nfs
i.e. the non-existent module qqqq is silently ignored
which is backward compatible - i.e. it matches the
behaviour in ResolveModules
for module in "$@"; do
...
# Check if the module actually exists
if ! modinfo $module &>/dev/null; then
continue
fi
jsmeix commented at 2017-05-12 13:30:¶
Now I did a more evil test (on my SLES12 system):
# modprobe --dry-run --show-depends lp insmod /lib/modules/4.4.21-69-default/kernel/drivers/parport/parport.ko insmod /lib/modules/4.4.21-69-default/kernel/drivers/char/lp.ko # mv /lib/modules/4.4.21-69-default/kernel/drivers/parport/parport.ko /lib/modules/4.4.21-69-default/kernel/drivers/parport/away_parport.ko # mv /lib/modules/4.4.21-69-default/kernel/drivers/parport/parport_pc.ko /lib/modules/4.4.21-69-default/kernel/drivers/parport/away_parport_pc.ko # modprobe --dry-run --show-depends lp modprobe: ERROR: ctx=0x22f0010 path=/lib/modules/4.4.21-69-default/kernel/drivers/parport/parport.ko error=No such file or directory modprobe: ERROR: ctx=0x22f0010 path=/lib/modules/4.4.21-69-default/kernel/drivers/parport/parport.ko error=No such file or directory insmod /lib/modules/4.4.21-69-default/kernel/drivers/char/lp.ko
I have set in local.conf
MODULES=( 'qqqq' 'lp' )
and did with the old code
# usr/sbin/rear -d -D mkrescue ... Copying binaries and libraries Copying kernel modules Omit copying files in /lib*/firmware/ (FIRMWARE_FILES='No')
No error about the missing 'parport' modules.
I got same bahavior with my new code
# usr/sbin/rear -d -D mkrescue ... Copying binaries and libraries Copying kernel modules Omit copying files in /lib*/firmware/ (FIRMWARE_FILES='No')
so that I am sufficiently backward compatible
but I wonder if such errors should better be detected
and at least reported to the user via LogPrint?
jsmeix commented at 2017-05-12 14:25:¶
@schlomo
regarding your above question
"Why do we need to support old distros seemingly forever?
IMHO we should be able to draw a line and say that old
distros stay on old ReaR versions."
In general see
https://github.com/rear/rear/issues/1173
I do fully agree and SLES10 is already "out of support", see
https://github.com/rear/rear/blob/master/doc/rear-release-notes.txt
at about line 1328:
Rear-2.00 dropped officially support for the following Linux based operating systems: o Fedora <22 o RHEL 3 and 4 o SLES 9 and 10 o openSUSE <11 o Debian <6 o Ubuntu <12
Unfortunately in this particular case I detected the crappy
old modinfo behaviour by "bad luck" while playing around
with the modules stuff also on my SLES10 system (just to
get some experience how things work in a traditional way)
but since I knew that misbehaviour I could not "just ignore" it
so that I tried to make code that should behave reasonably
even for the crappy old modinfo because I do not want
to break SLES10 support knowingly as long as I can keep
it working even on SLES10 (with reasonable effort) and
perhaps also on whatever other "elderly" systems
that may also have that crappy modinfo and
might be even still "supported" by ReaR
(I don't know what there is on RHEL 5 or CentOS 5).
Next time I will not play around on SLES10 ;-)
jsmeix commented at 2017-05-15 14:11:¶
With
https://github.com/rear/rear/pull/1365
merged
stderr is no longer needlessly redirected to /dev/null
see
https://github.com/rear/rear/pull/1359#discussion_r116239195
In particular non-existent modules are now at least reported in the
log
cf.
https://github.com/rear/rear/pull/1359#issuecomment-301071974
On the other hand useless stderr warnings from 'cp'
are avoided by removing duplicate module files
in the 'modprobe --show-depends' output, cf.
http://blog.schlomo.schapiro.org/2015/04/warning-is-waste-of-my-time.html
[Export of Github issue for rear/rear.]