From patchwork Fri Oct 10 13:16:16 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eryu Guan X-Patchwork-Id: 5065351 Return-Path: X-Original-To: patchwork-linux-btrfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id E836D9F2F1 for ; Fri, 10 Oct 2014 13:18:16 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id E869A20212 for ; Fri, 10 Oct 2014 13:18:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E8B8C2020E for ; Fri, 10 Oct 2014 13:18:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752614AbaJJNSL (ORCPT ); Fri, 10 Oct 2014 09:18:11 -0400 Received: from mail-pd0-f176.google.com ([209.85.192.176]:53006 "EHLO mail-pd0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751684AbaJJNSK (ORCPT ); Fri, 10 Oct 2014 09:18:10 -0400 Received: by mail-pd0-f176.google.com with SMTP id fp1so1635809pdb.7 for ; Fri, 10 Oct 2014 06:18:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=L3zoXcqd15DV/EnDnM5jy55qWM7GVxI++wUinyncS44=; b=dmgeCUCcZwJkQAk7NHmWId+9mRTPCMKA5MC1o7/qU4FoxUSnpjlM6+rD7whqu9eWA6 j9pzE/bCaCD5sNvdqmFtmF9yujgwRbyD9OqQnZ5Y0Oxe9PnsBz5dcsw38j43Zy/HQdqi QkvTBFNqBqJUobjNweHL9x19M8CdPxIAP9JGTKAXOFSgpkziJwn4u87w/B2CFi6ANHJc XhrC7mFxAfKkFrGeiW4gDdojdQ9qZZwB7hPNlkrdZHJHZrLtCeFSb/V0rWmiuyf0bRAD IXT4/e3+GGfezOlIiK/aJ1i3HlE/35dfhpYhMrFfba+64KpCeFRYF+lHC0t1iDhouxZs byEQ== X-Received: by 10.67.24.8 with SMTP id ie8mr5059606pad.21.1412947089513; Fri, 10 Oct 2014 06:18:09 -0700 (PDT) Received: from localhost ([203.114.244.88]) by mx.google.com with ESMTPSA id sb2sm3501701pbc.24.2014.10.10.06.18.08 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 10 Oct 2014 06:18:08 -0700 (PDT) From: Eryu Guan To: linux-btrfs@vger.kernel.org Cc: Eryu Guan Subject: [PATCH v2 1/2] btrfs-progs: only report internal dev replace result if there's a result Date: Fri, 10 Oct 2014 21:16:16 +0800 Message-Id: <1412946976-28364-1-git-send-email-guaneryu@gmail.com> X-Mailer: git-send-email 1.9.3 In-Reply-To: <20141010082023.GF22943@twin.jikos.cz> References: <20141010082023.GF22943@twin.jikos.cz> Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Spam-Status: No, score=-7.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP If BTRFS_IOC_DEV_REPLACE ioctl failed, args.result usually won't be updated by the ioctl. And the arg has been initialized with 0, the result is always 0, which is BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR, and the resulting error message looks confusing: ERROR: ioctl(DEV_REPLACE_START) failed on "/mnt/btrfs": No such file or directory, no error But in case there's an internal result returned in future, don't drop the result completely, instead print dev replace result message only if the result is updated by a failed ioctl call. Signed-off-by: Eryu Guan --- I didn't update replace_dev_result2string() function, which seems not necessary here. And I personally still prefer v1 :) v2: - don't drop the result completely but print result if it's got updated cmds-replace.c | 44 ++++++++++++++++++++++++++++++++------------ ioctl.h | 1 + 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/cmds-replace.c b/cmds-replace.c index 9fe7ad8..bfcc161 100644 --- a/cmds-replace.c +++ b/cmds-replace.c @@ -184,12 +184,17 @@ static int cmd_start_replace(int argc, char **argv) /* check for possible errors before backgrounding */ status_args.cmd = BTRFS_IOCTL_DEV_REPLACE_CMD_STATUS; + status_args.result = BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_RESULT; ret = ioctl(fdmnt, BTRFS_IOC_DEV_REPLACE, &status_args); if (ret) { fprintf(stderr, - "ERROR: ioctl(DEV_REPLACE_STATUS) failed on \"%s\": %s, %s\n", - path, strerror(errno), - replace_dev_result2string(status_args.result)); + "ERROR: ioctl(DEV_REPLACE_STATUS) failed on \"%s\": %s", + path, strerror(errno)); + if (status_args.result != BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_RESULT) + fprintf(stderr, ", %s\n", + replace_dev_result2string(status_args.result)); + else + fprintf(stderr, "\n"); goto leave_with_error; } @@ -301,13 +306,18 @@ static int cmd_start_replace(int argc, char **argv) } start_args.cmd = BTRFS_IOCTL_DEV_REPLACE_CMD_START; + start_args.result = BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_RESULT; ret = ioctl(fdmnt, BTRFS_IOC_DEV_REPLACE, &start_args); if (do_not_background) { if (ret) { fprintf(stderr, - "ERROR: ioctl(DEV_REPLACE_START) failed on \"%s\": %s, %s\n", - path, strerror(errno), - replace_dev_result2string(start_args.result)); + "ERROR: ioctl(DEV_REPLACE_START) failed on \"%s\": %s", + path, strerror(errno)); + if (start_args.result != BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_RESULT) + fprintf(stderr, ", %s\n", + replace_dev_result2string(start_args.result)); + else + fprintf(stderr, "\n"); if (errno == EOPNOTSUPP) fprintf(stderr, @@ -402,11 +412,16 @@ static int print_replace_status(int fd, const char *path, int once) for (;;) { args.cmd = BTRFS_IOCTL_DEV_REPLACE_CMD_STATUS; + args.result = BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_RESULT; ret = ioctl(fd, BTRFS_IOC_DEV_REPLACE, &args); if (ret) { - fprintf(stderr, "ERROR: ioctl(DEV_REPLACE_STATUS) failed on \"%s\": %s, %s\n", - path, strerror(errno), - replace_dev_result2string(args.result)); + fprintf(stderr, "ERROR: ioctl(DEV_REPLACE_STATUS) failed on \"%s\": %s", + path, strerror(errno)); + if (args.result != BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_RESULT) + fprintf(stderr, ", %s\n", + replace_dev_result2string(args.result)); + else + fprintf(stderr, "\n"); return ret; } @@ -550,13 +565,18 @@ static int cmd_cancel_replace(int argc, char **argv) } args.cmd = BTRFS_IOCTL_DEV_REPLACE_CMD_CANCEL; + args.result = BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_RESULT; ret = ioctl(fd, BTRFS_IOC_DEV_REPLACE, &args); e = errno; close_file_or_dir(fd, dirstream); if (ret) { - fprintf(stderr, "ERROR: ioctl(DEV_REPLACE_CANCEL) failed on \"%s\": %s, %s\n", - path, strerror(e), - replace_dev_result2string(args.result)); + fprintf(stderr, "ERROR: ioctl(DEV_REPLACE_CANCEL) failed on \"%s\": %s", + path, strerror(e)); + if (args.result != BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_RESULT) + fprintf(stderr, ", %s\n", + replace_dev_result2string(args.result)); + else + fprintf(stderr, "\n"); return 1; } if (args.result == BTRFS_IOCTL_DEV_REPLACE_RESULT_NOT_STARTED) { diff --git a/ioctl.h b/ioctl.h index f0fc060..df9e4b7 100644 --- a/ioctl.h +++ b/ioctl.h @@ -141,6 +141,7 @@ struct btrfs_ioctl_dev_replace_status_params { #define BTRFS_IOCTL_DEV_REPLACE_CMD_START 0 #define BTRFS_IOCTL_DEV_REPLACE_CMD_STATUS 1 #define BTRFS_IOCTL_DEV_REPLACE_CMD_CANCEL 2 +#define BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_RESULT -1 #define BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR 0 #define BTRFS_IOCTL_DEV_REPLACE_RESULT_NOT_STARTED 1 #define BTRFS_IOCTL_DEV_REPLACE_RESULT_ALREADY_STARTED 2