[systemd-devel] [PATCH] udev: don't create symlink or rename if it already exists

2013-02-12 Thread Robert Milasan
Under some circumstances udev mixed with multipath fails:

udevd-work[1376]:
symlink(../../sdk, 
/dev/disk/by-id/scsi-36005076305ffc0672842.udev-tmp)
failed: File exists udevd-work[1432]:
rename(/dev/disk/by-id/scsi-36005076305ffc0850a88.udev-tmp, 
/dev/disk/by-id/scsi-36005076305ffc0850a88)
failed: No such file or directory

This is non-fatal, but there is no point of created the symlink or
renaming the symlink if it already exists.

Reference: https://bugzilla.novell.com/show_bug.cgi?id=791503

---
 src/udev/udev-node.c |   14 ++
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c
index bce4cfe..a92d727 100644
--- a/src/udev/udev-node.c
+++ b/src/udev/udev-node.c
@@ -107,6 +107,10 @@ static int node_symlink(struct udev *udev, const
char *node, const char *slink) err = mkdir_parents_label(slink_tmp,
0755); if (err != 0  err != -ENOENT)
 break;
+if (lstat(slink_tmp, stats) == 0) {
+err = 0;
+break;
+}
 label_context_set(slink_tmp, S_IFLNK);
 err = symlink(target, slink_tmp);
 if (err != 0)
@@ -117,10 +121,12 @@ static int node_symlink(struct udev *udev, const
char *node, const char *slink) log_error(symlink '%s' '%s' failed:
%m\n, target, slink_tmp); goto exit;
 }
-err = rename(slink_tmp, slink);
-if (err != 0) {
-log_error(rename '%s' '%s' failed: %m\n, slink_tmp,
slink);
-unlink(slink_tmp);
+if (lstat(slink_tmp, stats) == 0) {
+err = rename(slink_tmp, slink);
+if (err != 0) {
+log_error(rename '%s' '%s' failed: %m\n,
slink_tmp, slink);
+unlink(slink_tmp);
+}
 }
 exit:
 return err;
-- 
1.7.7

-- 
Robert Milasan

L3 Support Engineer
SUSE Linux (http://www.suse.com)
email: rmila...@suse.com
GPG fingerprint: B6FE F4A8 0FA3 3040 3402  6FE7 2F64 167C 1909 6D1A
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] udev: don't create symlink or rename if it already exists

2013-02-12 Thread Kay Sievers
On Tue, Feb 12, 2013 at 2:18 PM, Robert Milasan rmila...@suse.com wrote:
 Under some circumstances udev mixed with multipath fails:

 udevd-work[1376]:
 symlink(../../sdk, 
 /dev/disk/by-id/scsi-36005076305ffc0672842.udev-tmp)
 failed: File exists udevd-work[1432]:
 rename(/dev/disk/by-id/scsi-36005076305ffc0850a88.udev-tmp, 
 /dev/disk/by-id/scsi-36005076305ffc0850a88)
 failed: No such file or directory

 This is non-fatal, but there is no point of created the symlink or
 renaming the symlink if it already exists.

 Reference: https://bugzilla.novell.com/show_bug.cgi?id=791503

There is always a window between the check and the action, this patch
makes it only smaller, but it can still happen.

We have to accept some of the races in hotplug setups and devices
fighting about the same resources/names, some of these problems cannot
be easily solved, but in this case it is a race for a name we created
on our own, and which can be fixed without leaving any race window
open.

We should probably include the device dev_t (like
udev_device_get_id_filename()) or worker PID or something else in the
temporary name to make it unique, or just don't print an error when
the temporary file is gone before the renaming.

Adding racy checks for an existing temp file name does not seem like
the right thing to do.

Btw, you should use a different mailer for sending patches, or attach
them; this seems all mangled and would not apply.

Kay
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] udev: don't create symlink or rename if it already exists

2013-02-12 Thread Robert Milasan
On Tue, 12 Feb 2013 14:57:28 +0100
Kay Sievers k...@vrfy.org wrote:

 
 Btw, you should use a different mailer for sending patches, or attach
 them; this seems all mangled and would not apply.
 
 Kay
 

OK, as you wish and yes I noticed only after I sent the patch :)

-- 
Robert Milasan

L3 Support Engineer
SUSE Linux (http://www.suse.com)
email: rmila...@suse.com
GPG fingerprint: B6FE F4A8 0FA3 3040 3402  6FE7 2F64 167C 1909 6D1A
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] udev: don't create symlink or rename if it already exists

2013-02-12 Thread Kay Sievers
On Tue, Feb 12, 2013 at 3:07 PM, Robert Milasan rmila...@suse.com wrote:
 On Tue, 12 Feb 2013 14:57:28 +0100
 Kay Sievers k...@vrfy.org wrote:

 Btw, you should use a different mailer for sending patches, or attach
 them; this seems all mangled and would not apply.

 OK, as you wish and yes I noticed only after I sent the patch :)

My wish? Will you update the patch, or should I do it? :)

Kay
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] udev: don't create symlink or rename if it already exists

2013-02-12 Thread Robert Milasan
On Tue, 12 Feb 2013 15:09:18 +0100
Kay Sievers k...@vrfy.org wrote:

 On Tue, Feb 12, 2013 at 3:07 PM, Robert Milasan rmila...@suse.com
 wrote:
  On Tue, 12 Feb 2013 14:57:28 +0100
  Kay Sievers k...@vrfy.org wrote:
 
  Btw, you should use a different mailer for sending patches, or
  attach them; this seems all mangled and would not apply.
 
  OK, as you wish and yes I noticed only after I sent the patch :)
 
 My wish? Will you update the patch, or should I do it? :)
 
 Kay
 

Sorry, was referring to patch, it looks good to me, but I'm not really
a developer, so it's up to you to change it.

-- 
Robert Milasan

L3 Support Engineer
SUSE Linux (http://www.suse.com)
email: rmila...@suse.com
GPG fingerprint: B6FE F4A8 0FA3 3040 3402  6FE7 2F64 167C 1909 6D1A
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel