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 <sean...@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@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
> -- Regards Vladimir 'φ-coder/phcoder' Serbinenko
=== modified file 'disk/lvm.c'
--- disk/lvm.c 2010-04-27 15:25:12 +0000
+++ disk/lvm.c 2010-06-29 05:27:15 +0000
@@ -46,6 +46,52 @@ grub_lvm_getvalue (char **p, char *str)
}
static int
+grub_lvm_checkvalue (char **p, char *str, char *tmpl)
+{
+ int tmpllen = grub_strlen (tmpl);
+ *p = grub_strstr (*p, str);
+ if (! *p)
+ return 0;
+ *p += grub_strlen (str);
+ if (**p != '"')
+ return 0;
+ return (grub_memcmp (*p + 1, tmpl, tmpllen) == 0 && (*p)[tmpllen + 1] == '"');
+}
+
+static int
+grub_lvm_check_flag (char *p, char *str, char *flag)
+{
+ int len_str = grub_strlen (str), len_flag = grub_strlen (flag);
+ while (1)
+ {
+ char *q;
+ p = grub_strstr (p, str);
+ if (! p)
+ return 0;
+ p += len_str;
+ if (grub_memcmp (p, " = [", sizeof (" = [") - 1) != 0)
+ continue;
+ q = p + sizeof (" = [") - 1;
+ while (1)
+ {
+ while (grub_isspace (*q))
+ q++;
+ if (*q != '"')
+ return 0;
+ q++;
+ if (grub_memcmp (q, flag, len_flag) == 0 && q[len_flag] == '"')
+ return 1;
+ while (*q != '"')
+ q++;
+ q++;
+ if (*q == ']')
+ return 0;
+ q++;
+ }
+ }
+}
+
+static int
grub_lvm_iterate (int (*hook) (const char *name))
{
struct grub_lvm_vg *vg;
@@ -421,6 +467,7 @@ grub_lvm_scan_device (const char *name)
while (1)
{
int s;
+ int skip_lv = 0;
struct grub_lvm_lv *lv;
struct grub_lvm_segment *seg;
@@ -445,6 +492,12 @@ grub_lvm_scan_device (const char *name)
lv->size = 0;
+ if (!grub_lvm_check_flag (p, "status", "VISIBLE"))
+ {
+ skip_lv = 1;
+ goto lv_parsed;
+ }
+
lv->segment_count = grub_lvm_getvalue (&p, "segment_count = ");
if (p == NULL)
goto lvs_fail;
@@ -465,6 +518,14 @@ grub_lvm_scan_device (const char *name)
seg->extent_count = grub_lvm_getvalue (&p, "extent_count = ");
if (p == NULL)
goto lvs_segment_fail;
+
+ if (grub_lvm_checkvalue (&p, "type = ", "snapshot"))
+ {
+ /* 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 +592,20 @@ grub_lvm_scan_device (const char *name)
goto fail4;
}
+ lv_parsed:
if (p != NULL)
p = grub_strchr (p, '}');
if (p == NULL)
goto lvs_fail;
p += 3;
+ if (skip_lv)
+ {
+ grub_free (lv->name);
+ grub_free (lv);
+ continue;
+ }
+
lv->number = lv_count++;
lv->vg = vg;
lv->next = vg->lvs;
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
