snapshot: remove a snapshot's brick pid directory on deactivate and delete (bug-1597662.t on any localstatedir)

tests/bugs/snapshot/bug-1597662-zfs.t

@@ -20,15 +20,18 @@ 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
+# <run-directory>/snaps/<snap-name>; env.rc exports that directory
+# as GLUSTERD_PIDFILEDIR.
+snap_path=$GLUSTERD_PIDFILEDIR/snaps
 
 TEST $CLI snapshot create snap1 $V0 no-timestamp;
 
 $CLI snapshot activate snap1;
 
 EXPECT 'Started' snapshot_status snap1;
 
-# This Function will check for entry /var/run/gluster/snaps/<snap-name>
+# This Function will check for entry $snap_path/<snap-name>
 # against snap-name
 
 function is_snap_path

tests/bugs/snapshot/bug-1597662.t

@@ -14,15 +14,18 @@ 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
+# <run-directory>/snaps/<snap-name>; env.rc exports that directory
+# as GLUSTERD_PIDFILEDIR.
+snap_path=$GLUSTERD_PIDFILEDIR/snaps
 
 TEST $CLI snapshot create snap1 $V0 no-timestamp;
 
 $CLI snapshot activate snap1;
 
 EXPECT 'Started' snapshot_status snap1;
 
-# This Function will check for entry /var/run/gluster/snaps/<snap-name>
+# This Function will check for entry $snap_path/<snap-name>
 # against snap-name
 
 function is_snap_path

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"
 

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
 }

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, <run-directory>/snaps/<snapname>/<volname> (see
+ * GLUSTERD_GET_VOLUME_PID_DIR), and the <snapname> directory above it once
+ * that is empty. It is the same directory as
+ * <snap_mount_dir>/<snapname>/<volname> 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) {

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;                                                 \