Hi Jan,
Thank you for the review. Comments inline...
> Hi Sarah,
>
> the changes look good, I have only couple of nits
> (please see below).
>
> Thank you,
> Jan
>
>
> 852 (void) fprintf(stderr, "ERROR: remove current controller\n");
> ->
> if (dm_debug) {
> (void) fprintf(stderr, "ERROR: remove current controller\n");
> }
>
>
> 871 for (i = 0; dp->controllers[i]; i ++) {
> ->
> 871 for (i = 0; dp->controllers[i]; i++) {
>
>
I will fix the two above.
> 876-879, 1867-1870
> - Since I am not sure how controllers[] were initially allocated,
> do we need to take care of freeing the space for last member somehow
> after that move is done ?
>
We do free space for the "offending" controller later in this function:
> cache_free_controller(cp);
What I am doing is basically is this:
If controller at position 3, associated with disk N is the controller we
want disassociate with disk N, then I start at controller[3] and assign
it to the controller[3 +1], and so on, for all controllers associated
with disk N. The only space I need to free is the controller[3], which I
am doing.
Does this answer your question?
thanks,
sarah
****
>
> On 01/15/10 02:16 AM, Sarah Jelinek wrote:
>> Hi Jan and Ginnie,
>>
>> Could you do a code review for:
>>
>> 6795293 libdiskmgt coredump on x86 with devices under mpxio
>> control/SUN-Universal Xport inbound mangement
>>
>> Webrev is here:
>>
>> http://cr.opensolaris.org/~sjelinek/6795293/
>>
>> ****Building and Testing done:
>> -Both sparc and x86 built clean
>> -Tested on both x86 and sparc:
>> x86 testing reproduced bug and also shows bug fix
>> sparc testing for regression testing. Bug does not present
>> itself on sparc.
>>
>> -Reproduced bug and tested fix on oaf627.ireland. This is an x86 box
>> with Qlogic FC controllers.
>> -Tested both mpxio and non-mpxio
>> -ran ::findleaks. No leaks detected.
>>
>> Test output is located here:
>>
>> /net/irperf.ireland/export/work/bfu/sjelinek
>>
>> The files in this directory are specifically:
>> ***Bad run(bug is present):
>> -core.test_td.2664.1263495933.gz-core showing original bug reproduced
>> -libdiskmgt_bad.txt
>> -test_td_bad.txt
>>
>> ***Good run(bug fixed):
>> -libdiskmgt_good.txt
>> -test_td_good.txt
>> -format.txt - shows output of format to show match with
>> test_td_good.txt data.
>>
>> ***Good run, mpxio disabled(bug fixed):
>> -libdiskmgt_nonmpxio.txt
>> -test_td_nonmpxio.txt
>> -format_nonmpxio.txt - shows output of format to show match with
>> -test_td_nonmpxio.txt data.
>>
>>
>> thanks,
>> sarah
>>
>
