Code review comments. Please don't putback until zpool status info
gathering is resolved...
td_be.c:
-What is the meant by 'conflicting names' in this comment:
BUG:"zpool import -f -R /mnt/zpools -a" has problems if root pools have
146 * conflicting names
-Why is this code conditional:
254 #if 0
255 if ((err = be_mount(be)) != 0) {
256 continue;
257 }
258 #endif
-It seems as if you are not deleting all the tmpbemp directories that
could have possibly been created?
mount:
313 /* if BE not previously mounted, unmount it */
314 if (was_be_mounted && be_unmount(be) != 0) {
315 /* if unmount fails, directory will be busy */
316 tmpbemp = NULL; /* new mountpt created if
needed */
317 }
318 } /* for each BE */
thanks,
sarah
*****
You set it to NULL but in done:
done:
320 if (tmpbemp != NULL)
321 (void) rmdir(tmpbemp);
Don't you need to rmdir each instance of this directory for each be?
322 }
William Schumann wrote:
> Request for code review: 909 - Target Discovery must find Solaris
> instances in BEs
>
> Target Discovery must provide the disk devices containing Snap Boot
> Environments to the GUI so that user is informed of Solaris BEs on a
> disk.
>
> be_list() provides a list of Solaris Boot environments, but requires
> some preparation, since zpools must first be imported
>
> This code performs two workarounds:
>
> Workaround: INST_RELEASE, which contains release information, is
> missing from
> Caiman, and is in the process of being re-established. Code to read
> INST_RELEASE is conditionally coded
>
> Workaround: 'zpool status' is used to retrieve the disk device name
> holding the
> Solaris BE. The output of 'zpool status' was not designed to be
> parsed; the
> parsing of this output is perhaps unreliable. An outstanding CR
> exists to
> supply this functionality:
> 6667439 A parsable option for zpool status and zpool list is needed
>
>
> http://cr.opensolaris.org/~wmsch/bug-909/
> http://defect.opensolaris.org/bz/show_bug.cgi?id=909
>
> William
>
>
