Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Follow symbolic link with COPY_AS_IS targets #1636

Closed
wants to merge 1 commit into from
Closed

Follow symbolic link with COPY_AS_IS targets #1636

wants to merge 1 commit into from

Conversation

gdha
Copy link
Member

@gdha gdha commented Dec 8, 2017

  • added the "-h" option to tar to follow the symbolic links
    instead of adding just the symbolic link to the tar archive
    We run into this issue with /etc/localtime (issue Timezone cannot be set due to missing file #1635)
  • commented out in skel/default/etc/scripts/dhcp-setup-functions.sh
    the localtime copy lines (as the /usr/share/zoneinfo is not
    present anyway in the rescue environment)

  instead of adding just the symbolic link to the tar archive
  We run into this issue with /etc/localtime (issue #1635)
- commented out in skel/default/etc/scripts/dhcp-setup-functions.sh
  the localtime copy lines (as the /usr/share/zoneinfo is not
  present anyway in the rescue environment)
@gdha gdha added the minor bug An alternative or workaround exists label Dec 8, 2017
@gdha gdha added this to the ReaR v2.3 milestone Dec 8, 2017
@gdha gdha self-assigned this Dec 8, 2017
@gdha
Copy link
Member Author

gdha commented Dec 8, 2017

I think this is serious enough to still push it into the release 2.3, agree?

@jsmeix
Copy link
Member

jsmeix commented Dec 8, 2017

As far as I see this is a backward incompatible change
because without '-h' tar copies symbolic links as symbolic links
so that there can be symbolic links in the recovery system
while with '-h' tar copies the files symbolic links point to
so that there can be no longer symbolic links in the recovery system
which might blow up the recovery system with several copies
of the same (possibly big) file where several symbolic links
point to on the original system.

I fear such a backward incompatible change will cause
unexpected and unwanted bad side effects in this or that
unforeseen special cases.

Couldn't the actual issue #1635
be solved with a specific solution only for that issue e.g. like

localtime_link_target="$( readlink -e /etc/localtime )"
COPY_AS_IS=( "${COPY_AS_IS[@]}" /etc/localtime "$localtime_link_target" )

FYI:

On my SLES11 system:

# ls -l /etc/localtime
-rw-r--r-- 3 root root 2335 Jun 17  2015 /etc/localtime

# readlink -e /etc/localtime
/etc/localtime

On my SLES12 system:

# ls -l /etc/localtime
lrwxrwxrwx 1 root root 35 Jul 28 10:40 /etc/localtime -> ../usr/share/zoneinfo/Europe/Berlin

# readlink -e /etc/localtime
/usr/share/zoneinfo/Europe/Berlin

Copy link
Member

@jsmeix jsmeix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have almost 2000 symbolic links in the recovery system:

# find /tmp/rear.n8XNBDuAB0fhlXn/rootfs -type l | wc -l
1873

I don't know how many of them were created via COPY_AS_IS
but I fear at least some are created via COPY_AS_IS
which would be no longer the case if this pull request was merged.

@jsmeix
Copy link
Member

jsmeix commented Dec 8, 2017

I even found some broken symbolic links in the recovery system:

# for l in $( find /tmp/rear.n8XNBDuAB0fhlXn/rootfs -xtype l ) ; do file $l ; done
/tmp/rear.n8XNBDuAB0fhlXn/rootfs/etc/localtime: broken symbolic link to `../usr/share/zoneinfo/Europe/Berlin'
/tmp/rear.n8XNBDuAB0fhlXn/rootfs/dev/device-mapper: broken symbolic link to `mapper/control'
/tmp/rear.n8XNBDuAB0fhlXn/rootfs/dev/char/10:236: broken symbolic link to `../mapper/control'

@jsmeix
Copy link
Member

jsmeix commented Dec 8, 2017

The change

--- a/usr/share/rear/build/GNU/Linux/100_copy_as_is.sh
+++ b/usr/share/rear/build/GNU/Linux/100_copy_as_is.sh

-if ! tar -v -X $copy_as_is_exclude_file -P -C / -c "${COPY_AS_IS[@]}" 2>$copy_as_is_filelist_file | tar $v -C $ROOTFS_DIR/ -x 1>/dev/null ; then
+if ! tar -v -X $copy_as_is_exclude_file -h -P -C / -c "${COPY_AS_IS[@]}" 2>$copy_as_is_filelist_file | tar $v -C $ROOTFS_DIR/ -x 1>/dev/null ; then

has horrible consequences on my SLES12 test system:

Now "usr/sbin/rear -D mkrescue" takes ages at
"Copying files and directories" so that I aborted it
and the recovery system size before was 208M
but with that change (and aborted "Copying files and directories")
it became 1.8G

@schlomo
Copy link
Member

schlomo commented Dec 8, 2017

I would like to caution against this change: IMHO symlinks should be copied as-is into the rescue system and not converted into files or directories. IMHO the danger to mess up the system is way too big and especially the danger to bloat up the rescue image (as @jsmeix observed). I think that in most cases the symlinks were put in place purposefully so that we should leave them as there were intended.

For the few known cases where this is a problem we should add extra code to handle it, e.g. /etc/localtime.

If you want to help with finding such places then I would suggest to add code that checks symlinks and reports broken symlinks, especially if they were not broken in the source system.

@jsmeix
Copy link
Member

jsmeix commented Dec 8, 2017

With the added '-h' it seems "rear mkrescue" even hangs up at
"Copying files and directories" because since several minutes
the ReaR log file stays at

# tail /root/rear.master/var/log/rear/rear-f48.log
...
++ tar -v -C /tmp/rear.9h061udK1yV7mTo/rootfs/ -x
++ tar -v -X /tmp/rear.9h061udK1yV7mTo/tmp/copy-as-is-exclude -h -P -C / -c /root/rear.master/usr/share/rear /root/rear.master/var/lib/rear /dev /etc/inputrc ...
tar: Removing leading `/' from member names
tar: Removing leading `/' from hard link targets

The recovery system does no longer grow noticeable
because since several minutes it stays at 1.8G

# du -hs /tmp/rear.9h061udK1yV7mTo/rootfs/
1.8G    /tmp/rear.9h061udK1yV7mTo/rootfs/

but when counting bytes one can see it still grows slowly:

# for i in 1 2 3 ; do date +%T ; du -bs /tmp/rear.9h061udK1yV7mTo/rootfs/ ; sleep 10 ; done
14:29:17
1739527435      /tmp/rear.9h061udK1yV7mTo/rootfs/
14:29:28
1739533555      /tmp/rear.9h061udK1yV7mTo/rootfs/
14:29:38
1739541339      /tmp/rear.9h061udK1yV7mTo/rootfs/

so that something is still creeping on...
...
...after more than half an hour I aborted it again
and got:

# date +%T ; du -bs /tmp/rear.9h061udK1yV7mTo/rootfs/
14:56:02
1740243069      /tmp/rear.9h061udK1yV7mTo/rootfs/

@gdha
Copy link
Member Author

gdha commented Dec 11, 2017

Good that I asked your opinion! As issue #1638 has been made to tackle this I will close this PR without merging. We can solve it differently.

@gdha gdha closed this Dec 11, 2017
jsmeix added a commit that referenced this pull request Oct 9, 2018
Added comment why symbolic links must be copied as symbolic links
(why 'tar -h' must not be used), see #1636
@jsmeix
Copy link
Member

jsmeix commented Oct 9, 2018

Via
5410104
I documented that symbolic links must be copied as symbolic links
(i.e. why 'tar -h' must not be used for COPY_AS_IS).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor bug An alternative or workaround exists won't fix / can't fix / obsolete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants