Bug#574863: [PATCH] Updated fix for Debian Bug#574863: boot failure with lvm2 and snapshot volumes

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 06/03/2010 08:41 PM, sean finney wrote:
> and here is a slightly updated patch.  there is no functional change in the
> code, i have only reformatted the whitespace etc so that the code matches
> with the style of the surrounding code.
>
> i have also tested this now on my primary testing system and haven't
> noticed any problems.
>
>   
I've cleaned up your patch using my more generic string parsing
functions. Could you test attached patch?
> 	sean
>
> On Thu, Jun 03, 2010 at 12:41:32AM +0200, sean finney wrote:
>   
>> okay, I think the attached patch should fix the problem.
>>
>> I haven't tested it thoroughly, though my system does boot.  It seems
>> there may be a seperate issue with os-prober that results in some junk
>> entries being added to grub.conf if the snapshot volumes happen to be
>> root filesystems, but that probably needs to be taken up seperately and
>> the critical aspect of the bug is fixed anyway.
>>
>>
>> 	sean
>>     
>   
>> Author: Sean Finney <seanius at debian.org>
>> Description: Fix for lvm2 parsing failures with snapshot logical volumes
>>
>> This patch prevents the lvm2 parsing code from prematurely aborting
>> when encountering LV and segment metadata related to snapshot volumes.
>> Instead, the parser will now skip over these as if it never saw them,
>> which is probably the safest thing to do without a major injection of
>> lvm2 support code.
>>
>> Bug-Debian: #574863
>> --- disk/lvm.c	2010-04-27 15:25:12 +0000
>> +++ disk/lvm.c	2010-06-02 22:12:58 +0000
>> @@ -420,9 +420,11 @@
>>  	  /* And add all the lvs to the volume group. */
>>  	  while (1)
>>  	    {
>> -	      int s;
>> +	      int s, skip_lv = 0, status_visible = 0;
>>  	      struct grub_lvm_lv *lv;
>>  	      struct grub_lvm_segment *seg;
>> +	      char *status = NULL, *status_end = NULL;
>> +	      grub_size_t status_len = 0;
>>  
>>  	      while (grub_isspace (*p))
>>  		p++;
>> @@ -431,6 +433,8 @@
>>  		break;
>>  
>>  	      lv = grub_malloc (sizeof (*lv));
>> +		skip_lv = 0; /*Flag to skip snapshots */
>> +		status_visible = 0; /*Flag to skip non-visible LV's */
>>  
>>  	      q = p;
>>  	      while (*q != ' ')
>> @@ -445,6 +449,25 @@
>>  
>>  	      lv->size = 0;
>>  
>> +		/* read LV status and ignore ones not listed as "VISIBLE" */
>> +		p = grub_strstr (p, "status = ");
>> +		if (p == NULL)
>> +			goto lvs_fail;
>> +		status_end = grub_strchr(p, ']');
>> +		if (status_end == NULL)
>> +			goto lvs_fail;
>> +		status_len = (status_end - p) + 1;
>> +		status = grub_malloc(status_len + 1);
>> +		if (status == NULL)
>> +			goto lvs_fail;
>> +		grub_memcpy(status, p, status_len);
>> +		status[status_len] = '\0';
>> +		if (grub_strstr(status, "VISIBLE") != NULL)
>> +			status_visible = 1;
>> +		grub_free(status);
>> +		if (!status_visible)
>> +			goto lv_parsed;  /* don't bother parsing this one */
>> +
>>  	      lv->segment_count = grub_lvm_getvalue (&p, "segment_count = ");
>>  	      if (p == NULL)
>>  		goto lvs_fail;
>> @@ -465,6 +488,18 @@
>>  		  seg->extent_count = grub_lvm_getvalue (&p, "extent_count = ");
>>  		  if (p == NULL)
>>  		    goto lvs_segment_fail;
>> +
>> +			/* Skip LV's that have snapshot segments */
>> +			p = grub_strstr (p, "type = ");
>> +			if (p == NULL)
>> +				goto lvs_segment_fail;
>> +			p += sizeof("type = ") - 1;
>> +			if (!grub_strncmp(p, "\"snapshot\"", 10)) {
>> +				/* Found a snapshot, give up and move on */
>> +				skip_lv=1;
>> +				break;
>> +			}
>> +
>>  		  seg->stripe_count = grub_lvm_getvalue (&p, "stripe_count = ");
>>  		  if (p == NULL)
>>  		    goto lvs_segment_fail;
>> @@ -531,12 +566,19 @@
>>  		  goto fail4;
>>  		}
>>  
>> +	    lv_parsed: /* all done with parsing this LV, seek to the end */
>>  	      if (p != NULL)
>>  		p = grub_strchr (p, '}');
>>  	      if (p == NULL)
>>  		goto lvs_fail;
>>  	      p += 3;
>>  
>> +		if (skip_lv || ! status_visible) {
>> +			grub_free (lv->name);
>> +			grub_free (lv);
>> +			continue;
>> +		}
>> +
>>  	      lv->number = lv_count++;
>>  	      lv->vg = vg;
>>  	      lv->next = vg->lvs;
>>
>>     
>
>
>
>   
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel at gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>   


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

-------------- next part --------------
A non-text attachment was scrubbed...
Name: lvm.diff
Type: text/x-diff
Size: 2607 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-grub-devel/attachments/20100629/9320a5e6/attachment-0001.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 294 bytes
Desc: OpenPGP digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-grub-devel/attachments/20100629/9320a5e6/attachment-0001.pgp>