Bug 623774 - rhncfg-client: Creation of symbolic links fail on non-root partition
rhncfg-client: Creation of symbolic links fail on non-root partition
Status: CLOSED WORKSFORME
Product: Spacewalk
Classification: Community
Component: Clients (Show other bugs)
1.0
All Linux
low Severity medium
: ---
: ---
Assigned To: Milan Zázrivec
Red Hat Satellite QA List
:
Depends On:
Blocks: space16
  Show dependency treegraph
 
Reported: 2010-08-12 14:20 EDT by Ryan Falkenberg
Modified: 2012-11-19 10:34 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-08-24 08:14:28 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Ryan Falkenberg 2010-08-12 14:20:19 EDT
Description of problem:
Deploying a symbolic link to a spacewalk-managed system fails if the symlink location is not on the root (/) filesystem

Version-Release number of selected component (if applicable):
rhncfg-5.9.19-1.el5.noarch


How reproducible:
Always

Steps to Reproduce:
1. Add a locally managed symbolic link to a spacewalk managed system, set the symbolic link location to a filesystem other than / (I am using /opt), the target of the symlink does not matter
2. deploy the symlink to the system
  
Actual results:
Deployment fails with:
Client execution returned "Failed deployment, rolled back: [Errno 18] Invalid cross-device link" (code 49)

Expected results:
Symlink to be properly deployed


Additional info:
- Deploying a symbolic link to a directory on the root (/) file system works

- I've traced the problem back to an os.rename in /usr/share/rhn/config_common/transactions.py and attached a prof of concept fix.  Calling `mv` is a hack and probably fragile but hopefully it will help someone with better python skills than me write a better solution

--- /usr/share/rhn/config_common/transactions.py.orig   2010-08-11 17:04:33.000000000 -0600
+++ /usr/share/rhn/config_common/transactions.py        2010-08-12 10:51:13.000000000 -0600
@@ -350,7 +351,21 @@
                     self.deployment_cb(path)

                 log_debug(6, "deploying %s ..." % path)
-                os.rename(self.newtemp_by_path[path], path)
+                # os.renames will fail if the path and the new_path are on different partitions
+                # need to make sure to handle it if we catch a 'OSError: [Errno 18] Invalid cross-device link'
+                try:
+                    log_debug(9, "trying to use os.rename to move the temp file into place")
+                    os.rename(self.newtemp_by_path[path], path)
+                except OSError, e:
+                    if e.errno == 18:
+                        #log_debug(9, "os.rename failed, using shutil functions")
+                        #shutil.copy(self.newtemp_by_path[path], path)
+                        # This is kind of hacky but i don't know a better way that properly handles dangling symlinks
+                        log_debug(9, "os.rename failed, using the system `mv` command to copy %s to %s" % (self.newtemp_by_path[path], path))
+                        os.system("mv %s %s" % (self.newtemp_by_path[path], path))
+                    else:
+                        raise
                 # race
                 del self.newtemp_by_path[path]
                 log_debug(9, "new version of %s deployed" % path)



- In my testing this fix works regardless of where the symbolic link points to (directory, file, or dangling symlink).
- With this fix symlink deployments will fail if the destination directory doesn't exist (i.e. deploying /nonresistant/directory/symlink will fail).  And deploying the same symlink twice will fail if the symlink target is a directory
Comment 1 Ryan Falkenberg 2010-08-12 14:23:37 EDT
/usr/share/rhn/config_common/transactions.py needs to be patched on the client systems, not the spacewalk server
Comment 2 Jan Pazdziora 2010-09-01 08:16:37 EDT
(In reply to comment #0)
> +                        os.system("mv %s %s" % (self.newtemp_by_path[path],
> path))

I don't like forking the mv, not mentioning the fact that the comemnt will fail if the path is strange (starts with a dash or has a space in it or something).

Why can't we create the temporary entries in the same directory as the final location, with some dot at start and some random suffix?
Comment 3 Jan Pazdziora 2010-11-19 11:03:26 EST
Mass-moving to space13.
Comment 4 Miroslav Suchý 2011-04-11 03:32:09 EDT
We did not have time for this one during Spacewalk 1.4 time frame. Mass moving to Spacewalk 1.5.
Comment 5 Miroslav Suchý 2011-04-11 03:36:37 EDT
We did not have time for this one during Spacewalk 1.4 time frame. Mass moving to Spacewalk 1.5.
Comment 6 Jan Pazdziora 2011-07-20 07:49:56 EDT
Aligning under space16.
Comment 7 Michael Mráka 2011-08-24 08:14:28 EDT
I was not able to reproduce this issue with the latest rhncfg-client.

#  rhncfg-client get
Deploying //mnt/dir
Deploying /mnt/dir/link
Deploying /mnt/dir/file
Deploying /mnt/file
# ll /mnt/dir/
-rw-r--r-- 1 root root 6 Aug 24 13:53 file
lrwxrwxrwx 1 root root 9 Aug 24 13:53 link -> /bin/bash
# df /bin /mnt
Filesystem           1K-blocks      Used Available Use% Mounted on
/dev/xvda1            40629568   1870084  36662856   5% /
/root/filesys.img        39663      4610     33005  13% /mnt
# rpm -q rhncfg-client
rhncfg-client-5.10.13-1.el6.noarch

So I'm closing it as WORKSFORME.

If you disagree please reopen and specify exact version of rhncfg-client.

Note You need to log in before you can comment on or make changes to this bug.