From 4534278809c71757ad31ed39052f4a1665e5de25 Mon Sep 17 00:00:00 2001 From: wang di Date: Sat, 4 Jan 2014 22:15:59 +0800 Subject: [PATCH] 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 Change-Id: Ic1ebc2a6b2ce4280c2123080171e203e99267b28 Signed-off-by: Jian Yu Reviewed-on: http://review.whamcloud.com/8723 Tested-by: Jenkins Reviewed-by: Andreas Dilger Tested-by: Maloo Reviewed-by: Oleg Drokin --- lustre/tests/conf-sanity.sh | 13 +++++++++++-- lustre/utils/mkfs_lustre.c | 17 ++++++++++------- lustre/utils/mount_lustre.c | 6 +++++- lustre/utils/mount_utils.c | 16 ++++++++++++++-- lustre/utils/mount_utils_ldiskfs.c | 4 ++-- 5 files changed, 42 insertions(+), 14 deletions(-) diff --git a/lustre/tests/conf-sanity.sh b/lustre/tests/conf-sanity.sh index ae51f6a..8552663 100644 --- 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 --- 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 --- 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 --- 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 --- 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; -- 1.8.3.1