fs/btrfs: Return EPERM for rmdir on subvolumes and snapshots
PATCH 1/1 fs/btrfs: Return EPERM for rmdir on subvolumes and snapshots
Break the conditional to return EPERM for subvolumes,snapshots and ENOTEMPTY for normal directories with files.
--- fs/btrfs/inode.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)
fs/btrfs/inode.c +5 -3
@@ -2591,9 +2591,11 @@ static int btrfs_rmdir(struct inode *dir, struct dentry *dentry)struct btrfs_trans_handle *trans;unsigned long nr =0 ; -if (inode->i_size >BTRFS_EMPTY_DIR_SIZE || - inode->i_ino ==BTRFS_FIRST_FREE_OBJECTID ) -return -ENOTEMPTY ; +if (inode->i_size >BTRFS_EMPTY_DIR_SIZE ) +return -ENOTEMPTY ; + +if (inode->i_ino ==BTRFS_FIRST_FREE_OBJECTID ) +return -EPERM ; ret =btrfs_reserve_metadata_space (root,5 );if (ret)
Signature · 1 line
1.6.6.1
Hi all,
Can I suggest to return -EINVAL instead of -EPERM ?
To me EPERM seems that the user don't have the right to perform an action. But the problem is that "rm" is not the right command to use in order to delete a subvolume.
As side note, what is the reason for which an user is able to create a subvolume, but not to destroy it ?
BR
Goffredo
fs/btrfs/inode.c static int btrfs_rmdir(struct inode *dir, struct
Quoted 13 lines · Harshavardhana
Break the conditional to return EPERM for subvolumes,snapshots and ENOTEMPTY for normal directories with files.
Signed-off-by Harshavardhana <[email protected]>--- fs/btrfs/inode.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)fs/btrfs/inode.c +0
-0@@ -2591,9 +2591,11 @@ static int btrfs_rmdir(struct inode *dir, struct
dentry *dentry)
Quoted 22 lines
fs/btrfs/inode.c +5
-3struct btrfs_trans_handle *trans;unsigned long nr =0 ; -if (inode->i_size >BTRFS_EMPTY_DIR_SIZE || - inode->i_ino ==BTRFS_FIRST_FREE_OBJECTID ) -return -ENOTEMPTY ; +if (inode->i_size >BTRFS_EMPTY_DIR_SIZE ) +return -ENOTEMPTY ; + +if (inode->i_ino ==BTRFS_FIRST_FREE_OBJECTID ) +return -EPERM ; ret =btrfs_reserve_metadata_space (root,5 );if (ret)Signature · 6 lines
1.6.6.1 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 2 lines
gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo) <kreijackATinwind.it> Key fingerprint = 4769 7E51 5293 D36C 814E C054 BF04 F161 3DC5 0512
Quoted 5 lines · Goffredo Baroncelli
Can I suggest to return
-EINVALinstead of-EPERM?
To me EPERM seems that the user don't have the right to perform an action. But the problem is that "rm" is not the right command to use in order to delete a subvolume.
Quoted 7 lines
As side note, what is the reason for which an user is able to create a subvolume, but not to destroy it ?
BR
Goffredo
We can decide on that, but let me explaing in more detail.
From what i understood is that since when you are not allowed to delete the volume or a snapshot by "rmdir" (conventional means). Then it is perfectly safe to call it EPERM as it would be as if suggesting "pathname" doesn't support removal of directories.
EINVAL is a for other needs which i feel is not suitable in the current case.
There is another scenario of what if btrfs_rmdir itself can call subvol removal ioctl for snapshots and subvolumes in case someone issues a rmdir on such directories.
Suggestions please.
Regards
Signature · 2 lines
Harshavardhana http://www.gluster.com
Ok,
I read the unlink (2) man page which says:
[...]
EBUSY (not on Linux)
The file pathname cannot be unlinked because it is being used by the system or another process and the implementation considers this an error.
[...]
EPERM The system does not allow unlinking of directories, or unlinking
of directories requires privileges that the calling process
doesn't have. (This is the POSIX prescribed error return; as
noted above, Linux returns EISDIR for this case.)
EPERM (Linux only)
The file system does not allow unlinking of files.
[...]
In fact when I tried to unlink a directory where a filesystem is mounted, I got -EBUSY. So for consistency EBUSY may be another error which may be returned.
Quoted 3 lines · Harshavardhana
Quoted 2 lines · Goffredo Baroncelli
Can I suggest to return
-EINVALinstead of-EPERM?
To me EPERM seems that the user don't have the right to perform an action.
But
Quoted 1 line
Quoted 1 line
the problem is that "rm" is not the right command to use in order to
delete a
Quoted 36 lines
Quoted 9 lines
subvolume.
As side note, what is the reason for which an user is able to create a subvolume, but not to destroy it ?
BR
GoffredoWe can decide on that, but let me explaing in more detail.
From what i understood is that since when you are not allowed to delete
the volume or a snapshot by "rmdir" (conventional means). Then it is perfectly safe to call it EPERM as it would be as if suggesting "pathname" doesn't support removal of directories.EINVAL is a for other needs which i feel is not suitable in the current case.
There is another scenario of what if
btrfs_rmdiritself can call subvol removal ioctl for snapshots and subvolumes in case someone issues a rmdir on such directories.Suggestions please.
Regards
Signature · 7 lines
Harshavardhana http://www.gluster.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 2 lines
gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo) <[email protected]> Key fingerprint = 4769 7E51 5293 D36C 814E C054 BF04 F161 3DC5 0512
Quoted 11 lines · Goffredo Baroncelli
EBUSY (not on Linux)
The file pathname cannot be unlinked because it is being used by the system or another process and the implementation considers this an error.[...]
EPERM The system does not allow unlinking of directories, or unlinking
of directories requires privileges that the calling process
doesn't have. (This is the POSIX prescribed error return; as
noted above, Linux returns EISDIR for this case.)
Quoted 9 lines
EPERM (Linux only)
The file system does not allow unlinking of files.[...]
In fact when I tried to unlink a directory where a filesystem is mounted, I got
-EBUSY. So for consistency EBUSY may be another error which may be returned.
EBUSY is again meant for different reason where in a super block is being locked or accessed by an Application which would mean unref on that block would cause Application to go nuts. In such cases EBUSY is returned.
Signature · 2 lines
Harshavardhana http://www.gluster.com
Quoted 1 line · Harshavardhana
EBUSY is again meant for different reason where in a super block is being locked or accessed by an Application which would mean unref on that block would cause Application to go nuts. In such cases EBUSY is returned.
Ok i think ENOTSUPP could be another alternative? . This should be ok i guess?
Regards
Signature · 2 lines
Harshavardhana http://www.gluster.com
Quoted 7 lines · Harshavardhana
Quoted 4 lines · Harshavardhana
EBUSY is again meant for different reason where in a super block is being locked or accessed by an Application which would mean unref on that block would cause Application to go nuts. In such cases EBUSY is returned.
Ok i think ENOTSUPP could be another alternative? . This should be ok i guess?
I prefer ENOTSUP and EINVAL to EPERM.
But the man page of unlink (2) doesn't cite nor EINVAL nor ENOTSUP.
So at this point I have to admit that EPERM could be the best compromise.
But let me highlight that unlink(2) returns -EBUSY when an user try to delete a mount-point. This is quite similar to unlink a subvolume.
So my list in order of preference is
1) EBUSY -> because it is used when unlink is called on a
mount-point and it is in the unlink (2) man page
2) EPERM -> because it is in the unlink man page, and it was
already discussed
3) ENOTSUP, EINVAL -> to me these make a lot of sense. But the man
page doesn't cite both of them
BR
Goffredo
Quoted 13 lines
Regards
Signature · 7 lines
Harshavardhana http://www.gluster.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 2 lines
gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo) <kreijackATinwind.it> Key fingerprint = 4769 7E51 5293 D36C 814E C054 BF04 F161 3DC5 0512