0.9.1 vgscan doesn't detect upgraded vg's

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

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.

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.

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

In article <[email protected]> you write:

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.

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.

Mine were originally created with 0.8final.

Dave Meyer
[email protected]

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

Excellent, thanks Andrea that works for me. I'll put into CVS.

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
@@ -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++) {