LU-4223 utils: fixing loop leaking in utils

1. If the file is being opened by popen, it should use
pclose instead of fclose to close the file, to make sure
the process created by popen is closed after pclose, then
to avoid loop device is being hold on release.

2. Give another try in loop_cleanup in case there are still
some process going on with the loop.

3. wait loop device to release before continue conf-sanity
32c.

4. Add losetup -a to list loop dev information when the
test(conf-sanity 32) fails.

This patch is back-ported from the following one:
Lustre-commit: 98ac0fe3a45dde62759ecaa4c84e6250ac2067f8
Lustre-change: http://review.whamcloud.com/8409

Test-Parameters: envdefinitions=SLOW=yes,ENABLE_QUOTA=yes \
mdscount=4 mdtcount=4 testlist=conf-sanity

Signed-off-by: wang di <di.wang@intel.com>
Change-Id: Ic1ebc2a6b2ce4280c2123080171e203e99267b28
Signed-off-by: Jian Yu <jian.yu@intel.com>
Reviewed-on: http://review.whamcloud.com/8723
Tested-by: Jenkins
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>

diff --git a/lustre/tests/conf-sanity.sh b/lustre/tests/conf-sanity.sh
index ae51f6a..8552663 100644 (file)
--- a/lustre/tests/conf-sanity.sh
+++ b/lustre/tests/conf-sanity.sh
@@ -1393,16 +1393,20 @@ t32_reload_modules() {
 t32_wait_til_devices_gone() {
        local node=$1
        local devices
+       local loops
        local i=0
 
        echo wait for devices to go
        while ((i < 20)); do
                devices=$(do_rpc_nodes $node $LCTL device_list | wc -l)
-               ((devices == 0)) && return 0
+               loops=$(do_rpc_nodes $node losetup -a | grep -c t32)
+               ((devices == 0 && loops == 0)) && return 0
                sleep 5
                i=$((i + 1))
        done
-       echo "waiting for devices on $node: Given up"
+       echo "waiting for dev on $node: dev $devices loop $loops given up"
+       do_rpc_nodes $node "losetup -a"
+       do_rpc_nodes $node "$LCTL devices_list"
        return 1
 }
 
@@ -1537,6 +1541,7 @@ t32_test() {
        $r $LCTL set_param debug="$PTLDEBUG"
 
        $r $TUNEFS --dryrun $tmp/mdt || {
+               $r losetup -a
                error_noexit "tunefs.lustre before mounting the MDT"
                return 1
        }
@@ -1544,6 +1549,7 @@ t32_test() {
                mopts=loop,writeconf
                if [ $fstype == "ldiskfs" ]; then
                        $r $TUNEFS --quota $tmp/mdt || {
+                               $r losetup -a
                                error_noexit "Enable mdt quota feature"
                                return 1
                        }
@@ -1569,6 +1575,7 @@ t32_test() {
        t32_wait_til_devices_gone $node
 
        $r mount -t lustre -o $mopts $tmp/mdt $tmp/mnt/mdt || {
+               $r losetup -a
                error_noexit "Mounting the MDT"
                return 1
        }
@@ -1620,6 +1627,7 @@ t32_test() {
                mopts=loop,mgsnode=$nid,$writeconf
                if [ $fstype == "ldiskfs" ]; then
                        $r $TUNEFS --quota $tmp/ost || {
+                               $r losetup -a
                                error_noexit "Enable ost quota feature"
                                return 1
                        }
@@ -1868,6 +1876,7 @@ t32_test() {
 
                # mount a second time to make sure we didnt leave upgrade flag on
                $r $TUNEFS --dryrun $tmp/mdt || {
+                       $r losetup -a
                        error_noexit "tunefs.lustre before remounting the MDT"
                        return 1
                }

diff --git a/lustre/utils/mkfs_lustre.c b/lustre/utils/mkfs_lustre.c
index ec85e08..49eb00a 100644 (file)
--- a/lustre/utils/mkfs_lustre.c
+++ b/lustre/utils/mkfs_lustre.c
@@ -540,13 +540,14 @@ int parse_opts(int argc, char *const argv[], struct mkfs_opts *mop,
 
 int main(int argc, char *const argv[])
 {
-        struct mkfs_opts mop;
-        struct lustre_disk_data *ldd;
-        char *mountopts = NULL;
-        char always_mountopts[512] = "";
-        char default_mountopts[512] = "";
+       struct mkfs_opts mop;
+       struct lustre_disk_data *ldd;
+       char *mountopts = NULL;
+       char always_mountopts[512] = "";
+       char default_mountopts[512] = "";
        unsigned mount_type;
-        int ret = 0;
+       int ret = 0;
+       int ret2 = 0;
 
         if ((progname = strrchr(argv[0], '/')) != NULL)
                 progname++;
@@ -802,8 +803,10 @@ int main(int argc, char *const argv[])
         }
 
 out:
-        loop_cleanup(&mop);
        osd_fini();
+       ret2 = loop_cleanup(&mop);
+       if (ret == 0)
+               ret = ret2;
 
         /* Fix any crazy return values from system() */
         if (ret && ((ret & 255) == 0))

diff --git a/lustre/utils/mount_lustre.c b/lustre/utils/mount_lustre.c
index 20abe44..4276e5c 100644 (file)
--- a/lustre/utils/mount_lustre.c
+++ b/lustre/utils/mount_lustre.c
@@ -320,6 +320,7 @@ static int clear_update_ondisk(char *source, struct lustre_disk_data *ldd)
        char default_mountopts[512] = "";
        struct mkfs_opts mkop;
        int ret;
+       int ret2;
 
        memset(&mkop, 0, sizeof(mkop));
        mkop.mo_ldd = *ldd;
@@ -366,7 +367,10 @@ static int clear_update_ondisk(char *source, struct lustre_disk_data *ldd)
                fprintf(stderr, "failed to write local files: %s\n",
                        strerror(ret));
        }
-       loop_cleanup(&mkop);
+
+       ret2 = loop_cleanup(&mkop);
+       if (ret == 0)
+               ret = ret2;
 
        return ret;
 }

diff --git a/lustre/utils/mount_utils.c b/lustre/utils/mount_utils.c
index e0502fe..5c9a845 100644 (file)
--- a/lustre/utils/mount_utils.c
+++ b/lustre/utils/mount_utils.c
@@ -368,11 +368,23 @@ int loop_setup(struct mkfs_opts *mop)
 int loop_cleanup(struct mkfs_opts *mop)
 {
        char cmd[150];
-       int ret = 1;
+       int ret = 0;
+
        if ((mop->mo_flags & MO_IS_LOOP) && *mop->mo_loopdev) {
+               int tries;
+
                sprintf(cmd, "losetup -d %s", mop->mo_loopdev);
-               ret = run_command(cmd, sizeof(cmd));
+               for (tries = 0; tries < 3; tries++) {
+                       ret = run_command(cmd, sizeof(cmd));
+                       if (ret == 0)
+                               break;
+                       sleep(1);
+               }
        }
+
+       if (ret != 0)
+               fprintf(stderr, "cannot cleanup %s: rc = %d\n",
+                       mop->mo_loopdev, ret);
        return ret;
 }
 

diff --git a/lustre/utils/mount_utils_ldiskfs.c b/lustre/utils/mount_utils_ldiskfs.c
index 728c349..e91a0aa 100644 (file)
--- a/lustre/utils/mount_utils_ldiskfs.c
+++ b/lustre/utils/mount_utils_ldiskfs.c
@@ -386,7 +386,7 @@ static int is_e2fsprogs_feature_supp(const char *feature)
                }
                ret = fread(supp_features, 1, sizeof(supp_features) - 1, fp);
                supp_features[ret] = '\0';
-               fclose(fp);
+               pclose(fp);
        }
        if (ret > 0 && strstr(supp_features,
                              strncmp(feature, "-O ", 3) ? feature : feature+3))
@@ -1217,7 +1217,7 @@ static int is_feature_enabled(const char *feature, const char *devpath)
 
        ret = fread(enabled_features, 1, sizeof(enabled_features) - 1, fp);
        enabled_features[ret] = '\0';
-       fclose(fp);
+       pclose(fp);
 
        if (strstr(enabled_features, feature))
                return 1;