public inbox for fstests@vger.kernel.org
 help / color / mirror / Atom feed
From: Zorro Lang <zlang@redhat.com>
To: fstests@vger.kernel.org
Cc: eguan@redhat.com, Zorro Lang <zlang@redhat.com>
Subject: [PATCH v5 1/2] common/dmerror: fix nonsensical arguments handling
Date: Tue,  5 Jul 2016 17:30:23 +0800	[thread overview]
Message-ID: <1467711024-12702-1-git-send-email-zlang@redhat.com> (raw)

By default, _dmerror_load_*_table() suspends the dm device with
"--nolockfs" option. Callers have to feed two arguments to these
functions to change the behavior, with the second being 1, but the
first argument is not used at all, which doesn't make sense.

Fix it by checking if the first argument is "lockfs" and removing
"--nolockfs" option if so, or passing all options to dmsetup.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---

Hi,

At first, I want to change the code just likes:

  suspend_opts="$*"

But after talked with Darrick J. Wong(the original author of
dmerror), he suggest to keep use --nolockfs as default:

  <djwong> i used --nolockfs so that dmsetup doesn't try to flush
           the fs on the device that we're deliberately erroring
  <djwong> (most probably the test is midway through its own fsync,
           so dmsetup trying to sync the fs will just get hung up
           on the io errors)

So follow this suggestion, and keep --nolockfs still as the default
option.

Thanks,
Zorro

 common/dmerror | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/common/dmerror b/common/dmerror
index 6df87bd..5ad9994 100644
--- a/common/dmerror
+++ b/common/dmerror
@@ -68,7 +68,12 @@ _dmerror_cleanup()
 _dmerror_load_error_table()
 {
 	suspend_opt="--nolockfs"
-	[ $# -gt 1 ] && [ $2 -eq 1 ] && suspend_opt=""
+
+	if [ "$1" = "lockfs" ]; then
+		suspend_opt=""
+	elif [ -n "$*" ]; then
+		suspend_opt="$*"
+	fi
 
 	$DMSETUP_PROG suspend $suspend_opt error-test
 	[ $? -ne 0 ] && _fail  "dmsetup suspend failed"
@@ -83,7 +88,12 @@ _dmerror_load_error_table()
 _dmerror_load_working_table()
 {
 	suspend_opt="--nolockfs"
-	[ $# -gt 1 ] && [ $2 -eq 1 ] && suspend_opt=""
+
+	if [ "$1" = "lockfs" ]; then
+		suspend_opt=""
+	elif [ -n "$*" ]; then
+		suspend_opt="$*"
+	fi
 
 	$DMSETUP_PROG suspend $suspend_opt error-test
 	[ $? -ne 0 ] && _fail  "dmsetup suspend failed"
-- 
2.5.5


             reply	other threads:[~2016-07-05  9:30 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-05  9:30 Zorro Lang [this message]
2016-07-05  9:30 ` [PATCH v5 2/2] xfs: configurable behavior on errors at unmount time Zorro Lang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1467711024-12702-1-git-send-email-zlang@redhat.com \
    --to=zlang@redhat.com \
    --cc=eguan@redhat.com \
    --cc=fstests@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox