If I''m not mistaken, there are some new reserved pool names (raidz1, raidz2 and spare) that weren''t being properly checked for (in zfs_namecheck.c). There were also some errors that weren''t being handled in libzfs_dataset.c (they were detected by gcc''s warnings). -------------- next part -------------- A non-text attachment was scrubbed... Name: namecheck.diff Type: text/x-diff Size: 1721 bytes Desc: not available URL: <http://mail.opensolaris.org/pipermail/zfs-code/attachments/20060620/9bf8332f/attachment.bin>
Ricardo Correia wrote:> If I''m not mistaken, there are some new reserved pool names (raidz1, raidz2 > and spare) that weren''t being properly checked for (in zfs_namecheck.c). > > There were also some errors that weren''t being handled in libzfs_dataset.c > (they were detected by gcc''s warnings). >I just opened the following bug: 6441206 missing checks in zfs_namecheck() -Mark> > ------------------------------------------------------------------------ > > Index: lib/libzfs/libzfs_dataset.c > ==================================================================> --- lib/libzfs/libzfs_dataset.c (revision 35) > +++ lib/libzfs/libzfs_dataset.c (working copy) > @@ -153,6 +153,18 @@ > zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, > "multiple ''@'' delimiters in name")); > break; > + > + case NAME_ERR_NOLETTER: > + zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, > + "pool doesn''t begin with a letter")); > + > + case NAME_ERR_RESERVED: > + zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, > + "name is reserved")); > + > + case NAME_ERR_DISKLIKE: > + zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, > + "reserved disk name")); > } > } > > Index: lib/libzfscommon/zfs_namecheck.c > ==================================================================> --- lib/libzfscommon/zfs_namecheck.c (revision 33) > +++ lib/libzfscommon/zfs_namecheck.c (working copy) > @@ -160,8 +160,8 @@ > /* > * For pool names, we have the same set of valid characters as described in > * dataset names, with the additional restriction that the pool name must begin > - * with a letter. The pool names ''raidz'' and ''mirror'' are also reserved names > - * that cannot be used. > + * with a letter. The pool names ''raidz'', ''raidz1'', ''raidz2'', ''mirror'' and > + * ''spare'' are also reserved names that cannot be used. > */ > int > pool_namecheck(const char *pool, namecheck_err_t *why, char *what) > @@ -201,7 +201,9 @@ > return (-1); > } > > - if (strcmp(pool, "mirror") == 0 || strcmp(pool, "raidz") == 0) { > + if (strcmp(pool, "mirror") == 0 || strcmp(pool, "raidz") == 0 || > + strcmp(pool, "raidz1") == 0 || strcmp(pool, "raidz2") == 0 || > + strcmp(pool, "spare") == 0) { > if (why) > *why = NAME_ERR_RESERVED; > return (-1); > > > ------------------------------------------------------------------------ > > _______________________________________________ > zfs-code mailing list > zfs-code at opensolaris.org > http://opensolaris.org/mailman/listinfo/zfs-code
On Tue, Jun 20, 2006 at 02:13:38PM -0600, Mark Shellenbaum wrote:> Ricardo Correia wrote: > >If I''m not mistaken, there are some new reserved pool names (raidz1, > >raidz2 and spare) that weren''t being properly checked for (in > >zfs_namecheck.c). > > > >There were also some errors that weren''t being handled in libzfs_dataset.c > >(they were detected by gcc''s warnings). > > > > > I just opened the following bug: > > 6441206 missing checks in zfs_namecheck()Actually, this was intentionally not put in zfs_namecheck() due to backwards compatibility issues. If you already had a pool named ''raidz_something'', then if we put the check in zfs_namecheck(), you wouldn''t be able to do _anything_ to the pool. See the comment in zpool_name_valid(): /* * The rules for reserved pool names were extended at a later point. * But we need to support users with existing pools that may now be * invalid. So we only check for this expanded set of names during a * create (or import), and only in userland. */ - Eric -- Eric Schrock, Solaris Kernel Development http://blogs.sun.com/eschrock
Ok, that makes sense. Thanks. By the way, Eric, were you able to reproduce the bug I mentioned earlier? It has proven itself to be very difficult to trigger. I received several reports of successfull 24-hour ztest runs, only 1 user has been affected. I''ve changed the ztest attach/detach frequency to "always", and was able to reproduce it once in 10 minutes. I think I was lucky, because I tried to reproduce it again and even after 10 hours I wasn''t able to do it.. I have the core file available, but I haven''t analyzed the cause yet. On Tuesday 20 June 2006 21:16, Eric Schrock wrote:> Actually, this was intentionally not put in zfs_namecheck() due to > backwards compatibility issues. If you already had a pool named > ''raidz_something'', then if we put the check in zfs_namecheck(), you > wouldn''t be able to do _anything_ to the pool. See the comment in > zpool_name_valid(): > > /* > * The rules for reserved pool names were extended at a later point. > * But we need to support users with existing pools that may now be > * invalid. So we only check for this expanded set of names during > a * create (or import), and only in userland. > */ > > - Eric > > -- > Eric Schrock, Solaris Kernel Development > http://blogs.sun.com/eschrock