Re: [Glibc-bsd-commits] r3162 - trunk/zfsutils/debian/patches
On 8/23/10, Aurelien Jarno aurel...@aurel32.net wrote: This makes zpool import work. The real problem here is NOT that /dev/ is not searched for. On FreeBSD scan is usually performed by geom_find_import(), and /dev is not searched unless that fails. The problem was that GNU realpath() fails for inexistant paths and BSD realpath() doesn't. Given the explanation you are giving there, I am not sure it is the correct fix. Given GNU realpath() behave differently than the BSD realpath() used in the original code, how about fixing the code that call realpath() to behave correctly with the GNU version? I'm sorry, this bug is a bit weird, I will try to explain better. This is what original code in zpool is doing: 1. Resolve /dev/dsk/ using realpath(). If realpath() reports that /dev/dsk/ does not exist, it aborts here. 2. Access the device using GEOM, instead of /dev/ nodes. If this suceeds, it ends here. I think this part was added by FreeBSD. 3. If GEOM failed, use the resolved path from step 1 to access the device with open()/read()/etc. /dev/dsk/ doesn't exist on FreeBSD, it's a Solaris directory. However, this is harmless because BSD realpath() is buggy and reports that the path has been correctly resolved. Then GEOM does the real job, so they don't notice. However with GNU realpath() zpool aborts in step 1. -- To UNSUBSCRIBE, email to debian-bsd-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/aanlktin7sxwvr1=-bnwaaqq5q1vb0ukdx6npfhevr...@mail.gmail.com
Re: [Glibc-bsd-commits] r3162 - trunk/zfsutils/debian/patches
On Tue, Aug 24, 2010 at 03:06:26PM -0400, Tuco wrote: On 8/23/10, Aurelien Jarno aurel...@aurel32.net wrote: This makes zpool import work. The real problem here is NOT that /dev/ is not searched for. On FreeBSD scan is usually performed by geom_find_import(), and /dev is not searched unless that fails. The problem was that GNU realpath() fails for inexistant paths and BSD realpath() doesn't. Given the explanation you are giving there, I am not sure it is the correct fix. Given GNU realpath() behave differently than the BSD realpath() used in the original code, how about fixing the code that call realpath() to behave correctly with the GNU version? I'm sorry, this bug is a bit weird, I will try to explain better. This is what original code in zpool is doing: 1. Resolve /dev/dsk/ using realpath(). If realpath() reports that /dev/dsk/ does not exist, it aborts here. 2. Access the device using GEOM, instead of /dev/ nodes. If this suceeds, it ends here. I think this part was added by FreeBSD. 3. If GEOM failed, use the resolved path from step 1 to access the device with open()/read()/etc. /dev/dsk/ doesn't exist on FreeBSD, it's a Solaris directory. However, this is harmless because BSD realpath() is buggy and reports that the path has been correctly resolved. Then GEOM does the real job, so they don't notice. However with GNU realpath() zpool aborts in step 1. What worry me is that /dev/dsk is the default path, but the user can specify another one through some options. Is it going to work in that case? Also what happens if it fails opening the device through GEOM? Is it safe? -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net -- To UNSUBSCRIBE, email to debian-bsd-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20100824191546.gr19...@hall.aurel32.net
Re: [Glibc-bsd-commits] r3162 - trunk/zfsutils/debian/patches
On 8/24/10, Aurelien Jarno aurel...@aurel32.net wrote: What worry me is that /dev/dsk is the default path, but the user can specify another one through some options. Is it going to work in that case? Yes, this already works without 10_dev_dsk.diff. Also what happens if it fails opening the device through GEOM? Without my patch it would try to use /dev/dsk/ (unless -d was used) and fail. With my patch it's supposed to work, but this hasn't been tested even on FreeBSD so I don't know. -- To UNSUBSCRIBE, email to debian-bsd-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/aanlktikg66mycsyrmksg2nzmwfrctguwnx8w9qhyo...@mail.gmail.com
Re: [Glibc-bsd-commits] r3162 - trunk/zfsutils/debian/patches
Hi, On Fri, Aug 13, 2010 at 10:46:46PM +, Tuco Xyz wrote: Author: tuco-guest Date: 2010-08-13 22:46:46 + (Fri, 13 Aug 2010) New Revision: 3162 Added: trunk/zfsutils/debian/patches/10_dev_dsk.diff Modified: trunk/zfsutils/debian/patches/series Log: This makes zpool import work. The real problem here is NOT that /dev/ is not searched for. On FreeBSD scan is usually performed by geom_find_import(), and /dev is not searched unless that fails. The problem was that GNU realpath() fails for inexistant paths and BSD realpath() doesn't. Given the explanation you are giving there, I am not sure it is the correct fix. Given GNU realpath() behave differently than the BSD realpath() used in the original code, how about fixing the code that call realpath() to behave correctly with the GNU version? Added: trunk/zfsutils/debian/patches/10_dev_dsk.diff === --- trunk/zfsutils/debian/patches/10_dev_dsk.diff (rev 0) +++ trunk/zfsutils/debian/patches/10_dev_dsk.diff 2010-08-13 22:46:46 UTC (rev 3162) @@ -0,0 +1,40 @@ +--- a/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c b/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c +@@ -1468,7 +1468,11 @@ + + if (searchdirs == NULL) { + searchdirs = safe_malloc(sizeof (char *)); ++#ifdef __sun + searchdirs[0] = /dev/dsk; ++#else ++searchdirs[0] = /dev; ++#endif + nsearch = 1; + } + +--- a/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_import.c b/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_import.c +@@ -848,7 +848,11 @@ + size_t pathleft; + struct stat64 statbuf; + nvlist_t *ret = NULL, *config; ++#ifdef __sun + static char *default_dir = /dev/dsk; ++#else ++static char *default_dir = /dev; ++#endif + int fd; + pool_list_t pools = { 0 }; + pool_entry_t *pe, *penext; +@@ -894,9 +898,11 @@ + * reading the labels skips a bunch of slow operations during + * close(2) processing, so we replace /dev/dsk with /dev/rdsk. + */ ++#ifdef __sun + if (strcmp(path, /dev/dsk/) == 0) + rdsk = /dev/rdsk/; + else ++#endif + rdsk = path; + + if ((dirp = opendir(rdsk)) == NULL) { Modified: trunk/zfsutils/debian/patches/series === --- trunk/zfsutils/debian/patches/series 2010-08-13 22:24:54 UTC (rev 3161) +++ trunk/zfsutils/debian/patches/series 2010-08-13 22:46:46 UTC (rev 3162) @@ -7,3 +7,4 @@ 07_manpages.diff 08_libbsd.diff 09_xdr_control.diff +10_dev_dsk.diff ___ Glibc-bsd-commits mailing list glibc-bsd-comm...@lists.alioth.debian.org http://lists.alioth.debian.org/mailman/listinfo/glibc-bsd-commits -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net -- To UNSUBSCRIBE, email to debian-bsd-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20100823231224.ga15...@hall.aurel32.net
Re: [Glibc-bsd-commits] r3162 - trunk/zfsutils/debian/patches
Aurelien Jarno dixit: Author: tuco-guest problem was that GNU realpath() fails for inexistant paths and BSD realpath() doesn't. AFAICT, it only doesn't fail if the pathname given may be created, i.e. if all but the last component exist (and there are no trailing slashes, per POSIX). bye, //mirabilos -- I believe no one can invent an algorithm. One just happens to hit upon it when God enlightens him. Or only God invents algorithms, we merely copy them. If you don't believe in God, just consider God as Nature if you won't deny existence. -- Coywolf Qi Hunt -- To UNSUBSCRIBE, email to debian-bsd-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/pine.bsm.4.64l.1008232332400.12...@herc.mirbsd.org