[linux-lvm] vgcfgrestore bug followup

[linux-lvm] vgcfgrestore bug followup

[Brad Corsello]

Following up on my own post, I seem to have solved my problem by editing the
vg data in the first physical volume with a hex editor, adding the uuid of the
second PV where it was supposed to be.  Once I did that, vgscan ran
successfully and found the volume group.  But I think this just proves
vgcfgrestore is not restoring the uuid list to disk properly.

(But then I pushed my luck and tried doing vgextends.  After adding three PVs,
vgscan started to throw up with a weird error:  vg_read_with_pv_and_lv():
allocated LE of LV" can't get data of volume group "vg1" from physical
volume(s).  I don't know if this is related to the vgcfgrestore problem or
not.)

Re: [linux-lvm] vgcfgrestore bug followup

[Jan Niehusmann]

On Sun, Jan 07, 2001 at 04:23:50PM -0500, Brad Corsello wrote:
> (But then I pushed my luck and tried doing vgextends.  After adding three PVs,
> vgscan started to throw up with a weird error:  vg_read_with_pv_and_lv():
> allocated LE of LV" can't get data of volume group "vg1" from physical
> volume(s).  I don't know if this is related to the vgcfgrestore problem or
> not.)

This one looks like a problem I had myself. For me, the following patch
did help: (I already posted this patch, with a description, on Dec 23)

Index: pv_read_all_pv_of_vg.c
===================================================================
RCS file: /data/cvs/LVM/tools/lib/pv_read_all_pv_of_vg.c,v
retrieving revision 1.3
diff -u -1 -b -p -r1.3 pv_read_all_pv_of_vg.c
--- pv_read_all_pv_of_vg.c	2000/11/20 02:47:20	1.3
+++ pv_read_all_pv_of_vg.c	2000/12/22 23:27:00
@@ -103,2 +103,3 @@ int pv_read_all_pv_of_vg ( char *vg_name
             pv_this_sav = pv_this;
+	    if(np<pv_tmp[p]->pv_number) np=pv_tmp[p]->pv_number;
             if ( ( pv_this = realloc ( pv_this,
@@ -111,5 +112,4 @@ int pv_read_all_pv_of_vg ( char *vg_name
             }
-            pv_this[np] = pv_tmp[p];
-            pv_this[np+1] = NULL;
-            np++;
+            pv_this[pv_tmp[p]->pv_number-1] = pv_tmp[p];
+            //pv_this[np+1] = NULL; // XXX???
          }

Re: [linux-lvm] vgcfgrestore bug followup

[Holger Grothe]

On Thu, Jan 04, 2001 at 03:49:26PM -0200, Rik van Riel wrote:
> The patch posted on december 23rd by Jan Niehusmann
> has the extra result that vgscan always segfaults,
> and not just when in debugging output...

and

On Sun, Jan 07, 2001 at 10:33:00PM +0100, Jan Niehusmann wrote:
[...]
> This one looks like a problem I had myself. For me, the following patch
> did help: (I already posted this patch, with a description, on Dec 23)
> 
> Index: pv_read_all_pv_of_vg.c
> ===================================================================
> RCS file: /data/cvs/LVM/tools/lib/pv_read_all_pv_of_vg.c,v
> retrieving revision 1.3
> diff -u -1 -b -p -r1.3 pv_read_all_pv_of_vg.c
> --- pv_read_all_pv_of_vg.c	2000/11/20 02:47:20	1.3
> +++ pv_read_all_pv_of_vg.c	2000/12/22 23:27:00
> @@ -103,2 +103,3 @@ int pv_read_all_pv_of_vg ( char *vg_name
>              pv_this_sav = pv_this;
> +	    if(np<pv_tmp[p]->pv_number) np=pv_tmp[p]->pv_number;
>              if ( ( pv_this = realloc ( pv_this,
> @@ -111,5 +112,4 @@ int pv_read_all_pv_of_vg ( char *vg_name
>              }
> -            pv_this[np] = pv_tmp[p];
> -            pv_this[np+1] = NULL;
> -            np++;
> +            pv_this[pv_tmp[p]->pv_number-1] = pv_tmp[p];
> +            //pv_this[np+1] = NULL; // XXX???
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
IMHO the pv_this list has to be terminated by NULL (see next lines of 
source). Otherwise you may get "seg faults" while running vgscan
(see. Rik's mail). Replacing the marked line with 'pv_this[np] = NULL;' 
worked for me.

Holger
-- 
Holger Grothe  (Email: grothe@mathematik.tu-darmstadt.de)
Fachbereich Mathematik, TU Darmstadt

Re: [linux-lvm] vgcfgrestore bug followup

[Jan Niehusmann]

On Mon, Jan 08, 2001 at 01:26:32AM +0100, Holger Grothe wrote:
> > Index: pv_read_all_pv_of_vg.c
> > ===================================================================
> > RCS file: /data/cvs/LVM/tools/lib/pv_read_all_pv_of_vg.c,v
> > retrieving revision 1.3
> > diff -u -1 -b -p -r1.3 pv_read_all_pv_of_vg.c
> > --- pv_read_all_pv_of_vg.c	2000/11/20 02:47:20	1.3
> > +++ pv_read_all_pv_of_vg.c	2000/12/22 23:27:00
> > @@ -103,2 +103,3 @@ int pv_read_all_pv_of_vg ( char *vg_name
> >              pv_this_sav = pv_this;
> > +	    if(np<pv_tmp[p]->pv_number) np=pv_tmp[p]->pv_number;
> >              if ( ( pv_this = realloc ( pv_this,
> > @@ -111,5 +112,4 @@ int pv_read_all_pv_of_vg ( char *vg_name
> >              }
> > -            pv_this[np] = pv_tmp[p];
> > -            pv_this[np+1] = NULL;
> > -            np++;
> > +            pv_this[pv_tmp[p]->pv_number-1] = pv_tmp[p];
> > +            //pv_this[np+1] = NULL; // XXX???
>                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> IMHO the pv_this list has to be terminated by NULL (see next lines of 
> source). Otherwise you may get "seg faults" while running vgscan
> (see. Rik's mail). Replacing the marked line with 'pv_this[np] = NULL;' 
> worked for me.

Yes, that's why I marked this line with XXX ;-). 
I did fix the bug that hit me, but I didn't check if I may have introduced
new bugs.
You are probably right that pv_this[np]=NULL is needed. By the way, 
the realloc can be changed to (np+1)*sizeof(...) instead of (np+2)*sizeof(...).

Jan