On 01/22/2015 02:16 PM, Wang Shilong wrote:
> Hello,
>
>> When removing a subvol, if the related qgroup has no child qgroup, we should destroy
>> it at the same time. Also remove the TODO entry in qgroup.c.
> My two cents here:
>
> Take a quick look at this, i am not sure whether this is right way to do it.
>
> Actually, i think we can only remove a subvolume qgroup safely when both refer and excl are 0,
> because for subvolume removal case, to make qgroup accounting right, we need walk tree.
>
> At this time, qgroup walking might have not finished, if we remove this qgroup directly,
> accounting for this qgroup will be skipped.
Good point!
>
> Maybe we could remove such qgroup when at qgroup accounting, we find an qgroup’s refer and excl
> are 0, we could remove it, or we do it at the background..
>
> Also, there is another risk, that is to say, if qgroup accounting not work right, So
> we might need gurantee that subvolume it going to be deleted or have been deleted at the same time,
> then we could remove qgroup safely etc.
Yes, you are right. This is not a good timing to remove the qgroup directly.
I need some more investigation for a better solution.
As I have some emergency to deal with in this period, maybe the V2 will
come a little late.
Sorry about it.
Thanx
Yang
>
>
>> Signed-off-by: Dongsheng Yang <yangds.fnst@xxxxxxxxxxxxxx>
>> ---
>> fs/btrfs/inode.c | 4 ++++
>> fs/btrfs/qgroup.c | 1 -
>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
>> index e687bb0..31de211 100644
>> --- a/fs/btrfs/inode.c
>> +++ b/fs/btrfs/inode.c
>> @@ -59,6 +59,7 @@
>> #include "backref.h"
>> #include "hash.h"
>> #include "props.h"
>> +#include "qgroup.h"
>>
>> struct btrfs_iget_args {
>> struct btrfs_key *location;
>> @@ -4029,6 +4030,9 @@ int btrfs_unlink_subvol(struct btrfs_trans_handle *trans,
>> ret = btrfs_update_inode_fallback(trans, root, dir);
>> if (ret)
>> btrfs_abort_transaction(trans, root, ret);
>> + ret = btrfs_remove_qgroup(trans, root->fs_info, objectid);
>> + if (ret == -EBUSY)
>> + ret = 0;
>> out:
>> btrfs_free_path(path);
>> return ret;
>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
>> index c3b1e4f..303c078 100644
>> --- a/fs/btrfs/qgroup.c
>> +++ b/fs/btrfs/qgroup.c
>> @@ -35,7 +35,6 @@
>> #include "qgroup.h"
>>
>> /* TODO XXX FIXME
>> - * - subvol delete -> delete when ref goes to 0? delete limits also?
>> * - reorganize keys
>> * - compressed
>> * - sync
>> --
>> 1.8.4.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Best Regards,
> Wang Shilong
>
> .
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
