| Message ID | 1457583735-3925-1-git-send-email-anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Mar 10, 2016 at 12:22:15PM +0800, Anand Jain wrote: > This patch will log return value of add/del_qgroup_relation() and pass the > err code of btrfs_run_qgroups to the btrfs_std_error(). > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > v2: fix the forgotten git commit amend, to take in compile fail, sorry > > fs/btrfs/ioctl.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > index f704d1c..f43a104 100644 > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -4803,10 +4803,15 @@ static long btrfs_ioctl_qgroup_assign(struct file *file, void __user *arg) > sa->src, sa->dst); > } > > + if (ret) > + btrfs_err(root->fs_info, > + "add/del qgroup relation failed, assign %llu ret %d", > + sa->assign, ret); It's just logging an error, but it should really fail, without a message. The error code will tell what went wrong. Assigning existing qgroups is eg. EEXIST, if either doesn't exist then ENOENT. There's a FIXME a few lines above that should be addressed. I'd like to avoid the message in this case because it should be handled by the ioctl caller, otherwise it could be just flooding the system log. > + > /* update qgroup status and info */ > err = btrfs_run_qgroups(trans, root->fs_info); > if (err < 0) > - btrfs_std_error(root->fs_info, ret, > + btrfs_std_error(root->fs_info, err, > "failed to update qgroup status and info\n"); > err = btrfs_end_transaction(trans, root); > if (err && !ret) -- 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
On 03/16/2016 09:10 PM, David Sterba wrote: > On Thu, Mar 10, 2016 at 12:22:15PM +0800, Anand Jain wrote: >> This patch will log return value of add/del_qgroup_relation() and pass the >> err code of btrfs_run_qgroups to the btrfs_std_error(). >> >> Signed-off-by: Anand Jain <anand.jain@oracle.com> >> --- >> v2: fix the forgotten git commit amend, to take in compile fail, sorry >> >> fs/btrfs/ioctl.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c >> index f704d1c..f43a104 100644 >> --- a/fs/btrfs/ioctl.c >> +++ b/fs/btrfs/ioctl.c >> @@ -4803,10 +4803,15 @@ static long btrfs_ioctl_qgroup_assign(struct file *file, void __user *arg) >> sa->src, sa->dst); >> } >> >> + if (ret) >> + btrfs_err(root->fs_info, >> + "add/del qgroup relation failed, assign %llu ret %d", >> + sa->assign, ret); > > It's just logging an error, but it should really fail, without a > message. The error code will tell what went wrong. Assigning existing > qgroups is eg. EEXIST, if either doesn't exist then ENOENT. There's a > FIXME a few lines above that should be addressed. > > I'd like to avoid the message in this case because it should be handled > by the ioctl caller, otherwise it could be just flooding the system log. > >> + >> /* update qgroup status and info */ >> err = btrfs_run_qgroups(trans, root->fs_info); >> if (err < 0) >> - btrfs_std_error(root->fs_info, ret, >> + btrfs_std_error(root->fs_info, err, This patch is missing in the 'dev-del-by-id' branch. I agree with your the above comments, however the key intention of this patch was to get the correct parameter passed to the btrfs_std_error(). And the above btrfs_err() log should still help/motivate to get the part you mentioned corrected. Thanks, -Anand >> "failed to update qgroup status and info\n"); >> err = btrfs_end_transaction(trans, root); >> if (err && !ret) -- 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
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index f704d1c..f43a104 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -4803,10 +4803,15 @@ static long btrfs_ioctl_qgroup_assign(struct file *file, void __user *arg) sa->src, sa->dst); } + if (ret) + btrfs_err(root->fs_info, + "add/del qgroup relation failed, assign %llu ret %d", + sa->assign, ret); + /* update qgroup status and info */ err = btrfs_run_qgroups(trans, root->fs_info); if (err < 0) - btrfs_std_error(root->fs_info, ret, + btrfs_std_error(root->fs_info, err, "failed to update qgroup status and info\n"); err = btrfs_end_transaction(trans, root); if (err && !ret)
This patch will log return value of add/del_qgroup_relation() and pass the err code of btrfs_run_qgroups to the btrfs_std_error(). Signed-off-by: Anand Jain <anand.jain@oracle.com> --- v2: fix the forgotten git commit amend, to take in compile fail, sorry fs/btrfs/ioctl.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)