[zfs-discuss] zpool import minor bug in snv_64a

2007-06-25 Thread Dennis Clarke

Not sure if this has been reported or not.

This is fairly minor but slightly annoying.

After fresh install of snv_64a I run zpool import to find this :

# zpool import
  pool: zfs0
id: 13628474126490956011
 state: ONLINE
status: The pool is formatted using an older on-disk version.
action: The pool can be imported using its name or numeric identifier, though
some features will not be available without an explicit 'zpool
upgrade'.
config:

zfs0 ONLINE
  mirror ONLINE
c1t9d0   ONLINE
c0t9d0   ONLINE
  mirror ONLINE
c1t10d0  ONLINE
c0t10d0  ONLINE
  mirror ONLINE
c1t11d0  ONLINE
c0t11d0  ONLINE
  mirror ONLINE
c1t12d0  ONLINE
c0t12d0  ONLINE
  mirror ONLINE
c1t13d0  ONLINE
c0t13d0  ONLINE
  mirror ONLINE
c1t14d0  ONLINE
c0t14d0  ONLINE

So I then run a zpool import but I add in the -R option and specify root thus :

# zpool import -f -R / 13628474126490956011

One would think that the -R / would not result in any damage but this si
the result :

# zfs list
NAME   USED  AVAIL  REFER  MOUNTPOINT
zfs0   191G  8.23G  24.5K  legacy
zfs0/SUNWspro  567M   201M   567M  //opt/SUNWspro
zfs0/backup190G  8.23G   189G  //export/zfs/backup
zfs0/backup/qemu  1.09G   934M  1.09G  //export/zfs/qemu
zfs0/csw   124M  3.88G   124M  //opt/csw
zfs0/home  239M  7.77G   239M  //export/home
zfs0/titan24.5K  8.23G  24.5K  //export/zfs/titan

Note the extra / there that should not be there.

Not a simple thing to fix either :

# zfs set mountpoint=/opt/SUNWspro zfs0/SUNWspro
# zfs list
NAME   USED  AVAIL  REFER  MOUNTPOINT
zfs0   191G  8.23G  24.5K  legacy
zfs0/SUNWspro  567M   201M   567M  //opt/SUNWspro
zfs0/backup190G  8.23G   189G  //export/zfs/backup
zfs0/backup/qemu  1.09G   934M  1.09G  //export/zfs/qemu
zfs0/csw   124M  3.88G   124M  //opt/csw
zfs0/home  239M  7.77G   239M  //export/home
zfs0/titan24.5K  8.23G  24.5K  //export/zfs/titan

relatively harmless.

Looks like altroot should be assumed to be / unless otherwise specified and
if it is specified to be / then the altroot can be ignored.  I don't know if
that is clear but I think you know what I mean :

in /usr/src/cmd/zpool/zpool_main.c :

static int do_import(nvlist_t *config, const char *newname, const char
*mntopts, const char *altroot, int force, int argc, char **argv)


if that const char *altroot  happens to be nothing more than a forward slash
char ( nul terminated ) then I think it should be ignored.

What say you ?

Dennis

___
zfs-discuss mailing list
zfs-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/zfs-discuss


Re: [zfs-discuss] zpool import minor bug in snv_64a

2007-06-25 Thread Pawel Jakub Dawidek
On Mon, Jun 25, 2007 at 02:34:21AM -0400, Dennis Clarke wrote:
 
  in /usr/src/cmd/zpool/zpool_main.c :
 
 
 at line 680 forwards we can probably check for this scenario :
 
 if ( ( altroot != NULL )  ( altroot[0] != '/') ) {
 (void) fprintf(stderr, gettext(invalid alternate root '%s': 
 must be an absolute path\n), altroot);
 nvlist_free(nvroot);
 return (1);
 }
 
 /*  some altroot has been specified  *
  *  thus altroot[0] and altroot[1] exist */
 
 else if ( ( altroot[0] = '/')  ( altroot[1] = '\0') ) {

s/=/==/

 (void) fprintf(stderr, Do not specify / as alternate root.\n);

You need gettext() here.

 nvlist_free(nvroot);
 return (1);
 }
 
 
 not perfect .. but something along those lines.

-- 
Pawel Jakub Dawidek   http://www.wheel.pl
[EMAIL PROTECTED]   http://www.FreeBSD.org
FreeBSD committer Am I Evil? Yes, I Am!


pgpKWVUs2EH4y.pgp
Description: PGP signature
___
zfs-discuss mailing list
zfs-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/zfs-discuss


Re: [zfs-discuss] zpool import minor bug in snv_64a

2007-06-25 Thread Dennis Clarke

 in /usr/src/cmd/zpool/zpool_main.c :


at line 680 forwards we can probably check for this scenario :

if ( ( altroot != NULL )  ( altroot[0] != '/') ) {
(void) fprintf(stderr, gettext(invalid alternate root '%s': 
must be an absolute path\n), altroot);
nvlist_free(nvroot);
return (1);
}

/*  some altroot has been specified  *
 *  thus altroot[0] and altroot[1] exist */

else if ( ( altroot[0] = '/')  ( altroot[1] = '\0') ) {
(void) fprintf(stderr, Do not specify / as alternate root.\n);
nvlist_free(nvroot);
return (1);
}


not perfect .. but something along those lines.


Dennis

___
zfs-discuss mailing list
zfs-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/zfs-discuss


Re: [zfs-discuss] zpool import minor bug in snv_64a

2007-06-25 Thread Dennis Clarke

 On Mon, Jun 25, 2007 at 02:34:21AM -0400, Dennis Clarke wrote:

   note that it was well after 2 AM for me .. half blind asleep

   that's my excuse .. I'm sticking to it.   :-)


  in /usr/src/cmd/zpool/zpool_main.c :
 

 at line 680 forwards we can probably check for this scenario :

 if ( ( altroot != NULL )  ( altroot[0] != '/') ) {
 (void) fprintf(stderr, gettext(invalid alternate root '%s': 
 must be an absolute path\n), altroot);
 nvlist_free(nvroot);
 return (1);
 }

 /*  some altroot has been specified  *
  *  thus altroot[0] and altroot[1] exist */

 else if ( ( altroot[0] = '/')  ( altroot[1] = '\0') ) {

 s/=/==/

yep ... that's what I intended.  The above would bork royally.


 (void) fprintf(stderr, Do not specify / as alternate root.\n);

 You need gettext() here.

  why ?


 nvlist_free(nvroot);
 return (1);
 }


 not perfect .. but something along those lines.

even worse .. I was looking in the wrong section of the code or zpool_main.c

if I get coffee and wake up .. maybe I can take another kick at that eh?

Dennis
___
zfs-discuss mailing list
zfs-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/zfs-discuss


Re: [zfs-discuss] zpool import minor bug in snv_64a

2007-06-25 Thread Eric Schrock
You've tripped over a variant of:

6335095 Double-slash on /. pool mount points

- Eric

On Mon, Jun 25, 2007 at 02:11:33AM -0400, Dennis Clarke wrote:
 
 Not sure if this has been reported or not.
 
 This is fairly minor but slightly annoying.
 
 After fresh install of snv_64a I run zpool import to find this :
 
 # zpool import
   pool: zfs0
 id: 13628474126490956011
  state: ONLINE
 status: The pool is formatted using an older on-disk version.
 action: The pool can be imported using its name or numeric identifier, though
 some features will not be available without an explicit 'zpool
 upgrade'.
 config:
 
 zfs0 ONLINE
   mirror ONLINE
 c1t9d0   ONLINE
 c0t9d0   ONLINE
   mirror ONLINE
 c1t10d0  ONLINE
 c0t10d0  ONLINE
   mirror ONLINE
 c1t11d0  ONLINE
 c0t11d0  ONLINE
   mirror ONLINE
 c1t12d0  ONLINE
 c0t12d0  ONLINE
   mirror ONLINE
 c1t13d0  ONLINE
 c0t13d0  ONLINE
   mirror ONLINE
 c1t14d0  ONLINE
 c0t14d0  ONLINE
 
 So I then run a zpool import but I add in the -R option and specify root thus 
 :
 
 # zpool import -f -R / 13628474126490956011
 
 One would think that the -R / would not result in any damage but this si
 the result :
 
 # zfs list
 NAME   USED  AVAIL  REFER  MOUNTPOINT
 zfs0   191G  8.23G  24.5K  legacy
 zfs0/SUNWspro  567M   201M   567M  //opt/SUNWspro
 zfs0/backup190G  8.23G   189G  //export/zfs/backup
 zfs0/backup/qemu  1.09G   934M  1.09G  //export/zfs/qemu
 zfs0/csw   124M  3.88G   124M  //opt/csw
 zfs0/home  239M  7.77G   239M  //export/home
 zfs0/titan24.5K  8.23G  24.5K  //export/zfs/titan
 
 Note the extra / there that should not be there.
 
 Not a simple thing to fix either :
 
 # zfs set mountpoint=/opt/SUNWspro zfs0/SUNWspro
 # zfs list
 NAME   USED  AVAIL  REFER  MOUNTPOINT
 zfs0   191G  8.23G  24.5K  legacy
 zfs0/SUNWspro  567M   201M   567M  //opt/SUNWspro
 zfs0/backup190G  8.23G   189G  //export/zfs/backup
 zfs0/backup/qemu  1.09G   934M  1.09G  //export/zfs/qemu
 zfs0/csw   124M  3.88G   124M  //opt/csw
 zfs0/home  239M  7.77G   239M  //export/home
 zfs0/titan24.5K  8.23G  24.5K  //export/zfs/titan
 
 relatively harmless.
 
 Looks like altroot should be assumed to be / unless otherwise specified and
 if it is specified to be / then the altroot can be ignored.  I don't know if
 that is clear but I think you know what I mean :
 
 in /usr/src/cmd/zpool/zpool_main.c :
 
 static int do_import(nvlist_t *config, const char *newname, const char
 *mntopts, const char *altroot, int force, int argc, char **argv)
 
 
 if that const char *altroot  happens to be nothing more than a forward slash
 char ( nul terminated ) then I think it should be ignored.
 
 What say you ?
 
 Dennis
 
 ___
 zfs-discuss mailing list
 zfs-discuss@opensolaris.org
 http://mail.opensolaris.org/mailman/listinfo/zfs-discuss

--
Eric Schrock, Solaris Kernel Development   http://blogs.sun.com/eschrock
___
zfs-discuss mailing list
zfs-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/zfs-discuss


Re: [zfs-discuss] zpool import minor bug in snv_64a

2007-06-25 Thread Dennis Clarke

 You've tripped over a variant of:

 6335095 Double-slash on /. pool mount points

 - Eric


oh well .. no points for originality then I guess :-)

Thanks

___
zfs-discuss mailing list
zfs-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/zfs-discuss