[lvm-devel] [PATCH] Ignore _mlog name restriction for lvconvert repair

Jonathan Brassow jbrassow at redhat.com
Fri Feb 12 21:45:09 UTC 2010


On Feb 12, 2010, at 3:30 PM, Jonathan Brassow wrote:

>
> On Feb 11, 2010, at 7:14 PM, Takahiro Yasui wrote:
>
>> malahal at us.ibm.com wrote:
>>> Takahiro Yasui [tyasui at redhat.com] wrote:
>>>> Malahal Naineni wrote:
>>>>> lvconvert --repair is done on _mlog mirrored log logical volumes  
>>>>> from
>>>>> dmeventd if something fails.
>>>>>
>>>>> diff -r 86200db56a7c -r 471e224a5713 tools/lvconvert.c
>>>>> --- a/tools/lvconvert.c	Tue Feb 09 17:49:50 2010 -0800
>>>>> +++ b/tools/lvconvert.c	Wed Feb 10 09:12:11 2010 -0800
>>>>> @@ -105,8 +105,12 @@ static int _lvconvert_name_params(struct
>>>>> 	if ((ptr = strrchr(lp->lv_name_full, '/')))
>>>>> 		lp->lv_name = ptr + 1;
>>>>>
>>>>> -	if (!apply_lvname_restrictions(lp->lv_name))
>>>>> -		return_0;
>>>>> +	/* _mlog is an internal name, but it could be mirrored, so
>>>>> +	 * allow repairing it.
>>>>> +	 */
>>>>> +	if (!arg_count(cmd, repair_ARG) || !strstr(lp->lv_name,  
>>>>> "_mlog"))
>>>>> +		if (!apply_lvname_restrictions(lp->lv_name))
>>>>> +			return_0;
>>>>>
>>>>> 	if (*pargc && lp->snapshot) {
>>>>> 		log_error("Too many arguments provided for snapshots");
>>>> lvname is better to be checked if a logical volume is not  
>>>> mirrored log
>>>> but simple logical volume. How about using (lv->status &  
>>>> MIRRORED) for
>>>> the check?
>>>
>>> I thought about it but such details are not available at that  
>>> point. All
>>> the information available at that point is derived from the  
>>> command line
>>> arguments!
>>
>> Thank you for the explanation. I see we need to move this name check
>> at the place where lv->status check can be used.
>
> It might make sense to do it in this very spot...  If the repair_ARG  
> is present, why should we be checking lvname restrictions anyway?   
> No new LVs are being created.
>
> Something more like:
> if (!arg_count(cmd, repair_ARG) && !apply_lvname_restrictions(lp- 
> >lv_name))
>
> Anyone see anything wrong with that?
>
> Taka, I've not been able to reproduce your seg faults with '--alloc  
> anywhere' using two devices.  Is there a special restriction you are  
> using?

agk wants to see if we can /not/ use <lv>_mlog as the argument passed  
in.  So, dmeventd would probably strip the '_mlog' portion.(?)  There  
are definitely some items to clean up in this case.  I'm going to give  
it a shot.  If it turns out to be really bad, we might go with the  
above suggestion.

  brassow




More information about the lvm-devel mailing list