[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [lvm-devel] [PATCH] Fix zombie mirror poll daemon (bug 585328)



Milan Broz <mbroz redhat com> writes:

> On 08/25/2010 11:33 PM, Jonathan Brassow wrote:
>> -	if (!lv_mirror_percent(cmd, lv, !parms->interval, &segment_percent,
>> +	if (!(lv->status & MIRRORED) ||
>> +	    !lv_mirror_percent(cmd, lv, !parms->interval, &segment_percent,
>>  			       &percent_range, &event_nr) ||
>>  	    (percent_range == PERCENT_INVALID)) {
>>  		log_error("ABORTING: Mirror percentage check failed.");
>
> why not move this check inside lv_mirror_percent?
> (or is this only place where the bug can appear?)

I think it's actually safer to fix this here at this point in
time. Otherwise, lv_mirror_percent could start failing unexpectedly on
some mis-behaving code and introduce new bugs.

> (and it should probably use !lv_is_mirrored(lv) instead of mask)
Indeed. Please fix.

I would personally also change the code to print a different error if
the mirror is no longer a mirror, which would be likely less confusing
to the user.

Anyway, assuming the lv_is_mirrored change, OK.

Reviewed-By: Petr Rockai <prockai redhat com>

Yours,
   Petr.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]