- rrd_open() calculates file size for new files and calls mmap once for

the whole file

- rrd_resize() cleaned up, no longer passing a size through the cookie
argument

- rrd_init(&my_rrd) must be called before rrd_open() - if people are
calling rrd_open directly from application code, this might be
troublesome.  Alternative solutions: creating an additional function,
rrd_open_create(), or adding an extra argument to rrd_open() for setting
the file size

-- Daniel Pocock

git-svn-id: svn://svn.oetiker.ch/rrdtool/trunk/program@1613 a5681a0c-68f1-0310-ab6d-d61299d08faa

diff --git a/src/rrd.h b/src/rrd.h
index d87275e..34ce78e 100644 (file)
--- a/src/rrd.h
+++ b/src/rrd.h
@@ -84,6 +84,10 @@ extern    "C" {
         off_t     header_len;   /* length of the header of this rrd file */
         off_t     file_len; /* total size of the rrd file */
         off_t     pos;  /* current pos in file */
+#ifdef HAVE_MMAP
+        int       mm_prot;
+        int       mm_flags;
+#endif
     } rrd_file_t;
 
 /* rrd info interface */

diff --git a/src/rrd_create.c b/src/rrd_create.c
index 9ee6896..c48900f 100644 (file)
--- a/src/rrd_create.c
+++ b/src/rrd_create.c
@@ -229,7 +229,7 @@ int rrd_create_r(
             rrd.stat_head->ds_cnt++;
         } else if (strncmp(argv[i], "RRA:", 4) == 0) {
             char     *argvcopy;
-            char     *tokptr;
+            char     *tokptr = "";
             size_t    old_size = sizeof(rra_def_t) * (rrd.stat_head->rra_cnt);
             int       row_cnt;
 
@@ -671,36 +671,35 @@ int rrd_create_fn(
     rrd_t *rrd)
 {
     unsigned long i, ii;
-    int       rrd_file;
     rrd_value_t *unknown;
     int       unkn_cnt;
     rrd_file_t *rrd_file_dn;
     rrd_t     rrd_dn;
-    unsigned  flags = O_WRONLY | O_CREAT | O_TRUNC;
+    unsigned  rrd_flags = RRD_READWRITE | RRD_CREAT;
 
-#if defined(_WIN32) && !defined(__CYGWIN__) && !defined(__CYGWIN32__)
-    flags |= O_BINARY;
-#endif
+    unkn_cnt = 0;
+    for (i = 0; i < rrd->stat_head->rra_cnt; i++)
+        unkn_cnt += rrd->stat_head->ds_cnt * rrd->rra_def[i].row_cnt;
 
-    if ((rrd_file = open(file_name, flags, 0666)) < 0) {
+    if ((rrd_file_dn = rrd_open(file_name, rrd, rrd_flags)) == NULL) {
         rrd_set_error("creating '%s': %s", file_name, rrd_strerror(errno));
         rrd_free2(rrd);
         return (-1);
     }
 
-    write(rrd_file, rrd->stat_head, sizeof(stat_head_t));
+    rrd_write(rrd_file_dn, rrd->stat_head, sizeof(stat_head_t));
 
-    write(rrd_file, rrd->ds_def, sizeof(ds_def_t) * rrd->stat_head->ds_cnt);
+    rrd_write(rrd_file_dn, rrd->ds_def, sizeof(ds_def_t) * rrd->stat_head->ds_cnt);
 
-    write(rrd_file, rrd->rra_def,
+    rrd_write(rrd_file_dn, rrd->rra_def,
           sizeof(rra_def_t) * rrd->stat_head->rra_cnt);
 
-    write(rrd_file, rrd->live_head, sizeof(live_head_t));
+    rrd_write(rrd_file_dn, rrd->live_head, sizeof(live_head_t));
 
     if ((rrd->pdp_prep = calloc(1, sizeof(pdp_prep_t))) == NULL) {
         rrd_set_error("allocating pdp_prep");
         rrd_free2(rrd);
-        close(rrd_file);
+        rrd_close(rrd_file_dn);
         return (-1);
     }
 
@@ -711,12 +710,12 @@ int rrd_create_fn(
         rrd->live_head->last_up % rrd->stat_head->pdp_step;
 
     for (i = 0; i < rrd->stat_head->ds_cnt; i++)
-        write(rrd_file, rrd->pdp_prep, sizeof(pdp_prep_t));
+        rrd_write(rrd_file_dn, rrd->pdp_prep, sizeof(pdp_prep_t));
 
     if ((rrd->cdp_prep = calloc(1, sizeof(cdp_prep_t))) == NULL) {
         rrd_set_error("allocating cdp_prep");
         rrd_free2(rrd);
-        close(rrd_file);
+        rrd_close(rrd_file_dn);
         return (-1);
     }
 
@@ -753,7 +752,7 @@ int rrd_create_fn(
         }
 
         for (ii = 0; ii < rrd->stat_head->ds_cnt; ii++) {
-            write(rrd_file, rrd->cdp_prep, sizeof(cdp_prep_t));
+            rrd_write(rrd_file_dn, rrd->cdp_prep, sizeof(cdp_prep_t));
         }
     }
 
@@ -763,7 +762,7 @@ int rrd_create_fn(
     if ((rrd->rra_ptr = calloc(1, sizeof(rra_ptr_t))) == NULL) {
         rrd_set_error("allocating rra_ptr");
         rrd_free2(rrd);
-        close(rrd_file);
+        rrd_close(rrd_file_dn);
         return (-1);
     }
 
@@ -773,40 +772,42 @@ int rrd_create_fn(
      * the pointer a priori. */
     for (i = 0; i < rrd->stat_head->rra_cnt; i++) {
         rrd->rra_ptr->cur_row = rra_random_row(&rrd->rra_def[i]);
-        write(rrd_file, rrd->rra_ptr, sizeof(rra_ptr_t));
+        rrd_write(rrd_file_dn, rrd->rra_ptr, sizeof(rra_ptr_t));
     }
 
     /* write the empty data area */
     if ((unknown = (rrd_value_t *) malloc(512 * sizeof(rrd_value_t))) == NULL) {
         rrd_set_error("allocating unknown");
         rrd_free2(rrd);
-        close(rrd_file);
+        rrd_close(rrd_file_dn);
         return (-1);
     }
     for (i = 0; i < 512; ++i)
         unknown[i] = DNAN;
 
-    unkn_cnt = 0;
-    for (i = 0; i < rrd->stat_head->rra_cnt; i++)
-        unkn_cnt += rrd->stat_head->ds_cnt * rrd->rra_def[i].row_cnt;
-
     while (unkn_cnt > 0) {
-        write(rrd_file, unknown, sizeof(rrd_value_t) * min(unkn_cnt, 512));
+        if(rrd_write(rrd_file_dn, unknown, sizeof(rrd_value_t) * min(unkn_cnt, 512)) < 0)
+        {
+            rrd_set_error("creating rrd: %s", rrd_strerror(errno));
+            return -1;
+        }
 
         unkn_cnt -= 512;
     }
     free(unknown);
-    fdatasync(rrd_file);
     rrd_free2(rrd);
-    if (close(rrd_file) == -1) {
+    if (rrd_close(rrd_file_dn) == -1) {
         rrd_set_error("creating rrd: %s", rrd_strerror(errno));
         return -1;
     }
     /* flush all we don't need out of the cache */
-    rrd_file_dn = rrd_open(file_name, &rrd_dn, RRD_READONLY);
-    rrd_dontneed(rrd_file_dn, &rrd_dn);
-    rrd_free(&rrd_dn);
-    rrd_close(rrd_file_dn);
+    rrd_init(&rrd_dn);
+    if((rrd_file_dn = rrd_open(file_name, &rrd_dn, RRD_READONLY)) != NULL)
+    {
+        rrd_dontneed(rrd_file_dn, &rrd_dn);
+        /* rrd_free(&rrd_dn); */
+        rrd_close(rrd_file_dn);
+    }
     return (0);
 }
 

diff --git a/src/rrd_dump.c b/src/rrd_dump.c
index 3f79a96..aa24882 100644 (file)
--- a/src/rrd_dump.c
+++ b/src/rrd_dump.c
@@ -65,6 +65,7 @@ static int rrd_dump_opt_r(
     rrd_value_t value;
     struct tm tm;
 
+    rrd_init(&rrd);
     rrd_file = rrd_open(filename, &rrd, RRD_READONLY | RRD_READAHEAD);
     if (rrd_file == NULL) {
         rrd_free(&rrd);

diff --git a/src/rrd_fetch.c b/src/rrd_fetch.c
index 568e262..d821dfa 100644 (file)
--- a/src/rrd_fetch.c
+++ b/src/rrd_fetch.c
@@ -235,6 +235,7 @@ int rrd_fetch_fn(
             *start, *end, *step);
 #endif
 
+    rrd_init(&rrd);
     rrd_file = rrd_open(filename, &rrd, RRD_READONLY);
     if (rrd_file == NULL)
         goto err_free;

diff --git a/src/rrd_first.c b/src/rrd_first.c
index 07c5a44..82b99f3 100644 (file)
--- a/src/rrd_first.c
+++ b/src/rrd_first.c
@@ -65,6 +65,7 @@ time_t rrd_first_r(
     rrd_t     rrd;
     rrd_file_t *rrd_file;
 
+    rrd_init(&rrd);
     rrd_file = rrd_open(filename, &rrd, RRD_READONLY);
     if (rrd_file == NULL) {
         goto err_free;

diff --git a/src/rrd_info.c b/src/rrd_info.c
index 679b024..3b5364a 100644 (file)
--- a/src/rrd_info.c
+++ b/src/rrd_info.c
@@ -146,6 +146,7 @@ rrd_info_t *rrd_info_r(
     enum cf_en current_cf;
     enum dst_en current_ds;
 
+    rrd_init(&rrd);
     rrd_file = rrd_open(filename, &rrd, RRD_READONLY);
     if (rrd_file == NULL)
         goto err_free;

diff --git a/src/rrd_last.c b/src/rrd_last.c
index 517b65a..09879c1 100644 (file)
--- a/src/rrd_last.c
+++ b/src/rrd_last.c
@@ -73,6 +73,7 @@ time_t rrd_last_r(
 
     rrd_t     rrd;
 
+    rrd_init(&rrd);
     rrd_file = rrd_open(filename, &rrd, RRD_READONLY);
     if (rrd_file != NULL) {
         lastup = rrd.live_head->last_up;

diff --git a/src/rrd_lastupdate.c b/src/rrd_lastupdate.c
index 62e3b8b..1546f6a 100644 (file)
--- a/src/rrd_lastupdate.c
+++ b/src/rrd_lastupdate.c
@@ -99,6 +99,7 @@ int rrd_lastupdate_r(const char *filename,
     rrd_t     rrd;
     rrd_file_t *rrd_file;
 
+    rrd_init(&rrd);
     rrd_file = rrd_open(filename, &rrd, RRD_READONLY);
     if (rrd_file == NULL) {
         rrd_free(&rrd);

diff --git a/src/rrd_open.c b/src/rrd_open.c
index e407acc..8ae5fd8 100644 (file)
--- a/src/rrd_open.c
+++ b/src/rrd_open.c
@@ -58,6 +58,10 @@
  * positioned to the first cdp in the first rra.
  * In the error path of rrd_open, only rrd_free(&rrd) has to be called
  * before returning an error. Do not call rrd_close upon failure of rrd_open.
+ * If creating a new file, the parameter rrd must be initialised with
+ * details of the file content.
+ * If opening an existing file, then use rrd must be initialised by
+ * rrd_init(rrd) prior to invoking rrd_open
  */
 
 rrd_file_t *rrd_open(
@@ -65,27 +69,43 @@ rrd_file_t *rrd_open(
     rrd_t *rrd,
     unsigned rdwr)
 {
+    int i;
     int       flags = 0;
     mode_t    mode = S_IRUSR;
     int       version;
 
 #ifdef HAVE_MMAP
     ssize_t   _page_size = sysconf(_SC_PAGESIZE);
-    int       mm_prot = PROT_READ, mm_flags = 0;
     char     *data = MAP_FAILED;
 #endif
     off_t     offset = 0;
     struct stat statb;
     rrd_file_t *rrd_file = NULL;
     off_t     newfile_size = 0;
+    off_t header_len, value_cnt, data_len;
 
-    if (rdwr & RRD_CREAT) {
-        /* yes bad inline signaling alert, we are using the
-           floatcookie to pass the size in ... only used in resize */
-        newfile_size = (off_t) rrd->stat_head->float_cookie;
-        free(rrd->stat_head);
+    /* Are we creating a new file? */
+    if((rdwr & RRD_CREAT) && (rrd->stat_head != NULL))
+    {
+        header_len = \
+          sizeof(stat_head_t) + \
+          sizeof(ds_def_t) * rrd->stat_head->ds_cnt + \
+          sizeof(rra_def_t) * rrd->stat_head->rra_cnt + \
+          sizeof(time_t) + \
+          sizeof(live_head_t) + \
+          sizeof(pdp_prep_t) * rrd->stat_head->ds_cnt + \
+          sizeof(cdp_prep_t) * rrd->stat_head->ds_cnt * rrd->stat_head->rra_cnt + \
+          sizeof(rra_ptr_t) * rrd->stat_head->rra_cnt;
+
+        value_cnt = 0;
+        for (i = 0; i < rrd->stat_head->rra_cnt; i++)
+            value_cnt += rrd->stat_head->ds_cnt * rrd->rra_def[i].row_cnt;
+
+        data_len = sizeof(rrd_value_t) * value_cnt;
+
+        newfile_size = header_len + data_len;
     }
-    rrd_init(rrd);
+    
     rrd_file = malloc(sizeof(rrd_file_t));
     if (rrd_file == NULL) {
         rrd_set_error("allocating rrd_file descriptor for '%s'", file_name);
@@ -101,12 +121,18 @@ rrd_file_t *rrd_open(
         exit(-1);
     }
 #endif
+
+#ifdef HAVE_MMAP
+    rrd_file->mm_prot = PROT_READ;
+    rrd_file->mm_flags = 0;
+#endif
+
     if (rdwr & RRD_READONLY) {
         flags |= O_RDONLY;
 #ifdef HAVE_MMAP
-        mm_flags = MAP_PRIVATE;
+        rrd_file->mm_flags = MAP_PRIVATE;
 # ifdef MAP_NORESERVE
-        mm_flags |= MAP_NORESERVE;  /* readonly, so no swap backing needed */
+        rrd_file->mm_flags |= MAP_NORESERVE;  /* readonly, so no swap backing needed */
 # endif
 #endif
     } else {
@@ -114,8 +140,8 @@ rrd_file_t *rrd_open(
             mode |= S_IWUSR;
             flags |= O_RDWR;
 #ifdef HAVE_MMAP
-            mm_flags = MAP_SHARED;
-            mm_prot |= PROT_WRITE;
+            rrd_file->mm_flags = MAP_SHARED;
+            rrd_file->mm_prot |= PROT_WRITE;
 #endif
         }
         if (rdwr & RRD_CREAT) {
@@ -124,10 +150,10 @@ rrd_file_t *rrd_open(
     }
     if (rdwr & RRD_READAHEAD) {
 #ifdef MAP_POPULATE
-        mm_flags |= MAP_POPULATE;   /* populate ptes and data */
+        rrd_file->mm_flags |= MAP_POPULATE;   /* populate ptes and data */
 #endif
 #if defined MAP_NONBLOCK
-        mm_flags |= MAP_NONBLOCK;   /* just populate ptes */
+        rrd_file->mm_flags |= MAP_NONBLOCK;   /* just populate ptes */
 #endif
     }
 #if defined(_WIN32) && !defined(__CYGWIN__) && !defined(__CYGWIN32__)
@@ -170,8 +196,9 @@ rrd_file_t *rrd_open(
            }
         }
 */
+
 #ifdef HAVE_MMAP
-    data = mmap(0, rrd_file->file_len, mm_prot, mm_flags,
+    data = mmap(0, rrd_file->file_len, rrd_file->mm_prot, rrd_file->mm_flags,
                 rrd_file->fd, offset);
 
     /* lets see if the first read worked */
@@ -570,10 +597,17 @@ ssize_t rrd_write(
     size_t count)
 {
 #ifdef HAVE_MMAP
+    int old_size = rrd_file->file_len;
     if (count == 0)
         return 0;
     if (buf == NULL)
         return -1;      /* EINVAL */
+    
+    if((rrd_file->pos + count) > old_size)
+    {
+        rrd_set_error("attempting to write beyond end of file");
+        return -1;
+    }
     memcpy(rrd_file->file_start + rrd_file->pos, buf, count);
     rrd_file->pos += count;
     return count;       /* mimmic write() semantics */

diff --git a/src/rrd_resize.c b/src/rrd_resize.c
index 57adbf0..32a501a 100644 (file)
--- a/src/rrd_resize.c
+++ b/src/rrd_resize.c
@@ -55,6 +55,7 @@ int rrd_resize(
         modify = -modify;
 
 
+    rrd_init(&rrdold);
     rrd_file = rrd_open(infilename, &rrdold, RRD_READWRITE | RRD_COPY);
     if (rrd_file == NULL) {
         rrd_free(&rrdold);
@@ -83,16 +84,32 @@ int rrd_resize(
             rrd_close(rrd_file);
             return (-1);
         }
-    /* the size of the new file */
-    /* yes we are abusing the float cookie for this, aargh */
+
+    rrd_init(&rrdnew);
+    /* These need to be initialised before calling rrd_open() with 
+       the RRD_CREATE flag */
     if ((rrdnew.stat_head = calloc(1, sizeof(stat_head_t))) == NULL) {
         rrd_set_error("allocating stat_head for new RRD");
         rrd_free(&rrdold);
         rrd_close(rrd_file);
         return (-1);
     }
-    rrdnew.stat_head->float_cookie = rrd_file->file_len +
-        (rrdold.stat_head->ds_cnt * sizeof(rrd_value_t) * modify);
+
+    if ((rrdnew.rra_def = malloc(sizeof(rra_def_t) * rrdold.stat_head->rra_cnt)) == NULL) {
+        rrd_set_error("allocating rra_def for new RRD");
+        rrd_free(&rrdnew);
+        rrd_free(&rrdold);
+        rrd_close(rrd_file);
+        rrd_close(rrd_out_file);
+        return (-1);
+    }
+
+    memcpy(rrdnew.stat_head,rrdold.stat_head,sizeof(stat_head_t));
+    memcpy(rrdnew.rra_def,rrdold.rra_def,sizeof(rra_def_t) * rrdold.stat_head->rra_cnt);
+
+    /* Set this so that the file will be created with the correct size */
+    rrdnew.rra_def[target_rra].row_cnt += modify;
+
     rrd_out_file = rrd_open(outfilename, &rrdnew, RRD_READWRITE | RRD_CREAT);
     if (rrd_out_file == NULL) {
         rrd_set_error("Can't create '%s': %s", outfilename,
@@ -108,15 +125,6 @@ int rrd_resize(
         return (-1);
     }
 /*XXX: do one write for those parts of header that are unchanged */
-    if ((rrdnew.stat_head = malloc(sizeof(stat_head_t))) == NULL) {
-        rrd_set_error("allocating stat_head for new RRD");
-        rrd_free(&rrdnew);
-        rrd_free(&rrdold);
-        rrd_close(rrd_file);
-        rrd_close(rrd_out_file);
-        return (-1);
-    }
-
     if ((rrdnew.rra_ptr = malloc(sizeof(rra_ptr_t) * rrdold.stat_head->rra_cnt)) == NULL) {
         rrd_set_error("allocating rra_ptr for new RRD");
         rrd_free(&rrdnew);
@@ -126,18 +134,11 @@ int rrd_resize(
         return (-1);
     }
 
-    if ((rrdnew.rra_def = malloc(sizeof(rra_def_t) * rrdold.stat_head->rra_cnt)) == NULL) {
-        rrd_set_error("allocating rra_def for new RRD");
-        rrd_free(&rrdnew);
-        rrd_free(&rrdold);
-        rrd_close(rrd_file);
-        rrd_close(rrd_out_file);
-        return (-1);
-    }
-     
-    memcpy(rrdnew.stat_head,rrdold.stat_head,sizeof(stat_head_t));
+    /* Put this back the way it was so that the rest of the algorithm
+       below remains unchanged, it will be corrected later */
+    rrdnew.rra_def[target_rra].row_cnt -= modify;
+
     rrdnew.ds_def = rrdold.ds_def;
-    memcpy(rrdnew.rra_def,rrdold.rra_def,sizeof(rra_def_t) * rrdold.stat_head->rra_cnt);    
     rrdnew.live_head = rrdold.live_head;
     rrdnew.pdp_prep = rrdold.pdp_prep;
     rrdnew.cdp_prep = rrdold.cdp_prep;

diff --git a/src/rrd_tune.c b/src/rrd_tune.c
index 4975d21..89559af 100644 (file)
--- a/src/rrd_tune.c
+++ b/src/rrd_tune.c
@@ -98,6 +98,7 @@ int rrd_tune(
     opterr = 0;         /* initialize getopt */
 
 
+    rrd_init(&rrd);
     rrd_file = rrd_open(argv[1], &rrd, RRD_READWRITE);
     if (rrd_file == NULL) {
         rrd_free(&rrd);

diff --git a/src/rrd_update.c b/src/rrd_update.c
index 108dcbb..62c8c3b 100644 (file)
--- a/src/rrd_update.c
+++ b/src/rrd_update.c
@@ -519,6 +519,7 @@ int _rrd_update(
         goto err_out;
     }
 
+    rrd_init(&rrd);
     if ((rrd_file = rrd_open(filename, &rrd, RRD_READWRITE)) == NULL) {
         goto err_free;
     }