[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Weird problem with RHEL5 isofs driver when using mkisofs -x --exclude options



Giulio Orsero <giulioo@gmail.com> wrote:

> isoinfo output was always right (at least as for permissions and timestamp
> which I was interested in).

isoinfo displayed wrong timestamps for "." and ".." with the old mkisofs clone 
that comes with RedHat. 

isoinfo still displayes wrong timestamps for "." even with a recent mkisofs.


> The incorrect permissions/timestamp were from "ls -l" on the iso image as
> mounted on linux using Linux isofs driver.
>
> That very same iso image, when mounted on Linux, would:
> - show uncorrect perms/timestamp if mounted on RHEL5
> - show correct perms/timestamp if mounted on RHEL3

The incorrect perms are a result of the bugs in the mkisofs version that comes 
with Redhat.

The fact that you see different timestamps is a result of the fact that ISO-9660
stores different meta data for dir/ and dir/. 
If mkisofs does not make sure that the related meta data is identical, you see 
the known issues.

> If I produce the iso image using "-find"; then Linux isofs RHEL5 driver will
> show correct perms/timestamp too.

see above

> My problem is that I have a backup script that when run on RHEL5 will
> produce iso image which RHEL5 will read in a wrong way, unless it seems I
> change the backup script to use "-find".
>
> What is weird for me is:
> 1) Why would RHEL5 isofs driver be confused by an iso image produced by
> mkisofs with the "-x" flag, even when the -x flag should have no effect
> since, as per my example, would exclude something is not there.
> As soon as I take the -x option out, RHEL5 will read the iso image
> correctly.
> I'd think mkisofs would produce the very same output if I tell it to exclude
> something that is not there, basically a noop.

They did change the filesystem implementation in the kernel.

> 2) Switching to "mkisofs -find" seems to fix the issue, but I don't
> understand why.

because mkisofs then stores the same meta data for dir/ and dir/. and you see 
the same values regardless of which values are taken by the FS in the kernel.

In general, your problem suffers from two reasons:

1)	The filesystem does not return ".". and ".." first with readdir()

2)	The deprecated -x option incorrectly excludes "." and ".."

Please check again with the following patch, it should then work even without 
-find:

------- tree.c -------
--- /tmp/sccs.I7aWQs	Mi Jul  9 14:54:29 2008
+++ tree.c	Mi Jul  9 14:53:55 2008
@@ -1187,7 +1187,7 @@
 		d_entry = readdir(current_dir);
 	}
 
-	if (!current_dir || !d_entry) {
+	if (!current_dir || (!d_entry && errno != 0)) {
 		int	ret = 1;
 
 		errmsg("Unable to open directory %s\n", path);
@@ -1234,20 +1234,39 @@
 	/*
 	 * Now we scan the directory itself, and look at what is inside of it.
 	 */
-	dflag = 0;
+	whole_path[0] = '\0';
+	dflag = -2;
 	while (1 == 1) {
+		char	*d_name;
 
-		/*
-		 * The first time through, skip this, since we already asked
-		 * for the first entry when we opened the directory.
-		 */
-		if (dflag)
-			d_entry = readdir(current_dir);
-		dflag++;
+		if (dflag < 0) {
+			/*
+			 * Some filesystems do not deliver "." and ".." at all,
+			 * others (on Linux) deliver them in the wrong order.
+			 * Make sure we add "." and ".." before all other
+			 * entries.
+			 */
+			if (dflag < -1)
+				d_name = ".";
+			else
+				d_name = "..";
+			dflag++;
+		} else {
+			/*
+			 * The first time through, skip this, since we already
+			 * asked for the first entry when we opened the
+			 * directory.
+			 */
+			if (dflag > 0) {
+				d_entry = readdir(current_dir);
+			} else {
+				dflag++;
+			}
+			if (!d_entry)
+				break;
+			d_name = d_entry->d_name;
+		}
 
-		if (!d_entry)
-			break;
-
 		/*
 		 * OK, got a valid entry
 		 *
@@ -1254,13 +1273,13 @@
 		 * If we do not want all files, then pitch the backups.
 		 */
 		if (!all_files) {
-			if (strchr(d_entry->d_name, '~') ||
-			    strchr(d_entry->d_name, '#') ||
-			    rstr(d_entry->d_name, ".bak")) {
+			if (strchr(d_name, '~') ||
+			    strchr(d_name, '#') ||
+			    rstr(d_name, ".bak")) {
 				if (verbose > 0) {
 					fprintf(stderr,
 						"Ignoring file %s\n",
-						d_entry->d_name);
+						d_name);
 				}
 				continue;
 			}
@@ -1271,15 +1290,14 @@
 			 * exclude certain HFS type files/directories for the
 			 * time being
 			 */
-			if (hfs_exclude(d_entry->d_name))
+			if (hfs_exclude(d_name))
 				continue;
 		}
 #endif	/* APPLE_HYB */
 
-		if (strlen(path) + strlen(d_entry->d_name) + 2 >
-							sizeof (whole_path)) {
+		if (strlen(path) + strlen(d_name) + 2 > sizeof (whole_path)) {
 			errmsgno(EX_BAD, "Path name %s/%s too long.\n",
-					path, d_entry->d_name);
+					path, d_name);
 			comerrno(EX_BAD, "Overflow of stat buffer\n");
 		};
 
@@ -1291,12 +1309,15 @@
 		if (whole_path[strlen(whole_path) - 1] != '/')
 			strcat(whole_path, "/");
 #endif
-		strcat(whole_path, d_entry->d_name);
+		strcat(whole_path, d_name);
 
 		/*
 		 * Should we exclude this file ?
+		 * Do no check "." and ".."
 		 */
-		if (matches(d_entry->d_name) || matches(whole_path)) {
+		if (!(d_name[0] == '.' && (d_name[1] == '\0' ||
+		    (d_name[1] == '.' && d_name[2] == '\0'))) &&
+		    (matches(d_name) || matches(whole_path))) {
 			if (verbose > 1) {
 				fprintf(stderr,
 					"Excluded by match: %s\n", whole_path);
@@ -1304,7 +1325,7 @@
 			continue;
 		}
 		if (generate_tables &&
-		    strcmp(d_entry->d_name, trans_tbl) == 0) {
+		    strcmp(d_name, trans_tbl) == 0) {
 			/*
 			 * Ignore this entry.  We are going to be generating
 			 * new versions of these files, and we need to ignore
@@ -1319,11 +1340,11 @@
 		 * If we already have a '.' or a '..' entry, then don't insert
 		 * new ones.
 		 */
-		if (strcmp(d_entry->d_name, ".") == 0 &&
+		if (strcmp(d_name, ".") == 0 &&
 		    this_dir->dir_flags & DIR_HAS_DOT) {
 			continue;
 		}
-		if (strcmp(d_entry->d_name, "..") == 0 &&
+		if (strcmp(d_name, "..") == 0 &&
 		    this_dir->dir_flags & DIR_HAS_DOTDOT) {
 			continue;
 		}
@@ -1335,9 +1356,9 @@
 		 * This actually adds the entry to the directory in question.
 		 */
 #ifdef APPLE_HYB
-		insert_file_entry(this_dir, whole_path, d_entry->d_name, NULL, 0);
+		insert_file_entry(this_dir, whole_path, d_name, NULL, 0);
 #else
-		insert_file_entry(this_dir, whole_path, d_entry->d_name, NULL);
+		insert_file_entry(this_dir, whole_path, d_name, NULL);
 #endif	/* APPLE_HYB */
 	}
 	closedir(current_dir);


Jörg

-- 
 EMail:joerg@schily.isdn.cs.tu-berlin.de (home) Jörg Schilling D-13353 Berlin
       js@cs.tu-berlin.de                (uni)  
       schilling@fokus.fraunhofer.de     (work) Blog: http://schily.blogspot.com/
 URL:  http://cdrecord.berlios.de/old/private/ ftp://ftp.berlios.de/pub/schily


Reply to: