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