Device majors incorrectly set to 0 during rsync

Wayne Davison wayned at samba.org
Fri Apr 9 21:45:44 GMT 2004


On Thu, Apr 08, 2004 at 02:42:10PM -0700, Wayne Davison wrote:
> To fix this will require a protocol change.

Since the 2.6.1 version that will be coming out soon has several
protocol improvements in the new protocol-28 (including one relating to
rdev-sending), it seems like this would be a good time to get rsync to
send devices using the separated major and minor values (even though it
would delay the release a little).

The current code depends on the ability to just cast a dev_t into a
uint64, but then send only the lower 32-bits of this value and have
that work.  Even if the code were fixed to send all 64-bits, an rsync
compiled as a 64-bit binary that sends data to an rsync compiled as a
32-bit binary might not work properly, even if the OS was identical on
both ends of the connection (this happens if the binaries have a
different internal representation for the dev_t value, as they appear
to have on Solaris, at least).

The attached patch implements the sending of separate major/minor device
numbers.  I've recently checked-in some configure changes(*) that ensure
that the macros for major(), minor(), and makedev() are available, so
that's already taken care of.

!! Note that the resulting version of rsync will not interoperate with
earlier (CVS/pre-release) protocol-28 versions of rsync IF (and only if)
you use the --devices and/or --hard-links options! !!  It should work
fine with all RELEASED versions of rsync, of course.

If anyone wants to try it out, the patch should be applied to the CVS
version, which is also available in this "nightly" tar file:

http://rsync.samba.org/ftp/rsync/nightly/rsync-HEAD-20040409-1343PDT.tar.gz

Comments welcomed.  Anyone testing this, please let me know.  Thanks.

..wayne..

* FYI, the aforementioned configure change was done so that I could fix
the tls.c test program to make it output the major/minor device info --
without this, the regression test for device copying wasn't even testing
that the device numbers had been sent correctly.
-------------- next part --------------
--- flist.c	8 Apr 2004 23:15:39 -0000	1.209
+++ flist.c	9 Apr 2004 20:22:52 -0000
@@ -324,8 +324,8 @@ void send_file_entry(struct file_struct 
 	unsigned short flags;
 	static time_t modtime;
 	static mode_t mode;
-	static DEV64_T rdev, rdev_high;
-	static DEV64_T dev;
+	static dev_t dev, rdev;
+	static uint32 rdev_major;
 	static uid_t uid;
 	static gid_t gid;
 	static char lastname[MAXPATHLEN];
@@ -338,7 +338,8 @@ void send_file_entry(struct file_struct 
 	if (!file) {
 		write_byte(f, 0);
 		modtime = 0, mode = 0;
-		rdev = 0, rdev_high = 0, dev = 0;
+		dev = rdev = makedev(0, 0);
+		rdev_major = 0;
 		uid = 0, gid = 0;
 		*lastname = '\0';
 		return;
@@ -357,21 +358,20 @@ void send_file_entry(struct file_struct 
 	if (preserve_devices) {
 		if (protocol_version < 28) {
 			if (IS_DEVICE(mode)) {
-				if (file->u.rdev == rdev) {
-					/* Set both flags to simplify the test
-					 * when writing the data. */
-					flags |= XMIT_SAME_RDEV_pre28
-					    | XMIT_SAME_HIGH_RDEV;
-				} else
+				if (file->u.rdev == rdev)
+					flags |= XMIT_SAME_RDEV_pre28;
+				else
 					rdev = file->u.rdev;
 			} else
-				rdev = 0;
+				rdev = makedev(0, 0);
 		} else if (IS_DEVICE(mode)) {
 			rdev = file->u.rdev;
-			if ((rdev & ~(DEV64_T)0xFF) == rdev_high)
-				flags |= XMIT_SAME_HIGH_RDEV;
+			if ((uint32)major(rdev) == rdev_major)
+				flags |= XMIT_SAME_RDEV_MAJOR;
 			else
-				rdev_high = rdev & ~(DEV64_T)0xFF;
+				rdev_major = major(rdev);
+			if ((uint32)minor(rdev) <= 0xFFu)
+				flags |= XMIT_RDEV_MINOR_IS_SMALL;
 		}
 	}
 	if (file->uid == uid)
@@ -452,12 +452,17 @@ void send_file_entry(struct file_struct 
 		write_int(f, gid);
 	}
 	if (preserve_devices && IS_DEVICE(mode)) {
-		/* If XMIT_SAME_HIGH_RDEV is off, XMIT_SAME_RDEV_pre28 is
-		 * also off. */
-		if (!(flags & XMIT_SAME_HIGH_RDEV))
-			write_int(f, rdev);
-		else if (protocol_version >= 28)
-			write_byte(f, rdev);
+		if (protocol_version < 28) {
+			if (!(flags & XMIT_SAME_RDEV_pre28))
+				write_int(f, (int)rdev);
+		} else {
+			if (!(flags & XMIT_SAME_RDEV_MAJOR))
+				write_int(f, major(rdev));
+			if (flags & XMIT_RDEV_MINOR_IS_SMALL)
+				write_byte(f, minor(rdev));
+			else
+				write_int(f, minor(rdev));
+		}
 	}
 
 #if SUPPORT_LINKS
@@ -470,14 +475,20 @@ void send_file_entry(struct file_struct 
 
 #if SUPPORT_HARD_LINKS
 	if (flags & XMIT_HAS_IDEV_DATA) {
-		if (protocol_version < 26) {
+		if (protocol_version >= 28) {
+			/* major/minor dev_t and 64-bit ino_t */
+			if (!(flags & XMIT_SAME_DEV)) {
+				write_int(f, major(dev));
+				write_int(f, minor(dev));
+			}
+			write_longint(f, file->F_INODE);
+		} else if (protocol_version < 26) {
 			/* 32-bit dev_t and ino_t */
 			write_int(f, dev);
 			write_int(f, file->F_INODE);
 		} else {
 			/* 64-bit dev_t and ino_t */
-			if (!(flags & XMIT_SAME_DEV))
-				write_longint(f, dev);
+			write_longint(f, dev);
 			write_longint(f, file->F_INODE);
 		}
 	}
@@ -510,8 +521,8 @@ void receive_file_entry(struct file_stru
 {
 	static time_t modtime;
 	static mode_t mode;
-	static DEV64_T rdev, rdev_high;
-	static DEV64_T dev;
+	static dev_t dev, rdev;
+	static uint32 rdev_major;
 	static uid_t uid;
 	static gid_t gid;
 	static char lastname[MAXPATHLEN], *lastdir;
@@ -525,7 +536,8 @@ void receive_file_entry(struct file_stru
 
 	if (!fptr) {
 		modtime = 0, mode = 0;
-		rdev = 0, rdev_high = 0, dev = 0;
+		dev = rdev = makedev(0, 0);
+		rdev_major = 0;
 		uid = 0, gid = 0;
 		*lastname = '\0';
 		return;
@@ -587,15 +599,18 @@ void receive_file_entry(struct file_stru
 		if (protocol_version < 28) {
 			if (IS_DEVICE(mode)) {
 				if (!(flags & XMIT_SAME_RDEV_pre28))
-					rdev = (DEV64_T)read_int(f);
+					rdev = (dev_t)read_int(f);
 			} else
-				rdev = 0;
+				rdev = makedev(0, 0);
 		} else if (IS_DEVICE(mode)) {
-			if (!(flags & XMIT_SAME_HIGH_RDEV)) {
-				rdev = (DEV64_T)read_int(f);
-				rdev_high = rdev & ~(DEV64_T)0xFF;
-			} else
-				rdev = rdev_high | (DEV64_T)read_byte(f);
+			uint32 rdev_minor;
+			if (!(flags & XMIT_SAME_RDEV_MAJOR))
+				rdev_major = read_int(f);
+			if (flags & XMIT_RDEV_MINOR_IS_SMALL)
+				rdev_minor = read_byte(f);
+			else
+				rdev_minor = read_int(f);
+			rdev = makedev(rdev_major, rdev_minor);
 		}
 	}
 
@@ -660,12 +675,18 @@ void receive_file_entry(struct file_stru
 		flags |= XMIT_HAS_IDEV_DATA;
 	if (flags & XMIT_HAS_IDEV_DATA) {
 		INO64_T inode;
-		if (protocol_version < 26) {
-			dev = read_int(f);
+		if (protocol_version >= 28) {
+			if (!(flags & XMIT_SAME_DEV)) {
+				uint32 dev_major = read_int(f);
+				uint32 dev_minor = read_int(f);
+				dev = makedev(dev_major, dev_minor);
+			}
+			inode = read_longint(f);
+		} else if (protocol_version < 26) {
+			dev = (dev_t)read_int(f);
 			inode = read_int(f);
 		} else {
-			if (!(flags & XMIT_SAME_DEV))
-				dev = read_longint(f);
+			dev = (dev_t)read_longint(f);
 			inode = read_longint(f);
 		}
 		if (flist->hlink_pool) {
--- generator.c	7 Mar 2004 20:29:59 -0000	1.77
+++ generator.c	9 Apr 2004 20:22:52 -0000
@@ -378,7 +378,7 @@ void recv_generator(char *fname, struct 
 	if (am_root && preserve_devices && IS_DEVICE(file->mode)) {
 		if (statret != 0 ||
 		    st.st_mode != file->mode ||
-		    (DEV64_T)st.st_rdev != file->u.rdev) {
+		    st.st_rdev != file->u.rdev) {
 			delete_file(fname);
 			if (verbose > 2)
 				rprintf(FINFO,"mknod(%s,0%o,0x%x)\n",
--- rsync.h	9 Apr 2004 19:04:03 -0000	1.191
+++ rsync.h	9 Apr 2004 20:22:52 -0000
@@ -50,9 +50,10 @@
 #define XMIT_SAME_NAME (1<<5)
 #define XMIT_LONG_NAME (1<<6)
 #define XMIT_SAME_TIME (1<<7)
-#define XMIT_SAME_HIGH_RDEV (1<<8)
+#define XMIT_SAME_RDEV_MAJOR (1<<8)
 #define XMIT_HAS_IDEV_DATA (1<<9)
 #define XMIT_SAME_DEV (1<<10)
+#define XMIT_RDEV_MINOR_IS_SMALL (1<<11)
 
 /* These flags are used in the live flist data. */
 
@@ -345,17 +346,11 @@ enum msgcode {
  * device numbers will be truncated.  But it's a kind of silly thing
  * to do anyhow.
  *
- * FIXME: In future, we should probable split the device number into
- * major/minor, and transfer the two parts as 32-bit ints.  That gives
- * you somewhat more of a chance that they'll come from a big machine
- * to a little one in a useful way.
- *
  * FIXME: Really we need an unsigned type, and we perhaps ought to
  * cope with platforms on which this is an unsigned int or even a
  * struct.  Later.
  */ 
 #define INO64_T uint64
-#define DEV64_T uint64
 
 #ifndef MIN
 #define MIN(a,b) ((a)<(b)?(a):(b))
@@ -403,7 +398,7 @@ struct hlink {
 
 struct idev {
 	INO64_T inode;
-	DEV64_T dev;
+	dev_t dev;
 };
 
 #define F_DEV	link_u.idev->dev
@@ -414,9 +409,9 @@ struct idev {
 
 struct file_struct {
 	union {
-		DEV64_T rdev;	/* The device number, if this is a device */
+		dev_t rdev;	/* The device number, if this is a device */
 		char *sum;	/* Only a normal file can have a checksum */
-		char *link;	/* Holds symlink string, if a symlink */
+		char *link;	/* Points to symlink string, if a symlink */
 	} u;
 	OFF_T length;
 	char *basename;


More information about the rsync mailing list