From f9b4ef19d9e80ab4723e947a04c1c094843a0b6f Mon Sep 17 00:00:00 2001 From: Avra Sengupta Date: Tue, 21 Oct 2014 08:42:40 +0000 Subject: glusterd/snapshot: Check if LVM device path exists before delete Check if the LV is present before deleting the LV. In case where the LV is absent (already deleted?), need not fail the snap delete operation. Also check if the LV is mounted before trying umount. In case it isn't umounted, only remove the LV. Change-Id: I0f5b2674797299d8748c6fac5b091f0caba65ca4 BUG: 1175754 Signed-off-by: Avra Sengupta Reviewed-on: http://review.gluster.org/8954 Tested-by: Gluster Build System Reviewed-by: Krishnan Parthasarathi Tested-by: Krishnan Parthasarathi Signed-off-by: Sachin Pandit Reviewed-on: http://review.gluster.org/9299 Reviewed-by: Raghavendra Bhat --- xlators/mgmt/glusterd/src/glusterd-snapshot.c | 105 +++++++++++++++----------- 1 file changed, 59 insertions(+), 46 deletions(-) (limited to 'xlators/mgmt/glusterd/src') diff --git a/xlators/mgmt/glusterd/src/glusterd-snapshot.c b/xlators/mgmt/glusterd/src/glusterd-snapshot.c index 66935154608..b347e9d9206 100644 --- a/xlators/mgmt/glusterd/src/glusterd-snapshot.c +++ b/xlators/mgmt/glusterd/src/glusterd-snapshot.c @@ -2249,6 +2249,9 @@ glusterd_do_lvm_snapshot_remove (glusterd_volinfo_t *snap_vol, char pidfile[PATH_MAX] = {0, }; pid_t pid = -1; int retry_count = 0; + char *mnt_pt = NULL; + struct mntent *entry = NULL; + gf_boolean_t unmount = _gf_true; this = THIS; GF_ASSERT (this); @@ -2273,9 +2276,32 @@ glusterd_do_lvm_snapshot_remove (glusterd_volinfo_t *snap_vol, } } + /* Check if the brick is mounted and then try unmounting the brick */ + ret = glusterd_get_brick_root (brickinfo->path, &mnt_pt); + if (ret) { + gf_log (this->name, GF_LOG_WARNING, "Getting the root " + "of the brick for volume %s (snap %s) failed. " + "Removing lv (%s).", snap_vol->volname, + snap_vol->snapshot->snapname, snap_device); + /* The brick path is already unmounted. Remove the lv only * + * Need not fail the operation */ + ret = 0; + unmount = _gf_false; + } + + if ((unmount == _gf_true) && (strcmp (mnt_pt, mount_pt))) { + gf_log (this->name, GF_LOG_WARNING, + "Lvm is not mounted for brick %s:%s. " + "Removing lv (%s).", brickinfo->hostname, + brickinfo->path, snap_device); + /* The brick path is already unmounted. Remove the lv only * + * Need not fail the operation */ + unmount = _gf_false; + } + /* umount cannot be done when the brick process is still in the process of shutdown, so give three re-tries */ - while (retry_count < 3) { + while ((unmount == _gf_true) && (retry_count < 3)) { retry_count++; /*umount2 system call doesn't cleanup mtab entry after un-mount. So use external umount command*/ @@ -2318,8 +2344,6 @@ out: int32_t glusterd_lvm_snapshot_remove (dict_t *rsp_dict, glusterd_volinfo_t *snap_vol) { - char *mnt_pt = NULL; - struct mntent *entry = NULL; struct mntent save_entry = {0,}; int32_t brick_count = -1; int32_t ret = -1; @@ -2355,7 +2379,19 @@ glusterd_lvm_snapshot_remove (dict_t *rsp_dict, glusterd_volinfo_t *snap_vol) continue; } - ret = lstat (brickinfo->path, &stbuf); + /* Fetch the brick mount path from the brickinfo->path */ + ret = glusterd_find_brick_mount_path (brickinfo->path, + brick_count + 1, + &brick_mount_path); + if (ret) { + gf_log (this->name, GF_LOG_ERROR, + "Failed to find brick_mount_path for %s", + brickinfo->path); + ret = 0; + continue; + } + + ret = lstat (brick_mount_path, &stbuf); if (ret) { gf_log (this->name, GF_LOG_DEBUG, "Brick %s:%s already deleted.", @@ -2393,54 +2429,36 @@ glusterd_lvm_snapshot_remove (dict_t *rsp_dict, glusterd_volinfo_t *snap_vol) continue; } - ret = glusterd_get_brick_root (brickinfo->path, &mnt_pt); - if (ret) { - gf_log (this->name, GF_LOG_WARNING, "getting the root " - "of the brick for volume %s (snap %s) failed ", - snap_vol->volname, snap_vol->snapshot->snapname); - continue; + /* Check if the brick has a LV associated with it */ + if (!brickinfo->device_path) { + gf_log (this->name, GF_LOG_DEBUG, + "Brick (%s:%s) does not have a LV " + "associated with it. Removing the brick path", + brickinfo->hostname, brickinfo->path); + goto remove_brick_path; } - /* Fetch the brick mount path from the brickinfo->path */ - ret = glusterd_find_brick_mount_path (brickinfo->path, - brick_count + 1, - &brick_mount_path); + /* Verify if the device path exists or not */ + ret = stat (brickinfo->device_path, &stbuf); if (ret) { - gf_log (this->name, GF_LOG_ERROR, - "Failed to find brick_mount_path for %s", - brickinfo->path); - GF_FREE (mnt_pt); - mnt_pt = NULL; - continue; - } - - if (strcmp (mnt_pt, brick_mount_path)) { gf_log (this->name, GF_LOG_DEBUG, - "Lvm is not mounted for brick %s:%s. " - "Removing the brick path.", + "LV (%s) for brick (%s:%s) not present. " + "Removing the brick path", + brickinfo->device_path, brickinfo->hostname, brickinfo->path); - err = -1; /* We need to record this failure */ + /* Making ret = 0 as absence of device path should * + * not fail the remove operation */ + ret = 0; goto remove_brick_path; } - entry = glusterd_get_mnt_entry_info (mnt_pt, buff, - sizeof (buff), &save_entry); - if (!entry) { - gf_log (this->name, GF_LOG_WARNING, "getting the mount" - " entry for the brick %s:%s of the snap %s " - "(volume: %s) failed", brickinfo->hostname, - brickinfo->path, snap_vol->snapshot->snapname, - snap_vol->volname); - err = -1; /* We need to record this failure */ - goto remove_brick_path; - } ret = glusterd_do_lvm_snapshot_remove (snap_vol, brickinfo, - mnt_pt, - entry->mnt_fsname); + brick_mount_path, + brickinfo->device_path); if (ret) { - gf_log (this->name, GF_LOG_ERROR, "failed to " + gf_log (this->name, GF_LOG_ERROR, "Failed to " "remove the snapshot %s (%s)", - brickinfo->path, entry->mnt_fsname); + brickinfo->path, brickinfo->device_path); err = -1; /* We need to record this failure */ } @@ -2457,9 +2475,7 @@ remove_brick_path: if (!tmp) { gf_log (this->name, GF_LOG_ERROR, "Invalid brick %s", brickinfo->path); - GF_FREE (mnt_pt); GF_FREE (brick_mount_path); - mnt_pt = NULL; brick_mount_path = NULL; continue; } @@ -2471,9 +2487,7 @@ remove_brick_path: is_brick_dir_present = _gf_true; } - GF_FREE (mnt_pt); GF_FREE (brick_mount_path); - mnt_pt = NULL; brick_mount_path = NULL; } @@ -2504,7 +2518,6 @@ out: if (err) { ret = err; } - GF_FREE (mnt_pt); GF_FREE (brick_mount_path); gf_log (this->name, GF_LOG_TRACE, "Returning %d", ret); return ret; -- cgit