Hi William,

>
>
> Sarah Jelinek wrote:
>> 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
> There is a problem with zpool import -R <root> -a in that if you have 
> pools with the same name, the importing isn't done right - the pools 
> are imported to the same mount point.  This could happen if you 
> installed LiveCD to  two different disks on the same system.
>>
>> -Why is this code conditional:
> An oversight - I will remove it.
>>
>>
>>  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?
> I try to do it with only one temp directory, creating more only if the 
> be_unmount() fails for some reason.  More explanation follows...
>> 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 */
> I coded this to delete any temp mount point it created.  If the 
> be_unmount() fails, there is still data there, and I won't be able to 
> delete the directory.  By marking tmpbemp as NULL, the next time 
> through the BE list loop, a new temp directory will be created (if 
> there is another BE in the list).
>> thanks,
>> sarah
>> *****
>> You set it to NULL but in done:
>>
>> done:
>>  320         if (tmpbemp != NULL)
>>  321                 (void) rmdir(tmpbemp);
> When I'm done: with listing the BEs, I delete the last temp directory 
> here.
>>
>> Don't you need to rmdir each instance of this directory for each be?
> I have tried to code this to use only a single temp directory, and 
> only to create another temp directory if I couldn't be_unmount() the 
> last one.
> I think I'm doing this right.  Have I made this clear?
yes, thanks.

sarah
*****
> William
>>
>>
>>  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
>>>
>>>
>> _______________________________________________
>> caiman-discuss mailing list
>> caiman-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>

Reply via email to