diff --git a/tests/bugs/snapshot/bug-1597662-zfs.t b/tests/bugs/snapshot/bug-1597662-zfs.t index 2988a3b9d80..7c3f8349fe2 100644 --- a/tests/bugs/snapshot/bug-1597662-zfs.t +++ b/tests/bugs/snapshot/bug-1597662-zfs.t @@ -20,7 +20,10 @@ TEST pidof glusterd; TEST $CLI volume create $V0 $H0:$L1 $H0:$L2 $H0:$L3; TEST $CLI volume start $V0; -snap_path=/var/run/gluster/snaps +# glusterd keeps the pid files of a snapshot volume's bricks under +# /snaps/; env.rc exports that directory +# as GLUSTERD_PIDFILEDIR. +snap_path=$GLUSTERD_PIDFILEDIR/snaps TEST $CLI snapshot create snap1 $V0 no-timestamp; @@ -28,7 +31,7 @@ $CLI snapshot activate snap1; EXPECT 'Started' snapshot_status snap1; -# This Function will check for entry /var/run/gluster/snaps/ +# This Function will check for entry $snap_path/ # against snap-name function is_snap_path diff --git a/tests/bugs/snapshot/bug-1597662.t b/tests/bugs/snapshot/bug-1597662.t index f582930476a..3d68ac3e4f4 100644 --- a/tests/bugs/snapshot/bug-1597662.t +++ b/tests/bugs/snapshot/bug-1597662.t @@ -14,7 +14,10 @@ TEST pidof glusterd; TEST $CLI volume create $V0 $H0:$L1 $H0:$L2 $H0:$L3; TEST $CLI volume start $V0; -snap_path=/var/run/gluster/snaps +# glusterd keeps the pid files of a snapshot volume's bricks under +# /snaps/; env.rc exports that directory +# as GLUSTERD_PIDFILEDIR. +snap_path=$GLUSTERD_PIDFILEDIR/snaps TEST $CLI snapshot create snap1 $V0 no-timestamp; @@ -22,7 +25,7 @@ $CLI snapshot activate snap1; EXPECT 'Started' snapshot_status snap1; -# This Function will check for entry /var/run/gluster/snaps/ +# This Function will check for entry $snap_path/ # against snap-name function is_snap_path diff --git a/tests/snapshot.rc b/tests/snapshot.rc index c298e78e26c..4012ba692d8 100644 --- a/tests/snapshot.rc +++ b/tests/snapshot.rc @@ -112,6 +112,10 @@ function _cleanup_lvm_again() { findmnt -nRlT "${B0}" -o TARGET,SOURCE | grep "${LVM_PREFIX}" | awk '{print $2}' | xargs -r ${UMOUNT_F} findmnt -nRlo TARGET,SOURCE | grep "run/gluster/snaps" | awk '{print $2}' | xargs -r ${UMOUNT_F} \rm -rf /var/run/gluster/snaps/* + # the pid files of snapshot bricks live under glusterd's run directory + if [ -n "${GLUSTERD_PIDFILEDIR}" ]; then + \rm -rf ${GLUSTERD_PIDFILEDIR}/snaps/* + fi vgremove -fyS "vg_name=~^${LVM_PREFIX}_vg" diff --git a/tests/snapshot_zfs.rc b/tests/snapshot_zfs.rc index 88f3fa54499..478d0ca01b3 100644 --- a/tests/snapshot_zfs.rc +++ b/tests/snapshot_zfs.rc @@ -64,6 +64,10 @@ function cleanup_zfs() { _cleanup_zfs_again >/dev/null 2>&1 \rm -rf /var/run/gluster/snaps/* + # the pid files of snapshot bricks live under glusterd's run directory + if [ -n "${GLUSTERD_PIDFILEDIR}" ]; then + \rm -rf ${GLUSTERD_PIDFILEDIR}/snaps/* + fi zfs list | grep "${ZFS_PREFIX}" | awk '{print $1}'| xargs -L 1 -r zpool destroy -f 2>/dev/null return 0 } diff --git a/xlators/mgmt/glusterd/src/glusterd-snapshot.c b/xlators/mgmt/glusterd/src/glusterd-snapshot.c index fbf510642ef..1a11f322a62 100644 --- a/xlators/mgmt/glusterd/src/glusterd-snapshot.c +++ b/xlators/mgmt/glusterd/src/glusterd-snapshot.c @@ -2651,6 +2651,52 @@ glusterd_snapshot_remove(dict_t *rsp_dict, glusterd_volinfo_t *snap_vol, return ret; } +/* Remove the directory glusterd keeps the pid files of a snapshot volume's + * bricks in, /snaps// (see + * GLUSTERD_GET_VOLUME_PID_DIR), and the directory above it once + * that is empty. It is the same directory as + * // only when localstatedir is /var; + * there the callers have already removed the mount-dir tree and this is a + * no-op (recursive_rmdir() returns 0 for a missing directory). Call it only + * once every brick of the volume on this node is stopped: the directory + * holds the pid files brick stop looks the processes up by. + * glusterd_brick_start() recreates it on activate. Best effort - it never + * fails the caller. Clones keep their pid directory under vols/ and are + * left alone. + */ +static void +glusterd_snap_volume_pid_dir_remove(glusterd_volinfo_t *snap_vol) +{ + xlator_t *this = THIS; + glusterd_conf_t *priv = this->private; + char pid_dir[PATH_MAX] = ""; + int ret = -1; + + if (!snap_vol->is_snap_volume || !snap_vol->snapshot) + return; + + GLUSTERD_GET_VOLUME_PID_DIR(pid_dir, snap_vol, priv); + if (!pid_dir[0]) + return; + + ret = recursive_rmdir(pid_dir); + if (ret) { + gf_msg(this->name, GF_LOG_WARNING, errno, GD_MSG_DIR_OP_FAILED, + "Failed to remove %s directory", pid_dir); + return; + } + + GLUSTERD_GET_SNAP_PID_DIR(pid_dir, snap_vol->snapshot->snapname, priv); + if (!pid_dir[0]) + return; + + ret = sys_rmdir(pid_dir); + if (ret && (errno != ENOENT) && (errno != ENOTEMPTY)) { + gf_msg(this->name, GF_LOG_WARNING, errno, GD_MSG_DIR_OP_FAILED, + "Failed to remove %s directory", pid_dir); + } +} + int32_t glusterd_snap_volume_remove(dict_t *rsp_dict, glusterd_volinfo_t *snap_vol, gf_boolean_t remove_snapshot, gf_boolean_t force) @@ -2708,6 +2754,11 @@ glusterd_snap_volume_remove(dict_t *rsp_dict, glusterd_volinfo_t *snap_vol, } } + /* Every brick of this node is stopped now (or a stop failed under + * force, in which case leave the pid files alone). */ + if (!save_ret) + glusterd_snap_volume_pid_dir_remove(snap_vol); + ret = glusterd_store_delete_volume(snap_vol); if (ret) { gf_msg(this->name, GF_LOG_WARNING, 0, GD_MSG_VOL_DELETE_FAIL, @@ -5799,6 +5850,10 @@ glusterd_snapshot_deactivate_commit(dict_t *dict, char **op_errstr, goto out; } + /* The bricks' pid files live under the run directory, which is that + * same tree only when localstatedir is /var. */ + glusterd_snap_volume_pid_dir_remove(snap_volinfo); + ret = dict_set_dynstr_with_alloc(rsp_dict, "snapuuid", uuid_utoa(snap->snap_id)); if (ret) { diff --git a/xlators/mgmt/glusterd/src/glusterd.h b/xlators/mgmt/glusterd/src/glusterd.h index b5382734cc6..824b6ba81d4 100644 --- a/xlators/mgmt/glusterd/src/glusterd.h +++ b/xlators/mgmt/glusterd/src/glusterd.h @@ -574,6 +574,16 @@ enum glusterd_op_ret { } \ } while (0) +#define GLUSTERD_GET_SNAP_PID_DIR(path, snapname, priv) \ + do { \ + int32_t _snap_pid_len; \ + _snap_pid_len = snprintf(path, PATH_MAX, "%s/snaps/%s", priv->rundir, \ + snapname); \ + if ((_snap_pid_len < 0) || (_snap_pid_len >= PATH_MAX)) { \ + path[0] = 0; \ + } \ + } while (0) + #define GLUSTERD_GET_SNAP_GEO_REP_DIR(path, snap, priv) \ do { \ int32_t _snap_geo_len; \