* [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-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* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox