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 <[email protected]> >> 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 > [email protected] > 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 [email protected] http://lists.gnu.org/mailman/listinfo/grub-devel
