Re: [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 <address@hidden>
>> 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
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel
>   


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

lvm.diff
Description: Text Data

signature.asc
Description: OpenPGP digital signature