All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] btrfs-progs: subvolume: Add warning on deleting default subvolume
  2020-09-30  1:33 ` Su Yue
@ 2020-09-24 12:45   ` Sidong Yang
  2020-09-30 16:26     ` David Sterba
  2020-09-30 16:27   ` David Sterba
  1 sibling, 1 reply; 6+ messages in thread
From: Sidong Yang @ 2020-09-24 12:45 UTC (permalink / raw)
  To: Su Yue; +Cc: linux-btrfs, David Sterba

On Wed, Sep 30, 2020 at 09:33:31AM +0800, Su Yue wrote:
> 
> On Mon 28 Sep 2020 at 23:07, Sidong Yang <realwakka@gmail.com> wrote:
> 
> > This patch add warning messages when user try to delete default
> > subvolume. When deleting default subvolume, kernel will not allow and
> > make error message on syslog. but there is only message that permission
> > denied on userspace. User can be noticed the reason by this warning
> > message.
> > 
> > This patch implements github issue.
> > https://github.com/kdave/btrfs-progs/issues/274
> > 
> > Signed-off-by: Sidong Yang <realwakka@gmail.com>
> > ---
> >  cmds/subvolume.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/cmds/subvolume.c b/cmds/subvolume.c
> > index 2020e486..0cdf7a68 100644
> > --- a/cmds/subvolume.c
> > +++ b/cmds/subvolume.c
> > @@ -264,6 +264,7 @@ static int cmd_subvol_delete(const struct cmd_struct
> > *cmd,
> >  	struct seen_fsid *seen_fsid_hash[SEEN_FSID_HASH_SIZE] = {  NULL, };
> >  	enum { COMMIT_AFTER = 1, COMMIT_EACH = 2 };
> >  	enum btrfs_util_error err;
> > +	uint64_t default_subvol_id = 0, target_subvol_id = 0;
> > 
> >  	optind = 0;
> >  	while (1) {
> > @@ -360,6 +361,25 @@ again:
> >  		goto out;
> >  	}
> > 
> > +	err = btrfs_util_get_default_subvolume_fd(fd, &default_subvol_id);
> > +	if (fd < 0) {
> > 
> Should it be
> "     if (err) { |
>         error_btrfs_util(err);
>         ...
> "?

Hi Su! Thanks for review!

Yeah, I think it's definitely wrong. My mistake.

> 
> > +		ret = 1;
> > +		goto out;
> > +	}
> > +
> > +	if (subvolid > 0)
> > +		target_subvol_id = subvolid;
> > +	else {
> > +		err = btrfs_util_subvolume_id(path, &target_subvol_id);
> > +		if (fd < 0) {
> > 
> And here.
> 

It's wrong too.

Dave, maybe this patch needs for Su's review.
Usually in this case, Could you fix it directly ?
or do I need to send a new patch?

Thanks,
Sidong

> > +			ret = 1;
> > +			goto out;
> > +		}
> > +	}
> > +
> > +	if (target_subvol_id == default_subvol_id)
> > +		warning("trying to delete default subvolume.");
> > +
> >  	pr_verbose(MUST_LOG, "Delete subvolume (%s): ",
> >  		commit_mode == COMMIT_EACH ||
> >  		(commit_mode == COMMIT_AFTER && cnt + 1 == argc) ?
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] btrfs-progs: subvolume: Add warning on deleting default subvolume
@ 2020-09-28 15:07 Sidong Yang
  2020-09-29 21:37 ` David Sterba
  2020-09-30  1:33 ` Su Yue
  0 siblings, 2 replies; 6+ messages in thread
From: Sidong Yang @ 2020-09-28 15:07 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Sidong Yang

This patch add warning messages when user try to delete default
subvolume. When deleting default subvolume, kernel will not allow and
make error message on syslog. but there is only message that permission
denied on userspace. User can be noticed the reason by this warning message.

This patch implements github issue.
https://github.com/kdave/btrfs-progs/issues/274

Signed-off-by: Sidong Yang <realwakka@gmail.com>
---
 cmds/subvolume.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/cmds/subvolume.c b/cmds/subvolume.c
index 2020e486..0cdf7a68 100644
--- a/cmds/subvolume.c
+++ b/cmds/subvolume.c
@@ -264,6 +264,7 @@ static int cmd_subvol_delete(const struct cmd_struct *cmd,
 	struct seen_fsid *seen_fsid_hash[SEEN_FSID_HASH_SIZE] = { NULL, };
 	enum { COMMIT_AFTER = 1, COMMIT_EACH = 2 };
 	enum btrfs_util_error err;
+	uint64_t default_subvol_id = 0, target_subvol_id = 0;
 
 	optind = 0;
 	while (1) {
@@ -360,6 +361,25 @@ again:
 		goto out;
 	}
 
+	err = btrfs_util_get_default_subvolume_fd(fd, &default_subvol_id);
+	if (fd < 0) {
+		ret = 1;
+		goto out;
+	}
+
+	if (subvolid > 0)
+		target_subvol_id = subvolid;
+	else {
+		err = btrfs_util_subvolume_id(path, &target_subvol_id);
+		if (fd < 0) {
+			ret = 1;
+			goto out;
+		}
+	}
+
+	if (target_subvol_id == default_subvol_id)
+		warning("trying to delete default subvolume.");
+
 	pr_verbose(MUST_LOG, "Delete subvolume (%s): ",
 		commit_mode == COMMIT_EACH ||
 		(commit_mode == COMMIT_AFTER && cnt + 1 == argc) ?
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs-progs: subvolume: Add warning on deleting default subvolume
  2020-09-28 15:07 [PATCH] btrfs-progs: subvolume: Add warning on deleting default subvolume Sidong Yang
@ 2020-09-29 21:37 ` David Sterba
  2020-09-30  1:33 ` Su Yue
  1 sibling, 0 replies; 6+ messages in thread
From: David Sterba @ 2020-09-29 21:37 UTC (permalink / raw)
  To: Sidong Yang; +Cc: linux-btrfs

On Mon, Sep 28, 2020 at 03:07:29PM +0000, Sidong Yang wrote:
> This patch add warning messages when user try to delete default
> subvolume. When deleting default subvolume, kernel will not allow and
> make error message on syslog. but there is only message that permission
> denied on userspace. User can be noticed the reason by this warning message.
> 
> This patch implements github issue.
> https://github.com/kdave/btrfs-progs/issues/274
> 
> Signed-off-by: Sidong Yang <realwakka@gmail.com>

Thanks.

> ---
>  cmds/subvolume.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/cmds/subvolume.c b/cmds/subvolume.c
> index 2020e486..0cdf7a68 100644
> --- a/cmds/subvolume.c
> +++ b/cmds/subvolume.c
> @@ -264,6 +264,7 @@ static int cmd_subvol_delete(const struct cmd_struct *cmd,
>  	struct seen_fsid *seen_fsid_hash[SEEN_FSID_HASH_SIZE] = { NULL, };
>  	enum { COMMIT_AFTER = 1, COMMIT_EACH = 2 };
>  	enum btrfs_util_error err;
> +	uint64_t default_subvol_id = 0, target_subvol_id = 0;
>  
>  	optind = 0;
>  	while (1) {
> @@ -360,6 +361,25 @@ again:
>  		goto out;
>  	}
>  
> +	err = btrfs_util_get_default_subvolume_fd(fd, &default_subvol_id);
> +	if (fd < 0) {
> +		ret = 1;
> +		goto out;
> +	}
> +
> +	if (subvolid > 0)
> +		target_subvol_id = subvolid;
> +	else {
> +		err = btrfs_util_subvolume_id(path, &target_subvol_id);
> +		if (fd < 0) {
> +			ret = 1;
> +			goto out;
> +		}
> +	}
> +
> +	if (target_subvol_id == default_subvol_id)
> +		warning("trying to delete default subvolume.");

I've changed that to skip the deletion and added id and path to the
message, otherwise it's not clear which one it was. Also I've added a
test.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs-progs: subvolume: Add warning on deleting default subvolume
  2020-09-28 15:07 [PATCH] btrfs-progs: subvolume: Add warning on deleting default subvolume Sidong Yang
  2020-09-29 21:37 ` David Sterba
@ 2020-09-30  1:33 ` Su Yue
  2020-09-24 12:45   ` Sidong Yang
  2020-09-30 16:27   ` David Sterba
  1 sibling, 2 replies; 6+ messages in thread
From: Su Yue @ 2020-09-30  1:33 UTC (permalink / raw)
  To: Sidong Yang; +Cc: linux-btrfs, David Sterba


On Mon 28 Sep 2020 at 23:07, Sidong Yang <realwakka@gmail.com> 
wrote:

> This patch add warning messages when user try to delete default
> subvolume. When deleting default subvolume, kernel will not 
> allow and
> make error message on syslog. but there is only message that 
> permission
> denied on userspace. User can be noticed the reason by this 
> warning message.
>
> This patch implements github issue.
> https://github.com/kdave/btrfs-progs/issues/274
>
> Signed-off-by: Sidong Yang <realwakka@gmail.com>
> ---
>  cmds/subvolume.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/cmds/subvolume.c b/cmds/subvolume.c
> index 2020e486..0cdf7a68 100644
> --- a/cmds/subvolume.c
> +++ b/cmds/subvolume.c
> @@ -264,6 +264,7 @@ static int cmd_subvol_delete(const struct 
> cmd_struct *cmd,
>  	struct seen_fsid *seen_fsid_hash[SEEN_FSID_HASH_SIZE] = { 
>  NULL, };
>  	enum { COMMIT_AFTER = 1, COMMIT_EACH = 2 };
>  	enum btrfs_util_error err;
> +	uint64_t default_subvol_id = 0, target_subvol_id = 0;
>
>  	optind = 0;
>  	while (1) {
> @@ -360,6 +361,25 @@ again:
>  		goto out;
>  	}
>
> +	err = btrfs_util_get_default_subvolume_fd(fd, 
> &default_subvol_id);
> +	if (fd < 0) {
>
Should it be
"     if (err) { 
|
         error_btrfs_util(err);
         ...
"?

> +		ret = 1;
> +		goto out;
> +	}
> +
> +	if (subvolid > 0)
> +		target_subvol_id = subvolid;
> +	else {
> +		err = btrfs_util_subvolume_id(path, &target_subvol_id);
> +		if (fd < 0) {
>
And here.

> +			ret = 1;
> +			goto out;
> +		}
> +	}
> +
> +	if (target_subvol_id == default_subvol_id)
> +		warning("trying to delete default subvolume.");
> +
>  	pr_verbose(MUST_LOG, "Delete subvolume (%s): ",
>  		commit_mode == COMMIT_EACH ||
>  		(commit_mode == COMMIT_AFTER && cnt + 1 == argc) ?


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs-progs: subvolume: Add warning on deleting default subvolume
  2020-09-24 12:45   ` Sidong Yang
@ 2020-09-30 16:26     ` David Sterba
  0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2020-09-30 16:26 UTC (permalink / raw)
  To: Sidong Yang; +Cc: Su Yue, linux-btrfs, David Sterba

On Thu, Sep 24, 2020 at 12:45:13PM +0000, Sidong Yang wrote:
> On Wed, Sep 30, 2020 at 09:33:31AM +0800, Su Yue wrote:
> > On Mon 28 Sep 2020 at 23:07, Sidong Yang <realwakka@gmail.com> wrote:
> > > +	err = btrfs_util_get_default_subvolume_fd(fd, &default_subvol_id);
> > > +	if (fd < 0) {
> > > 
> > Should it be
> > "     if (err) { |
> >         error_btrfs_util(err);
> >         ...
> > "?
> 
> Hi Su! Thanks for review!
> 
> Yeah, I think it's definitely wrong. My mistake.
> 
> > > +		err = btrfs_util_subvolume_id(path, &target_subvol_id);
> > > +		if (fd < 0) {
> > > 
> > And here.
> > 
> 
> It's wrong too.
> 
> Dave, maybe this patch needs for Su's review.
> Usually in this case, Could you fix it directly ?
> or do I need to send a new patch?

Yes I'll fix it in the commit, no need to resend.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs-progs: subvolume: Add warning on deleting default subvolume
  2020-09-30  1:33 ` Su Yue
  2020-09-24 12:45   ` Sidong Yang
@ 2020-09-30 16:27   ` David Sterba
  1 sibling, 0 replies; 6+ messages in thread
From: David Sterba @ 2020-09-30 16:27 UTC (permalink / raw)
  To: Su Yue; +Cc: Sidong Yang, linux-btrfs, David Sterba

On Wed, Sep 30, 2020 at 09:33:31AM +0800, Su Yue wrote:
> On Mon 28 Sep 2020 at 23:07, Sidong Yang <realwakka@gmail.com> 
> > +	err = btrfs_util_get_default_subvolume_fd(fd, 
> > &default_subvol_id);
> > +	if (fd < 0) {
> >
> Should it be
> "     if (err) { 
> |
>          error_btrfs_util(err);
>          ...
> "?

> > +		err = btrfs_util_subvolume_id(path, &target_subvol_id);
> > +		if (fd < 0) {
> >
> And here.

Thanks for catching it, I missed it too.

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-30 16:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-28 15:07 [PATCH] btrfs-progs: subvolume: Add warning on deleting default subvolume Sidong Yang
2020-09-29 21:37 ` David Sterba
2020-09-30  1:33 ` Su Yue
2020-09-24 12:45   ` Sidong Yang
2020-09-30 16:26     ` David Sterba
2020-09-30 16:27   ` David Sterba

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.