[linux-lvm] 0.9.1 vgscan doesn't detect upgraded vg's

[linux-lvm] 0.9.1 vgscan doesn't detect upgraded vg's

[Michael McLinn]

Hello,

I was running some large vg's with 0.8final, and upgraded
to 0.9.1_beta3 w/ kernel 2.4.1-ac2.  

After the upgrade, vgscan does not find any volume groups.

Reverting to prior tools + kernel does not work as the on disk
vg structures have been altered (presumably upgraded to the v2 
format). 

I have no suitably recent vgcfg backups, so restoring is not an
option.  

Thus, I have been digging in vgscan to see what is going wrong.

My question is this...

I look at the output of pvdata -U on a physical volume. 
I see 3 UUID slots, which matches the number of pv's in the vg.
However, on the first pv in the vg, only the first one is filled.
On the second pv in the vg, the first two are filled.  On the 
third pv of the vg, all three are filled. 

Within pv_read_all_pv_of_vg.c, there is a call to 
pv_read_uuidlist(), this routine is scanning the first pv, 
and thus only returns one uuid.

Later the result is only the first pv is returned from
pv_read_all_pv_of_vg().  

My theory is each pv is supposed to have the the UUID's of 
itself, and all other pv's in it's vg within it's UUID 
datastructure.  Is this so?

If this is the case, it seems it would be possible to write the 
correct UUID's to the datastructure on disk, and potentially 
improve (maybe even fix) my situation.

Any advice would be welcome.  A lot of important data is riding on 
solving this problem.

One more note that might be of interest, these volume groups were
created with a much earlier version of lvm, 0.6 or 0.7 I believe.

Thank you.

-Michael McLinn

Re: [linux-lvm] 0.9.1 vgscan doesn't detect upgraded vg's

[Andreas Dilger]

Michael McLinn writes:
> I was running some large vg's with 0.8final, and upgraded
> to 0.9.1_beta3 w/ kernel 2.4.1-ac2.  
> 
> After the upgrade, vgscan does not find any volume groups.
> 
> Reverting to prior tools + kernel does not work as the on disk
> vg structures have been altered (presumably upgraded to the v2 
> format). 
> 
> I have no suitably recent vgcfg backups, so restoring is not an
> option.  
> 
> I look at the output of pvdata -U on a physical volume. 
> I see 3 UUID slots, which matches the number of pv's in the vg.
> However, on the first pv in the vg, only the first one is filled.
> On the second pv in the vg, the first two are filled.  On the 
> third pv of the vg, all three are filled. 
> 
> Within pv_read_all_pv_of_vg.c, there is a call to 
> pv_read_uuidlist(), this routine is scanning the first pv, 
> and thus only returns one uuid.
> 
> Later the result is only the first pv is returned from
> pv_read_all_pv_of_vg().  
> 
> My theory is each pv is supposed to have the the UUID's of 
> itself, and all other pv's in it's vg within it's UUID 
> datastructure.  Is this so?

Correct.  There was another user who reported the same bug, but after
they were playing with the system a bit, it started to work (i.e. the
UUIDs were filled in) so it was not looked at further.

> If this is the case, it seems it would be possible to write the 
> correct UUID's to the datastructure on disk, and potentially 
> improve (maybe even fix) my situation.

Yes, this is the case.  To confirm that the UUIDs are causing the problem,
you can put printf() in pv_read_all_pv_of_vg() where it is checking
disks against UUIDs, whenever a disk is being discarded.  If this is
really the problem, simply comment out this UUID check for now.  It is
only an extra safety check for duplicate VG names or old PVs at this point.
This should allow you to load the VG again.

> One more note that might be of interest, these volume groups were
> created with a much earlier version of lvm, 0.6 or 0.7 I believe.

I don't know what the situation was with the other person who had
this same bug, but it is good to know anyways.

Cheers, Andreas
-- 
Andreas Dilger  \ "If a man ate a pound of pasta and a pound of antipasto,
                 \  would they cancel out, leaving him still hungry?"
http://www-mddsp.enel.ucalgary.ca/People/adilger/               -- Dogbert

Re: [linux-lvm] 0.9.1 vgscan doesn't detect upgraded vg's

[dmeyer]

In article <200102060630.f166UYM32021@webber.adilger.net> you write:
> Michael McLinn writes:
> > My theory is each pv is supposed to have the the UUID's of 
> > itself, and all other pv's in it's vg within it's UUID 
> > datastructure.  Is this so?
> 
> Correct.  There was another user who reported the same bug, but after
> they were playing with the system a bit, it started to work (i.e. the
> UUIDs were filled in) so it was not looked at further.

That other user was me.  I eventually commented out the UUID check in
pv_read_all_pv_of_vg as the previous poster suggested.  This let me
run vgscan, vgchange -ay, and mount my LVs.  Just go to
pv_read_all_pv_of_vg.c and get rid of the code after the
    if ( uuids > 0 ) {
check.

> > If this is the case, it seems it would be possible to write the 
> > correct UUID's to the datastructure on disk, and potentially 
> > improve (maybe even fix) my situation.

After getting vgscan to find my VGs, I pretty much just lucked into my
UUIDs getting set correctly and I'm still not sure what did it.  All I
did was activate my VGs (which I'm told couldn't have fixed the
UUIDs), run vgck (ditto), and run vgscan after the VGs were active.
vgscan at least _calls_ pv_write_uuidlist, so it is a possibility.
However, running vgscan on the non-active VGs definitely didn't update
the UUIDs for me.

> > One more note that might be of interest, these volume groups were
> > created with a much earlier version of lvm, 0.6 or 0.7 I believe.
> 
> I don't know what the situation was with the other person who had
> this same bug, but it is good to know anyways.

Mine were originally created with 0.8final.

-- 
Dave Meyer
dmeyer@dmeyer.net

Re: [linux-lvm] 0.9.1 vgscan doesn't detect upgraded vg's

[Andreas Dilger]

Michael McLinn writes:
> I look at the output of pvdata -U on a physical volume. 
> I see 3 UUID slots, which matches the number of pv's in the vg.
> However, on the first pv in the vg, only the first one is filled.
> On the second pv in the vg, the first two are filled.  On the 
> third pv of the vg, all three are filled. 

I've found the source of this bug.  It is as follows, assuming an
old VG with no UUID information:

vg_write_with_pv_and_lv() is called to write VGDA to disk
for each PVs in the VG do
   vg_write() - writes out VG data
   pv_write_with_pe() - writes out PV data, creates new UUID if needed
   pv_write_uuidlist() - writes out current UUID list
   lv_write_all_lv() - writes out LV data
done

The problem is that UUIDs are being created as we go, so the UUID list
on the first PV only has the first PV UUID in it.  Exactly the problem
you are having.  I would think this problem would also show up with the
PV namelist as well, but maybe things were done slightly differently then.
It is probably that if you make more than one change which writes out the
VGDA, the problem is gone because the UUIDs now exist on all PVs.

The fix is to verify/fix the UUID list before we start writing PV info.
It appears that vg_write_with_pv_and_lv() is the only place that the UUID
list is written, via pv_write_uuidlist().

Unfortunately, this doesn't help anyone that currently has a problem, but
it will prevent such problems for anyone else upgrading their system.

Cheers, Andreas
========================================================================
diff -u -u -r1.1 pv_write_uuidlist.c
--- tools/lib/pv_write_uuidlist.c	2000/11/13 00:26:33	1.1
+++ tools/lib/pv_write_uuidlist.c	2001/02/07 09:00:08
@@ -30,6 +30,8 @@
  *
  *    08/02/2000 - use debug_enter()/debug_leave()
  *    17/03/2000 - changed namelist to uuidlist
+ *    02/07/2001 - ensure we have valid UUIDs for all PVs before writing [AED]
+ *               - use memcpy instead of strncpy for UUIDs
  *
  */
   
@@ -37,7 +39,7 @@
 
 
 int pv_write_uuidlist ( char *pv_name, vg_t *vg) {
-   int p = 0, pp = 0;
+   int p = 0;
    int pv_handle = -1;
    int ret = 0;
    int size = 0;
@@ -61,14 +63,23 @@
                            __FILE__, __LINE__);
          ret = LVM_EPV_WRITE_UUIDLIST_MALLOC;
       } else {
-         memset ( pv_uuid_list, 0, size);
-         for ( p = 0; p < vg->pv_max; p++) {
-            if ( vg->pv[p] != NULL) {
-               strncpy ( &pv_uuid_list[pp*NAME_LEN],
-                         vg->pv[p]->pv_uuid,
-                         UUID_LEN);
-               pp++;
-            }
+         char *u = memset(pv_uuid_list, 0, size);
+         for (p = 0; p < vg->pv_max; p++) {
+            pv_t *pv = vg->pv[p];
+            if (pv == NULL)
+               continue;
+
+            /* Create/fix UUIDs for any PVs that need it */
+            if (lvm_check_uuid(pv->pv_uuid) < 0) {
+#ifdef DEBUG
+               debug(__FUNCTION__ " -- creating new UUID for PV %s\n",
+                     pv->pv_name);
+#endif
+               memset(pv->pv_uuid, 0, sizeof(pv->pv_uuid));
+               memcpy(pv->pv_uuid, lvm_create_uuid(UUID_LEN), UUID_LEN);
+            }
+            memcpy(u, pv->pv_uuid, UUID_LEN);
+            u += NAME_LEN;
          }
          if ( write ( pv_handle, pv_uuid_list, size) != size)
             ret = -LVM_EPV_WRITE_UUIDLIST_WRITE;
diff -u -u -r1.2 vg_write_with_pv_and_lv.c
--- tools/lib/vg_write_with_pv_and_lv.c	2000/11/13 00:20:11	1.2
+++ tools/lib/vg_write_with_pv_and_lv.c	2001/02/07 09:00:21
@@ -55,9 +55,9 @@
                   vg->pv[p]->pv_name);
 #endif
          if ( ( ret = vg_write ( vg->pv[p]->pv_name, vg->pv[p], vg)) < 0) break;
+         if ( ( ret = pv_write_uuidlist ( vg->pv[p]->pv_name, vg)) < 0) break;
          if ( ( ret = pv_write_with_pe ( vg->pv[p]->pv_name,
                                          vg->pv[p])) < 0) break;
-         if ( ( ret = pv_write_uuidlist ( vg->pv[p]->pv_name, vg)) < 0) break;
          if ( ( ret = lv_write_all_lv ( vg->pv[p]->pv_name, vg)) < 0)
             break;
       }
-- 
Andreas Dilger  \ "If a man ate a pound of pasta and a pound of antipasto,
                 \  would they cancel out, leaving him still hungry?"
http://www-mddsp.enel.ucalgary.ca/People/adilger/               -- Dogbert

Re: [linux-lvm] 0.9.1 vgscan doesn't detect upgraded vg's

[Patrick Caulfield]

On Wed, Feb 07, 2001 at 02:44:58AM -0700, Andreas Dilger wrote:
> 
> The problem is that UUIDs are being created as we go, so the UUID list
> on the first PV only has the first PV UUID in it.  Exactly the problem
> you are having.  I would think this problem would also show up with the
> PV namelist as well, but maybe things were done slightly differently then.
> It is probably that if you make more than one change which writes out the
> VGDA, the problem is gone because the UUIDs now exist on all PVs.
> 
> The fix is to verify/fix the UUID list before we start writing PV info.
> It appears that vg_write_with_pv_and_lv() is the only place that the UUID
> list is written, via pv_write_uuidlist().

Excellent, thanks Andrea that works for me. I'll put into CVS.
 
> Unfortunately, this doesn't help anyone that currently has a problem, but
> it will prevent such problems for anyone else upgrading their system.

No, but the patch below at least can get them running. It's necessary to
do some metadata changes to get the UUID list committed to disk after this
though.

patrick


diff -u -r1.3.2.3 tools/lib/pv_read_all_pv_of_vg.c
--- pv_read_all_pv_of_vg.c      2001/01/17 14:45:28     1.3.2.3
+++ pv_read_all_pv_of_vg.c      2001/02/06 15:42:40
@@ -114,7 +114,7 @@
             np++;
          }
       }
-
+#if 0 // PJC: work around UUID list problems.
       /* in case this PV already holds a uuid list: check against this list */
       if ( uuids > 0) {
          for ( p = 0; pv_this[p] != NULL; p++) {
@@ -135,7 +135,7 @@
          np = 0;
          while ( pv_this[np] != NULL) np++;
       }
-
+#endif
       /* avoid multiple access pathes */
       if (pv_this) {
          for ( p = 0; pv_this[p] != NULL; p++) {