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
>