From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f194.google.com ([209.85.192.194]:38302 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750895AbdITCcG (ORCPT ); Tue, 19 Sep 2017 22:32:06 -0400 Received: by mail-pf0-f194.google.com with SMTP id a7so615401pfj.5 for ; Tue, 19 Sep 2017 19:32:06 -0700 (PDT) Date: Wed, 20 Sep 2017 11:32:12 +0900 Message-ID: <87tvzyp0b7.wl-satoru.takeuchi@gmail.com> From: Satoru Takeuchi To: "Misono, Tomohiro" Cc: Subject: Re: [PATCH] btrfs-progs: subvolume: outputs message only when operation succeeds In-Reply-To: References: MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Sender: linux-btrfs-owner@vger.kernel.org List-ID: At Tue, 19 Sep 2017 16:41:52 +0900, Misono, Tomohiro wrote: > > "btrfs subvolume create/delete" outputs the message of "Create/Delete > subvolume ..." even when an operation fails. > Since it is confusing, let's outputs the message only when an operation succeeds. > > Signed-off-by: Tomohiro Misono Current message as follows is odd as you said. ``` Create subvolume './test' ERROR: cannot create subvolume: No such file or directory ``` It's ambiguous for users to know whether creating subvolume succeeded or not. I tested this patch with injecting error on ioctl() for subvol creation/deletion. Reviewed-by: Satoru Takeuchi Tested-by: Satoru Takeuchi > --- > cmds-subvolume.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/cmds-subvolume.c b/cmds-subvolume.c > index 666f6e0..6d4b0fe 100644 > --- a/cmds-subvolume.c > +++ b/cmds-subvolume.c > @@ -189,7 +189,6 @@ static int cmd_subvol_create(int argc, char **argv) > if (fddst < 0) > goto out; > > - printf("Create subvolume '%s/%s'\n", dstdir, newname); > if (inherit) { > struct btrfs_ioctl_vol_args_v2 args; > > @@ -213,6 +212,7 @@ static int cmd_subvol_create(int argc, char **argv) > error("cannot create subvolume: %s", strerror(errno)); > goto out; > } > + printf("Create subvolume '%s/%s'\n", dstdir, newname); > > retval = 0; /* success */ > out: > @@ -337,9 +337,6 @@ again: > goto out; > } > > - printf("Delete subvolume (%s): '%s/%s'\n", > - commit_mode == 2 || (commit_mode == 1 && cnt + 1 == argc) > - ? "commit" : "no-commit", dname, vname); > memset(&args, 0, sizeof(args)); > strncpy_null(args.name, vname); > res = ioctl(fd, BTRFS_IOC_SNAP_DESTROY, &args); > @@ -349,6 +346,9 @@ again: > ret = 1; > goto out; > } > + printf("Delete subvolume (%s): '%s/%s'\n", > + commit_mode == 2 || (commit_mode == 1 && cnt + 1 == argc) > + ? "commit" : "no-commit", dname, vname); > > if (commit_mode == 1) { > res = wait_for_commit(fd); > -- > 2.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html