From 98ac0fe3a45dde62759ecaa4c84e6250ac2067f8 Mon Sep 17 00:00:00 2001 From: wang di Date: Sat, 30 Nov 2013 07:40:22 -0800 Subject: [PATCH 1/1] 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. Signed-off-by: John L. Hammond Signed-off-by: wang di Change-Id: I0051de9b5a39d487fe34554c79773de4635178cc Reviewed-on: http://review.whamcloud.com/8409 Tested-by: Jenkins Reviewed-by: Andreas Dilger Tested-by: Maloo --- 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 af12da1..cc0f3dc 100644 --- a/lustre/tests/conf-sanity.sh +++ b/lustre/tests/conf-sanity.sh @@ -1391,16 +1391,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 } @@ -1535,6 +1539,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 } @@ -1542,6 +1547,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 } @@ -1567,6 +1573,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 } @@ -1618,6 +1625,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 } @@ -1866,6 +1874,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