Clear up precommitted metadata better on disk after use.

[Some activation-related features will stop working for a while now.
Some types of activation are getting split into two steps, with the
first step using the precommitted metadata.]

diff --git a/WHATS_NEW b/WHATS_NEW
index 24b659ed35506511f07a123d4f7aa0eebbd6d2b2..d0c550c199638ed13eb408a2c8e1101a30a13432 100644 (file)
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
 Version 2.02.00 - 
 ===================================
+  Clear up precommitted metadata better.
   A pvresize implementation.
   Fix contiguous allocation when there are no preceding segments.
   Add mirror_seg pointer to lv_segment struct.

diff --git a/lib/activate/activate.c b/lib/activate/activate.c
index 6bc72ed69643ef71c436528deb8611c8ac72e63a..f15123d74072517f3c6cfd3b7f76c2b5f7f23f58 100644 (file)
--- a/lib/activate/activate.c
+++ b/lib/activate/activate.c
@@ -388,7 +388,7 @@ int lv_info_by_lvid(struct cmd_context *cmd, const char *lvid_s,
 {
        struct logical_volume *lv;
 
-       if (!(lv = lv_from_lvid(cmd, lvid_s)))
+       if (!(lv = lv_from_lvid(cmd, lvid_s, 0)))
                return 0;
 
        return _lv_info(cmd, lv, 0, info, with_open_count);
@@ -571,7 +571,8 @@ static int _lv_suspend(struct cmd_context *cmd, const char *lvid_s,
        if (!activation())
                return 1;
 
-       if (!(lv = lv_from_lvid(cmd, lvid_s)))
+       /* Use precommitted metadata if present */
+       if (!(lv = lv_from_lvid(cmd, lvid_s, 1)))
                return 0;
 
        if (test_mode()) {
@@ -617,7 +618,7 @@ static int _lv_resume(struct cmd_context *cmd, const char *lvid_s,
        if (!activation())
                return 1;
 
-       if (!(lv = lv_from_lvid(cmd, lvid_s)))
+       if (!(lv = lv_from_lvid(cmd, lvid_s, 0)))
                return 0;
 
        if (test_mode()) {
@@ -662,7 +663,7 @@ int lv_deactivate(struct cmd_context *cmd, const char *lvid_s)
        if (!activation())
                return 1;
 
-       if (!(lv = lv_from_lvid(cmd, lvid_s)))
+       if (!(lv = lv_from_lvid(cmd, lvid_s, 0)))
                return 0;
 
        if (test_mode()) {
@@ -701,7 +702,7 @@ int lv_activation_filter(struct cmd_context *cmd, const char *lvid_s,
        if (!activation())
                goto activate;
 
-       if (!(lv = lv_from_lvid(cmd, lvid_s)))
+       if (!(lv = lv_from_lvid(cmd, lvid_s, 0)))
                return 0;
 
        if (!_passes_activation_filter(cmd, lv)) {
@@ -726,7 +727,7 @@ static int _lv_activate(struct cmd_context *cmd, const char *lvid_s,
        if (!activation())
                return 1;
 
-       if (!(lv = lv_from_lvid(cmd, lvid_s)))
+       if (!(lv = lv_from_lvid(cmd, lvid_s, 0)))
                return 0;
 
        if (filter && !_passes_activation_filter(cmd, lv)) {

diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index 01a7e5ce9031e3aeebe846c30f3379a06b93e323..07b0ddb0d19b3e057341d5f8e1507577beaae2f0 100644 (file)
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -187,17 +187,22 @@ static int _raw_write_mda_header(const struct format_type *fmt,
 static struct raw_locn *_find_vg_rlocn(struct device_area *dev_area,
                                       struct mda_header *mdah,
                                       const char *vgname,
-                                      int precommit)
+                                      int *precommitted)
 {
        size_t len;
        char vgnamebuf[NAME_LEN + 2];
-       struct raw_locn *rlocn;
+       struct raw_locn *rlocn, *rlocn_precommitted;
        struct lvmcache_info *info;
 
        rlocn = mdah->raw_locns;        /* Slot 0 */
+       rlocn_precommitted = rlocn + 1; /* Slot 1 */
 
-       if (precommit)
-               rlocn++;                /* Slot 1 */
+       /* Should we use precommitted metadata? */
+       if (*precommitted && rlocn_precommitted->size &&
+           (rlocn_precommitted->offset != rlocn->offset)) {
+               rlocn = rlocn_precommitted;
+       } else
+               *precommitted = 0;
 
        /* FIXME Loop through rlocns two-at-a-time.  List null-terminated. */
        /* FIXME Ignore if checksum incorrect!!! */
@@ -241,6 +246,7 @@ static int _raw_holds_vgname(struct format_instance *fid,
                             struct device_area *dev_area, const char *vgname)
 {
        int r = 0;
+       int noprecommit = 0;
        struct mda_header *mdah;
 
        if (!dev_open(dev_area->dev)) {
@@ -253,7 +259,7 @@ static int _raw_holds_vgname(struct format_instance *fid,
                return 0;
        }
 
-       if (_find_vg_rlocn(dev_area, mdah, vgname, 0))
+       if (_find_vg_rlocn(dev_area, mdah, vgname, &noprecommit))
                r = 1;
 
        if (!dev_close(dev_area->dev))
@@ -265,7 +271,7 @@ static int _raw_holds_vgname(struct format_instance *fid,
 static struct volume_group *_vg_read_raw_area(struct format_instance *fid,
                                              const char *vgname,
                                              struct device_area *area,
-                                             int precommit)
+                                             int precommitted)
 {
        struct volume_group *vg = NULL;
        struct raw_locn *rlocn;
@@ -284,7 +290,7 @@ static struct volume_group *_vg_read_raw_area(struct format_instance *fid,
                goto out;
        }
 
-       if (!(rlocn = _find_vg_rlocn(area, mdah, vgname, precommit))) {
+       if (!(rlocn = _find_vg_rlocn(area, mdah, vgname, &precommitted))) {
                log_debug("VG %s not found on %s", vgname, dev_name(area->dev));
                goto out;
        }
@@ -309,10 +315,13 @@ static struct volume_group *_vg_read_raw_area(struct format_instance *fid,
                goto out;
        }
        log_debug("Read %s %smetadata (%u) from %s at %" PRIu64 " size %"
-                 PRIu64, vg->name, precommit ? "pre-commit " : "",
+                 PRIu64, vg->name, precommitted ? "pre-commit " : "",
                  vg->seqno, dev_name(area->dev),
                  area->start + rlocn->offset, rlocn->size);
 
+       if (precommitted)
+               vg->status |= PRECOMMITTED;
+
       out:
        if (!dev_close(area->dev))
                stack;
@@ -349,6 +358,7 @@ static int _vg_write_raw(struct format_instance *fid, struct volume_group *vg,
        int r = 0;
        uint32_t new_wrap = 0, old_wrap = 0;
        int found = 0;
+       int noprecommit = 0;
 
        /* Ignore any mda on a PV outside the VG. vgsplit relies on this */
        list_iterate_items(pvl, &vg->pvs) {
@@ -371,7 +381,7 @@ static int _vg_write_raw(struct format_instance *fid, struct volume_group *vg,
                goto out;
        }
 
-       rlocn = _find_vg_rlocn(&mdac->area, mdah, vg->name, 0);
+       rlocn = _find_vg_rlocn(&mdac->area, mdah, vg->name, &noprecommit);
        mdac->rlocn.offset = _next_rlocn_offset(rlocn, mdah);
 
        if (!fidtc->raw_metadata_buf &&
@@ -456,6 +466,7 @@ static int _vg_commit_raw_rlocn(struct format_instance *fid,
        struct pv_list *pvl;
        int r = 0;
        int found = 0;
+       int noprecommit = 0;
 
        /* Ignore any mda on a PV outside the VG. vgsplit relies on this */
        list_iterate_items(pvl, &vg->pvs) {
@@ -473,23 +484,41 @@ static int _vg_commit_raw_rlocn(struct format_instance *fid,
                goto out;
        }
 
-       if (!(rlocn = _find_vg_rlocn(&mdac->area, mdah, vg->name, 0))) {
+       if (!(rlocn = _find_vg_rlocn(&mdac->area, mdah, vg->name, &noprecommit))) {
                mdah->raw_locns[0].offset = 0;
+               mdah->raw_locns[0].size = 0;
+               mdah->raw_locns[0].checksum = 0;
                mdah->raw_locns[1].offset = 0;
+               mdah->raw_locns[1].size = 0;
+               mdah->raw_locns[1].checksum = 0;
                mdah->raw_locns[2].offset = 0;
+               mdah->raw_locns[2].size = 0;
+               mdah->raw_locns[2].checksum = 0;
                rlocn = &mdah->raw_locns[0];
        }
 
        if (precommit)
                rlocn++;
+       else {
+               /* If not precommitting, wipe the precommitted rlocn */
+               mdah->raw_locns[1].offset = 0;
+               mdah->raw_locns[1].size = 0;
+               mdah->raw_locns[1].checksum = 0;
+       }
+
+       /* Is there new metadata to commit? */
+       if (mdac->rlocn.size) {
+               rlocn->offset = mdac->rlocn.offset;
+               rlocn->size = mdac->rlocn.size;
+               rlocn->checksum = mdac->rlocn.checksum;
+               log_debug("%sCommitting %s metadata (%u) to %s header at %"
+                         PRIu64, precommit ? "Pre-" : "", vg->name, vg->seqno,
+                         dev_name(mdac->area.dev), mdac->area.start);
+       } else
+               log_debug("Wiping pre-committed %s metadata from %s "
+                         "header at %" PRIu64, vg->name,
+                         dev_name(mdac->area.dev), mdac->area.start);
 
-       rlocn->offset = mdac->rlocn.offset;
-       rlocn->size = mdac->rlocn.size;
-       rlocn->checksum = mdac->rlocn.checksum;
-
-       log_debug("%sCommitting %s metadata (%u) to %s header at %" PRIu64,
-                 precommit ? "Pre-" : "", vg->name, vg->seqno,
-                 dev_name(mdac->area.dev), mdac->area.start);
        if (!_raw_write_mda_header(fid->fmt, mdac->area.dev, mdac->area.start,
                                   mdah)) {
                log_error("Failed to write metadata area header");
@@ -529,7 +558,6 @@ static int _vg_revert_raw(struct format_instance *fid, struct volume_group *vg,
                          struct metadata_area *mda)
 {
        struct mda_context *mdac = (struct mda_context *) mda->metadata_locn;
-       struct text_fid_context *fidtc = (struct text_fid_context *) fid->private;
        struct pv_list *pvl;
        int found = 0;
 
@@ -544,15 +572,9 @@ static int _vg_revert_raw(struct format_instance *fid, struct volume_group *vg,
        if (!found)
                return 1;
 
-       if (!dev_close(mdac->area.dev))
-               stack;
-
-       if (fidtc->raw_metadata_buf) {
-               dm_free(fidtc->raw_metadata_buf);
-               fidtc->raw_metadata_buf = NULL;
-       }
-
-       return 1;
+       /* Wipe pre-committed metadata */
+       mdac->rlocn.size = 0;
+       return _vg_commit_raw_rlocn(fid, vg, mda, 0);
 }
 
 static int _vg_remove_raw(struct format_instance *fid, struct volume_group *vg,
@@ -562,6 +584,7 @@ static int _vg_remove_raw(struct format_instance *fid, struct volume_group *vg,
        struct mda_header *mdah;
        struct raw_locn *rlocn;
        int r = 0;
+       int noprecommit = 0;
 
        if (!dev_open(mdac->area.dev)) {
                stack;
@@ -573,7 +596,7 @@ static int _vg_remove_raw(struct format_instance *fid, struct volume_group *vg,
                goto out;
        }
 
-       if (!(rlocn = _find_vg_rlocn(&mdac->area, mdah, vg->name, 0))) {
+       if (!(rlocn = _find_vg_rlocn(&mdac->area, mdah, vg->name, &noprecommit))) {
                rlocn = &mdah->raw_locns[0];
                mdah->raw_locns[1].offset = 0;
        }
@@ -640,8 +663,14 @@ static struct volume_group *_vg_read_precommit_file(struct format_instance *fid,
                                                    struct metadata_area *mda)
 {
        struct text_context *tc = (struct text_context *) mda->metadata_locn;
+       struct volume_group *vg;
+
+       if ((vg = _vg_read_file_name(fid, vgname, tc->path_edit)))
+               vg->status |= PRECOMMITTED;
+       else
+               vg = _vg_read_file_name(fid, vgname, tc->path_live);
 
-       return _vg_read_file_name(fid, vgname, tc->path_edit);
+       return vg;
 }
 
 static int _vg_write_file(struct format_instance *fid, struct volume_group *vg,

diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 0be839391439f37b5be22bcc0d337a2cf5b28814..9a3ff15275979cb370ff7d35d607e571d7959ad5 100644 (file)
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -902,6 +902,8 @@ static struct volume_group *_vg_read_orphans(struct cmd_context *cmd)
  * If consistent is 0, caller must check whether consistent == 1 on return
  * and take appropriate action if it isn't (e.g. abort; get write lock 
  * and call vg_read again).
+ *
+ * If precommitted is set, use precommitted metadata if present.
  */
 static struct volume_group *_vg_read(struct cmd_context *cmd,
                                     const char *vgname,
@@ -912,9 +914,10 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
        struct volume_group *vg, *correct_vg = NULL;
        struct metadata_area *mda;
        int inconsistent = 0;
+       int use_precommitted = precommitted;
 
        if (!*vgname) {
-               if (precommitted) {
+               if (use_precommitted) {
                        log_error("Internal error: vg_read requires vgname "
                                  "with pre-commit.");
                        return NULL;
@@ -940,11 +943,8 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
                }
        }
 
-       if (precommitted && !(fmt->features & FMT_PRECOMMIT)) {
-               log_error("Internal error: %s doesn't support "
-                         "pre-commit", fmt->name);
-               return NULL;
-       }
+       if (use_precommitted && !(fmt->features & FMT_PRECOMMIT))
+               use_precommitted = 0;
 
        /* create format instance with appropriate metadata area */
        if (!(fid = fmt->ops->create_instance(fmt, vgname, NULL))) {
@@ -954,9 +954,9 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
 
        /* Ensure contents of all metadata areas match - else do recovery */
        list_iterate_items(mda, &fid->metadata_areas) {
-               if ((precommitted &&
+               if ((use_precommitted &&
                     !(vg = mda->ops->vg_read_precommit(fid, vgname, mda))) ||
-                   (!precommitted &&
+                   (!use_precommitted &&
                     !(vg = mda->ops->vg_read(fid, vgname, mda)))) {
                        inconsistent = 1;
                        continue;
@@ -983,11 +983,8 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
                        return NULL;
                }
 
-               if (precommitted && !(fmt->features & FMT_PRECOMMIT)) {
-                       log_error("Internal error: %s doesn't support "
-                                 "pre-commit", fmt->name);
-                       return NULL;
-               }
+               if (precommitted && !(fmt->features & FMT_PRECOMMIT))
+                       use_precommitted = 0;
 
                /* create format instance with appropriate metadata area */
                if (!(fid = fmt->ops->create_instance(fmt, vgname, NULL))) {
@@ -997,10 +994,10 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
 
                /* Ensure contents of all metadata areas match - else recover */
                list_iterate_items(mda, &fid->metadata_areas) {
-                       if ((precommitted &&
+                       if ((use_precommitted &&
                             !(vg = mda->ops->vg_read_precommit(fid, vgname,
                                                                mda))) ||
-                           (!precommitted &&
+                           (!use_precommitted &&
                             !(vg = mda->ops->vg_read(fid, vgname, mda)))) {
                                inconsistent = 1;
                                continue;
@@ -1027,7 +1024,8 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
        lvmcache_update_vg(correct_vg);
 
        if (inconsistent) {
-               if (precommitted) {
+               /* FIXME Test should be if we're *using* precommitted metadata not if we were searching for it */
+               if (use_precommitted) {
                        log_error("Inconsistent pre-commit metadata copies "
                                  "for volume group %s", vgname);
                        return NULL;
@@ -1096,18 +1094,13 @@ struct volume_group *vg_read(struct cmd_context *cmd, const char *vgname,
        return vg;
 }
 
-struct volume_group *vg_read_precommitted(struct cmd_context *cmd,
-                                         const char *vgname,
-                                         int *consistent)
-{
-       return _vg_read(cmd, vgname, consistent, 1);
-}
-
 /* This is only called by lv_from_lvid, which is only called from 
  * activate.c so we know the appropriate VG lock is already held and 
  * the vg_read is therefore safe.
  */
-struct volume_group *vg_read_by_vgid(struct cmd_context *cmd, const char *vgid)
+static struct volume_group *_vg_read_by_vgid(struct cmd_context *cmd,
+                                           const char *vgid,
+                                           int precommitted)
 {
        const char *vgname;
        struct list *vgnames;
@@ -1119,7 +1112,8 @@ struct volume_group *vg_read_by_vgid(struct cmd_context *cmd, const char *vgid)
        /* Is corresponding vgname already cached? */
        if ((vginfo = vginfo_from_vgid(vgid)) &&
            vginfo->vgname && *vginfo->vgname) {
-               if ((vg = vg_read(cmd, vginfo->vgname, &consistent)) &&
+               if ((vg = _vg_read(cmd, vginfo->vgname,
+                                  &consistent, precommitted)) &&
                    !strncmp(vg->id.uuid, vgid, ID_LEN)) {
                        if (!consistent) {
                                log_error("Volume group %s metadata is "
@@ -1149,7 +1143,8 @@ struct volume_group *vg_read_by_vgid(struct cmd_context *cmd, const char *vgid)
                if (!vgname || !*vgname)
                        continue;       // FIXME Unnecessary? 
                consistent = 0;
-               if ((vg = vg_read(cmd, vgname, &consistent)) &&
+               if ((vg = _vg_read(cmd, vgname, &consistent,
+                                  precommitted)) &&
                    !strncmp(vg->id.uuid, vgid, ID_LEN)) {
                        if (!consistent) {
                                log_error("Volume group %s metadata is "
@@ -1164,7 +1159,8 @@ struct volume_group *vg_read_by_vgid(struct cmd_context *cmd, const char *vgid)
 }
 
 /* Only called by activate.c */
-struct logical_volume *lv_from_lvid(struct cmd_context *cmd, const char *lvid_s)
+struct logical_volume *lv_from_lvid(struct cmd_context *cmd, const char *lvid_s,
+                                   int precommitted)
 {
        struct lv_list *lvl;
        struct volume_group *vg;
@@ -1173,7 +1169,7 @@ struct logical_volume *lv_from_lvid(struct cmd_context *cmd, const char *lvid_s)
        lvid = (const union lvid *) lvid_s;
 
        log_very_verbose("Finding volume group for uuid %s", lvid_s);
-       if (!(vg = vg_read_by_vgid(cmd, lvid->id[0].uuid))) {
+       if (!(vg = _vg_read_by_vgid(cmd, lvid->id[0].uuid, precommitted))) {
                log_error("Volume group for uuid not found: %s", lvid_s);
                return NULL;
        }

diff --git a/lib/metadata/metadata.h b/lib/metadata/metadata.h
index 4ca82a257c1fc1de0471f16695fc69f2c3096a84..0ffde9b1e492e66f8a234c46d2d1e4bc09e66234 100644 (file)
--- a/lib/metadata/metadata.h
+++ b/lib/metadata/metadata.h
@@ -404,10 +404,6 @@ int vg_commit(struct volume_group *vg);
 int vg_revert(struct volume_group *vg);
 struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name,
                             int *consistent);
-// struct volume_group *vg_read_precommitted(struct cmd_context *cmd,
-//                                       const char *vg_name,
-//                                       int *consistent);
-// struct volume_group *vg_read_by_vgid(struct cmd_context *cmd, const char *vgid);
 struct physical_volume *pv_read(struct cmd_context *cmd, const char *pv_name,
                                struct list *mdas, uint64_t *label_sector,
                                int warnings);
@@ -495,7 +491,8 @@ struct volume_group *find_vg_with_lv(const char *lv_name);
 
 /* Find LV with given lvid (used during activation) */
 struct logical_volume *lv_from_lvid(struct cmd_context *cmd,
-                                   const char *lvid_s);
+                                   const char *lvid_s,
+                                   int precommitted);
 
 /* FIXME Merge these functions with ones above */
 struct physical_volume *find_pv(struct volume_group *vg, struct device *dev);