From 37897f0b72617e442e4799b35ebda94294218e05 Mon Sep 17 00:00:00 2001
From: Pranith Kumar K <pkarampu@redhat.com>
Date: Wed, 28 Feb 2018 17:58:31 +0530
Subject: [PATCH 183/201] cluster/afr: Fix dict-leak in pre-op
At the time of pre-op, pre_op_xdata is populted with the xattrs we get from the
disk and at the time of post-op it gets over-written without unreffing the
previous value stored leading to a leak.
This is a regression we missed in
https://review.gluster.org/#/q/ba149bac92d169ae2256dbc75202dc9e5d06538e
>BUG: 1550078
>Change-Id: I0456f9ad6f77ce6248b747964a037193af3a3da7
>Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
>Upstream: https://review.gluster.org/19647
BUG: 1552360
Change-Id: I0456f9ad6f77ce6248b747964a037193af3a3da7
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/131936
Tested-by: RHGS Build Bot <nigelb@redhat.com>
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
---
xlators/cluster/afr/src/afr-common.c | 16 ++++++++--------
xlators/cluster/afr/src/afr-transaction.c | 20 ++++++++++----------
xlators/cluster/afr/src/afr.h | 4 ++--
3 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c
index 6e6f5fa..855e568 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -1039,7 +1039,7 @@ afr_readables_fill (call_frame_t *frame, xlator_t *this, inode_t *inode,
xdata = replies[i].xdata;
ia_type = replies[i].poststat.ia_type;
} else {/* pre-op xattrop */
- xdata = local->transaction.pre_op_xdata[i];
+ xdata = local->transaction.changelog_xdata[i];
ia_type = inode->ia_type;
}
@@ -1757,13 +1757,13 @@ afr_local_transaction_cleanup (afr_local_t *local, xlator_t *this)
GF_FREE (local->transaction.pre_op);
GF_FREE (local->transaction.pre_op_sources);
- if (local->transaction.pre_op_xdata) {
+ if (local->transaction.changelog_xdata) {
for (i = 0; i < priv->child_count; i++) {
- if (!local->transaction.pre_op_xdata[i])
+ if (!local->transaction.changelog_xdata[i])
continue;
- dict_unref (local->transaction.pre_op_xdata[i]);
+ dict_unref (local->transaction.changelog_xdata[i]);
}
- GF_FREE (local->transaction.pre_op_xdata);
+ GF_FREE (local->transaction.changelog_xdata);
}
GF_FREE (local->transaction.eager_lock);
@@ -5531,10 +5531,10 @@ afr_transaction_local_init (afr_local_t *local, xlator_t *this)
if (!local->transaction.pre_op)
goto out;
- local->transaction.pre_op_xdata =
- GF_CALLOC (sizeof (*local->transaction.pre_op_xdata),
+ local->transaction.changelog_xdata =
+ GF_CALLOC (sizeof (*local->transaction.changelog_xdata),
priv->child_count, gf_afr_mt_dict_t);
- if (!local->transaction.pre_op_xdata)
+ if (!local->transaction.changelog_xdata)
goto out;
if (priv->arbiter_count == 1) {
diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c
index 19740e1..97f9dd4 100644
--- a/xlators/cluster/afr/src/afr-transaction.c
+++ b/xlators/cluster/afr/src/afr-transaction.c
@@ -276,9 +276,9 @@ afr_compute_pre_op_sources (call_frame_t *frame, xlator_t *this)
matrix = ALLOC_MATRIX (priv->child_count, int);
for (i = 0; i < priv->child_count; i++) {
- if (!local->transaction.pre_op_xdata[i])
+ if (!local->transaction.changelog_xdata[i])
continue;
- xdata = local->transaction.pre_op_xdata[i];
+ xdata = local->transaction.changelog_xdata[i];
afr_selfheal_fill_matrix (this, matrix, i, idx, xdata);
}
@@ -295,13 +295,6 @@ afr_compute_pre_op_sources (call_frame_t *frame, xlator_t *this)
for (j = 0; j < priv->child_count; j++)
if (matrix[i][j] != 0)
local->transaction.pre_op_sources[j] = 0;
-
- /*We don't need the xattrs any more. */
- for (i = 0; i < priv->child_count; i++)
- if (local->transaction.pre_op_xdata[i]) {
- dict_unref (local->transaction.pre_op_xdata[i]);
- local->transaction.pre_op_xdata[i] = NULL;
- }
}
gf_boolean_t
@@ -1173,7 +1166,7 @@ afr_changelog_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
}
if (xattr)
- local->transaction.pre_op_xdata[child_index] = dict_ref (xattr);
+ local->transaction.changelog_xdata[child_index] = dict_ref (xattr);
call_count = afr_frame_return (frame);
@@ -1605,6 +1598,13 @@ afr_changelog_do (call_frame_t *frame, xlator_t *this, dict_t *xattr,
local = frame->local;
priv = this->private;
+ for (i = 0; i < priv->child_count; i++) {
+ if (local->transaction.changelog_xdata[i]) {
+ dict_unref (local->transaction.changelog_xdata[i]);
+ local->transaction.changelog_xdata[i] = NULL;
+ }
+ }
+
ret = afr_changelog_prepare (this, frame, &call_count, changelog_resume,
op, &xdata, &newloc_xdata);
diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h
index 96fefb1..c822221 100644
--- a/xlators/cluster/afr/src/afr.h
+++ b/xlators/cluster/afr/src/afr.h
@@ -748,8 +748,8 @@ typedef struct _afr_local {
unsigned char *pre_op;
- /* For arbiter configuration only. */
- dict_t **pre_op_xdata;
+ /* Changelog xattr dict for [f]xattrop*/
+ dict_t **changelog_xdata;
unsigned char *pre_op_sources;
/* @failed_subvols: subvolumes on which a pre-op or a
--
1.8.3.1