Fix automatic text metadata buffer expansion (using macro). [stdarg usage bug]

Cache formatted text metadata buffer between metadata area writes.
  [improves write performance when lots of metadata area clones]

diff --git a/WHATS_NEW b/WHATS_NEW
index e2e06c08adf119fea0468b8995d5606438288645..099ae04508e7ed4f77a5298976420a6ad3635087 100644 (file)
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,7 @@
 Version 2.02.00 - 
 ===================================
+  Fix automatic text metadata buffer expansion (using macro).
+  Cache formatted text metadata buffer between metadata area writes.
   Add pe_start field to pvs.
   Add 'LVM-' prefix to uuids.
   Split lv_segment_area from lv_segment to permit extension.

diff --git a/lib/format_text/export.c b/lib/format_text/export.c
index a0a55e621105c6884c51a5743c27633379082f3f..69f0f60e1d00206f9d7d3b34e178c1340e8ac8cf 100644 (file)
--- a/lib/format_text/export.c
+++ b/lib/format_text/export.c
@@ -29,6 +29,19 @@ struct formatter;
 typedef int (*out_with_comment_fn) (struct formatter * f, const char *comment,
                                    const char *fmt, va_list ap);
 typedef int (*nl_fn) (struct formatter * f);
+
+/*
+ * Macro for formatted output.
+ * out_with_comment_fn returns -1 if data didn't fit and buffer was expanded.
+ * Then argument list is reset and out_with_comment_fn is called again.
+ */
+#define _out_with_comment(f, buffer, fmt, ap) \
+        do { \
+               va_start(ap, fmt); \
+               r = f->out_with_comment(f, buffer, fmt, ap); \
+               va_end(ap); \
+       } while (r == -1)
+
 /*
  * The first half of this file deals with
  * exporting the vg, ie. writing it to a file.
@@ -100,19 +113,30 @@ static int _nl_file(struct formatter *f)
        return 1;
 }
 
-static int _nl_raw(struct formatter *f)
+static int _extend_buffer(struct formatter *f)
 {
        char *newbuf;
 
-       /* If metadata doesn't fit, double the buffer size */
-       if (f->data.buf.used + 2 > f->data.buf.size) {
-               if (!(newbuf = dm_realloc(f->data.buf.start,
-                                          f->data.buf.size * 2))) {
-                       stack;
-                       return 0;
-               }
-               f->data.buf.start = newbuf;
-               f->data.buf.size *= 2;
+       log_debug("Doubling metadata output buffer to %" PRIu32,
+                 f->data.buf.size * 2);
+       if (!(newbuf = dm_realloc(f->data.buf.start,
+                                  f->data.buf.size * 2))) {
+               log_error("Buffer reallocation failed.");
+               return 0;
+       }
+       f->data.buf.start = newbuf;
+       f->data.buf.size *= 2;
+
+       return 1;
+}
+
+static int _nl_raw(struct formatter *f)
+{
+       /* If metadata doesn't fit, extend buffer */
+       if ((f->data.buf.used + 2 > f->data.buf.size) &&
+           (!_extend_buffer(f))) {
+               stack;
+               return 0;
        }
 
        *(f->data.buf.start + f->data.buf.used) = '\n';
@@ -163,22 +187,17 @@ static int _out_with_comment_raw(struct formatter *f, const char *comment,
                                 const char *fmt, va_list ap)
 {
        int n;
-       char *newbuf;
 
-      retry:
        n = vsnprintf(f->data.buf.start + f->data.buf.used,
                      f->data.buf.size - f->data.buf.used, fmt, ap);
 
-       /* If metadata doesn't fit, double the buffer size */
+       /* If metadata doesn't fit, extend buffer */
        if (n < 0 || (n + f->data.buf.used + 2 > f->data.buf.size)) {
-               if (!(newbuf = dm_realloc(f->data.buf.start,
-                                          f->data.buf.size * 2))) {
+               if (!_extend_buffer(f)) {
                        stack;
                        return 0;
                }
-               f->data.buf.start = newbuf;
-               f->data.buf.size *= 2;
-               goto retry;
+               return -1; /* Retry */
        }
 
        f->data.buf.used += n;
@@ -229,9 +248,7 @@ int out_size(struct formatter *f, uint64_t size, const char *fmt, ...)
        if (!_sectors_to_units(size, buffer, sizeof(buffer)))
                return 0;
 
-       va_start(ap, fmt);
-       r = f->out_with_comment(f, buffer, fmt, ap);
-       va_end(ap);
+       _out_with_comment(f, buffer, fmt, ap);
 
        return r;
 }
@@ -245,9 +262,7 @@ int out_hint(struct formatter *f, const char *fmt, ...)
        va_list ap;
        int r;
 
-       va_start(ap, fmt);
-       r = f->out_with_comment(f, "# Hint only", fmt, ap);
-       va_end(ap);
+       _out_with_comment(f, "# Hint only", fmt, ap);
 
        return r;
 }
@@ -260,9 +275,7 @@ int out_text(struct formatter *f, const char *fmt, ...)
        va_list ap;
        int r;
 
-       va_start(ap, fmt);
-       r = f->out_with_comment(f, NULL, fmt, ap);
-       va_end(ap);
+       _out_with_comment(f, NULL, fmt, ap);
 
        return r;
 }

diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index 635c1b337510a2b3dd00683d46e964f9a2283739..9c605b1fc81b6c47be51e2e3efe2099ad807fbe6 100644 (file)
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -43,6 +43,11 @@ static struct format_instance *_create_text_instance(const struct format_type
                                                     *fmt, const char *vgname,
                                                     void *context);
 
+struct text_fid_context {
+       char *raw_metadata_buf;
+       uint32_t raw_metadata_buf_size;
+};
+
 struct dir_list {
        struct list list;
        char dir[0];
@@ -337,12 +342,12 @@ static int _vg_write_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 raw_locn *rlocn;
        struct mda_header *mdah;
        struct pv_list *pvl;
        int r = 0;
        uint32_t new_wrap = 0, old_wrap = 0;
-       char *buf = NULL;
        int found = 0;
 
        /* Ignore any mda on a PV outside the VG. vgsplit relies on this */
@@ -369,11 +374,15 @@ static int _vg_write_raw(struct format_instance *fid, struct volume_group *vg,
        rlocn = _find_vg_rlocn(&mdac->area, mdah, vg->name, 0);
        mdac->rlocn.offset = _next_rlocn_offset(rlocn, mdah);
 
-       if (!(mdac->rlocn.size = text_vg_export_raw(vg, "", &buf))) {
+       if (!fidtc->raw_metadata_buf &&
+           !(fidtc->raw_metadata_buf_size =
+                       text_vg_export_raw(vg, "", &fidtc->raw_metadata_buf))) {
                log_error("VG %s metadata writing failed", vg->name);
                goto out;
        }
 
+       mdac->rlocn.size = fidtc->raw_metadata_buf_size;
+
        if (mdac->rlocn.offset + mdac->rlocn.size > mdah->size)
                new_wrap = (mdac->rlocn.offset + mdac->rlocn.size) - mdah->size;
 
@@ -396,7 +405,8 @@ static int _vg_write_raw(struct format_instance *fid, struct volume_group *vg,
 
        /* Write text out, circularly */
        if (!dev_write(mdac->area.dev, mdac->area.start + mdac->rlocn.offset,
-                      (size_t) (mdac->rlocn.size - new_wrap), buf)) {
+                      (size_t) (mdac->rlocn.size - new_wrap),
+                      fidtc->raw_metadata_buf)) {
                stack;
                goto out;
        }
@@ -409,18 +419,20 @@ static int _vg_write_raw(struct format_instance *fid, struct volume_group *vg,
                if (!dev_write(mdac->area.dev,
                               mdac->area.start + MDA_HEADER_SIZE,
                               (size_t) new_wrap,
-                              buf + mdac->rlocn.size - new_wrap)) {
+                              fidtc->raw_metadata_buf + 
+                              mdac->rlocn.size - new_wrap)) {
                        stack;
                        goto out;
                }
        }
 
-       mdac->rlocn.checksum = calc_crc(INITIAL_CRC, buf,
+       mdac->rlocn.checksum = calc_crc(INITIAL_CRC, fidtc->raw_metadata_buf,
                                        (uint32_t) (mdac->rlocn.size -
                                                    new_wrap));
        if (new_wrap)
                mdac->rlocn.checksum = calc_crc(mdac->rlocn.checksum,
-                                               buf + mdac->rlocn.size -
+                                               fidtc->raw_metadata_buf +
+                                               mdac->rlocn.size -
                                                new_wrap, new_wrap);
 
        r = 1;
@@ -429,8 +441,6 @@ static int _vg_write_raw(struct format_instance *fid, struct volume_group *vg,
        if (!r && !dev_close(mdac->area.dev))
                stack;
 
-       if (buf)
-               dm_free(buf);
        return r;
 }
 
@@ -440,6 +450,7 @@ static int _vg_commit_raw_rlocn(struct format_instance *fid,
                                int precommit)
 {
        struct mda_context *mdac = (struct mda_context *) mda->metadata_locn;
+       struct text_fid_context *fidtc = (struct text_fid_context *) fid->private;
        struct mda_header *mdah;
        struct raw_locn *rlocn;
        struct pv_list *pvl;
@@ -488,8 +499,14 @@ static int _vg_commit_raw_rlocn(struct format_instance *fid,
        r = 1;
 
       out:
-       if (!precommit && !dev_close(mdac->area.dev))
-               stack;
+       if (!precommit) {
+               if (!dev_close(mdac->area.dev))
+                       stack;
+               if (fidtc->raw_metadata_buf) {
+                       dm_free(fidtc->raw_metadata_buf);
+                       fidtc->raw_metadata_buf = NULL;
+               }
+       }
 
        return r;
 }
@@ -512,6 +529,7 @@ 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;
 
@@ -529,6 +547,11 @@ static int _vg_revert_raw(struct format_instance *fid, struct volume_group *vg,
        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;
 }
 
@@ -1401,6 +1424,7 @@ static struct format_instance *_create_text_instance(const struct format_type
                                                     void *context)
 {
        struct format_instance *fid;
+       struct text_fid_context *fidtc;
        struct metadata_area *mda, *mda_new;
        struct mda_context *mdac, *mdac_new;
        struct dir_list *dl;
@@ -1415,8 +1439,16 @@ static struct format_instance *_create_text_instance(const struct format_type
                return NULL;
        }
 
-       fid->fmt = fmt;
+       if (!(fidtc = (struct text_fid_context *)
+                       dm_pool_zalloc(fmt->cmd->mem,sizeof(*fidtc)))) {
+               log_error("Couldn't allocate text_fid_context.");
+               return NULL;
+       }
+
+       fidtc->raw_metadata_buf = NULL;
+       fid->private = (void *) fidtc;
 
+       fid->fmt = fmt;
        list_init(&fid->metadata_areas);
 
        if (!vgname) {

diff --git a/lib/metadata/metadata.h b/lib/metadata/metadata.h
index 7a87504e9015f5c907f64673efe33ea67f2be647..7eaa231ad501ba1cb1013d6d3d74cd1915b2181c 100644 (file)
--- a/lib/metadata/metadata.h
+++ b/lib/metadata/metadata.h
@@ -181,6 +181,7 @@ struct metadata_area {
 struct format_instance {
        const struct format_type *fmt;
        struct list metadata_areas;     /* e.g. metadata locations */
+       void *private;
 };
 
 struct volume_group {