From 1b42cb507c3f72225f03a4715e051d9eb8c08e43 Mon Sep 17 00:00:00 2001 From: Saravanakumar Arumugam Date: Wed, 29 Apr 2015 17:33:43 +0530 Subject: features/changelog: Avoid creation of empty changelogs An empty changelog when rolled over gets unlinked and indexed with a modified path-name in htime file. The modification is "changelog" not "CHANGELOG" in basename of the empty changelog file. BUG: 1219479 Change-Id: Ib5b825ab563fa34d8dcf4368cf6cbf4b25d78a6d Original-Author: Ajeet Jha Original-Author: Saravanakumar Arumugam Reviewed-On: http://review.gluster.org/#/c/9572/ Signed-off-by: Kotresh HR Reviewed-on: http://review.gluster.org/10642 Tested-by: Gluster Build System Reviewed-by: Aravinda VK Reviewed-by: Vijay Bellur --- .../changelog/lib/src/gf-history-changelog.c | 45 +++++-- xlators/features/changelog/src/changelog-helpers.c | 130 ++++++++++++++++++--- xlators/features/changelog/src/changelog-misc.h | 2 +- 3 files changed, 154 insertions(+), 23 deletions(-) (limited to 'xlators/features/changelog') diff --git a/xlators/features/changelog/lib/src/gf-history-changelog.c b/xlators/features/changelog/lib/src/gf-history-changelog.c index 50afb3fe0c7..ba312e3b10f 100644 --- a/xlators/features/changelog/lib/src/gf-history-changelog.c +++ b/xlators/features/changelog/lib/src/gf-history-changelog.c @@ -477,6 +477,36 @@ out: return -1; } +/* + * Description: Checks if the changelog path is usable or not, + * which is differenciated by checking for "changelog" + * in the path and not "CHANGELOG". + * + * Returns: + * 1 : Yes, usable ( contains "CHANGELOG" ) + * 0 : No, Not usable ( contains, "changelog") + */ +int +gf_is_changelog_usable (char *cl_path) +{ + int ret = -1; + const char low_c[] = "changelog"; + char *str_ret = NULL; + char *bname = NULL; + + bname = basename (cl_path); + + str_ret = strstr (bname, low_c); + + if (str_ret != NULL) + ret = 0; + else + ret = 1; + + return ret; + +} + void * gf_changelog_consume_wrap (void* data) { @@ -499,15 +529,16 @@ gf_changelog_consume_wrap (void* data) } /* TODO: handle short reads and EOF. */ + if (gf_is_changelog_usable (ccd->changelog) == 1) { - ret = gf_changelog_consume (ccd->this, - ccd->jnl, ccd->changelog, _gf_true); - if (ret) { - gf_log (this->name, GF_LOG_ERROR, - "could not parse changelog: %s", ccd->changelog); - goto out; + ret = gf_changelog_consume (ccd->this, + ccd->jnl, ccd->changelog, _gf_true); + if (ret) { + gf_log (this->name, GF_LOG_ERROR, + "could not parse changelog: %s", ccd->changelog); + goto out; + } } - ccd->retval = 0; out: diff --git a/xlators/features/changelog/src/changelog-helpers.c b/xlators/features/changelog/src/changelog-helpers.c index 8dae5efa454..17c085fe047 100644 --- a/xlators/features/changelog/src/changelog-helpers.c +++ b/xlators/features/changelog/src/changelog-helpers.c @@ -340,15 +340,91 @@ out: return ret; } +/* + * Description: Check if the changelog to rollover is empty or not. + * It is assumed that fd passed is already verified. + * + * Returns: + * 1 : If found empty, changed path from "CHANGELOG." to "changelog." + * 0 : If NOT empty, proceed usual. + */ +int +cl_is_empty (xlator_t *this, int fd) +{ + int ret = -1; + size_t elen = 0; + int encoding = -1; + char buffer[1024] = {0,}; + struct stat stbuf = {0,}; + + ret = fstat (fd, &stbuf); + if (ret) { + gf_log (this->name, GF_LOG_ERROR, + "Could not stat (CHANGELOG)"); + goto out; + } + + ret = lseek (fd, 0, SEEK_SET); + if (ret == -1) { + gf_log (this->name, GF_LOG_ERROR, + "Could not lseek (CHANGELOG)"); + goto out; + } + + CHANGELOG_GET_ENCODING (fd, buffer, 1024, encoding, elen); + + if (elen == stbuf.st_size) { + ret = 1; + } else { + ret = 0; + } + +out: + return ret; +} + +/* + * Description: Updates "CHANGELOG" to "changelog" for writing changelog path + * to htime file. + * + * Returns: + * 0 : Success + * -1 : Error + */ +int +update_path (xlator_t *this, char *cl_path) +{ + char low_cl[] = "changelog"; + char up_cl[] = "CHANGELOG"; + char *found = NULL; + int iter = 0; + int ret = -1; + + found = strstr(cl_path, up_cl); + + if (found == NULL) { + gf_log (this->name, GF_LOG_ERROR, + "Could not find CHANGELOG in changelog path"); + goto out; + } else { + strncpy(found, low_cl, strlen(low_cl)); + } + + ret = 0; +out: + return ret; +} + static int changelog_rollover_changelog (xlator_t *this, changelog_priv_t *priv, unsigned long ts) { - int ret = -1; - int notify = 0; - char ofile[PATH_MAX] = {0,}; - char nfile[PATH_MAX] = {0,}; - changelog_event_t ev = {0,}; + int ret = -1; + int notify = 0; + int cl_empty_flag = 0; + char ofile[PATH_MAX] = {0,}; + char nfile[PATH_MAX] = {0,}; + changelog_event_t ev = {0,}; if (priv->changelog_fd != -1) { ret = fsync (priv->changelog_fd); @@ -357,6 +433,14 @@ changelog_rollover_changelog (xlator_t *this, "fsync failed (reason: %s)", strerror (errno)); } + ret = cl_is_empty (this, priv->changelog_fd); + if (ret == 1) { + cl_empty_flag = 1; + } else if (ret == -1) { + /* Log error but proceed as usual */ + gf_log (this->name, GF_LOG_WARNING, + "Error detecting empty changelog"); + } close (priv->changelog_fd); priv->changelog_fd = -1; } @@ -367,22 +451,38 @@ changelog_rollover_changelog (xlator_t *this, "%s/"CHANGELOG_FILE_NAME".%lu", priv->changelog_dir, ts); - ret = rename (ofile, nfile); - if (!ret) - notify = 1; + if (cl_empty_flag == 1) { + ret = unlink (ofile); + if (ret) { + gf_log (this->name, GF_LOG_ERROR, + "error unlinking(empty cl) %s (reason %s)", + ofile, strerror(errno)); + ret = 0; /* Error in unlinking empty changelog should + not break further changelog operation, so + reset return value to 0*/ + } + } else { + ret = rename (ofile, nfile); - if (ret && (errno == ENOENT)) { - ret = 0; - goto out; + if (ret && (errno == ENOENT)) { + ret = 0; + goto out; + } + if (ret) { + gf_log (this->name, GF_LOG_ERROR, + "error renaming %s -> %s (reason %s)", + ofile, nfile, strerror (errno)); + } } - if (ret) { - gf_log (this->name, GF_LOG_ERROR, - "error renaming %s -> %s (reason %s)", - ofile, nfile, strerror (errno)); + if (!ret && (cl_empty_flag == 0)) { + notify = 1; } if (!ret) { + if (cl_empty_flag) { + update_path (this, nfile); + } ret = htime_update (this, priv, ts, nfile); if (ret == -1) { gf_log (this->name, GF_LOG_ERROR, diff --git a/xlators/features/changelog/src/changelog-misc.h b/xlators/features/changelog/src/changelog-misc.h index c0349ca3838..71c976f5010 100644 --- a/xlators/features/changelog/src/changelog-misc.h +++ b/xlators/features/changelog/src/changelog-misc.h @@ -73,7 +73,7 @@ fp = fdopen (fd_dup, "r"); \ if (fp) { \ if (fgets (buffer, len, fp)) { \ - elen = strlen (buffer); \ + enc_len = strlen (buffer); \ sscanf (buffer, \ CHANGELOG_HEADER, \ &maj, &min, &enc); \ -- cgit