| Message ID | 20200928150729.2239-1-realwakka@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: subvolume: Add warning on deleting default subvolume | expand |
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) ? >
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.
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) ?
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.
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.
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) ?
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(+)