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

[sean finney]

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.


        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;
> 




--

574863.patch.4.diff
Description: Text Data

signature.asc
Description: Digital signature