On 2018年05月07日 10:03, Lu Fengqi wrote: > On Wed, Apr 18, 2018 at 01:12:10PM +0800, Qu Wenruo wrote: >> >> >> On 2018年03月27日 15:06, Lu Fengqi wrote: >>> The function is used to determine whether the subvolume is intact. >>> >>> Signed-off-by: Lu Fengqi <lufq.f...@cn.fujitsu.com> >>> --- >>> Makefile | 3 ++- >>> undelete-subvol.c | 53 >>> +++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> undelete-subvol.h | 17 +++++++++++++++++ >>> 3 files changed, 72 insertions(+), 1 deletion(-) >>> create mode 100644 undelete-subvol.c >>> create mode 100644 undelete-subvol.h >>> >>> diff --git a/Makefile b/Makefile >>> index 6065522a615f..cb38984c7386 100644 >>> --- a/Makefile >>> +++ b/Makefile >>> @@ -116,7 +116,8 @@ objects =tree.o disk-io.o kernel-lib/radix-tree.o >>> extent-tree.o print-tree.o \ >>> qgroup.o free-space-cache.o kernel-lib/list_sort.o props.o \ >>> kernel-shared/ulist.o qgroup-verify.o backref.o string-table.o >>> task-utils.o \ >>> inode.o file.o find-root.o free-space-tree.o help.o send-dump.o \ >>> - fsfeatures.o kernel-lib/tables.o kernel-lib/raid56.o >>> transaction.o >>> + fsfeatures.o kernel-lib/tables.o kernel-lib/raid56.o >>> transaction.o \ >>> + undelete-subvol.o >>> cmds_objects =mds-subvolume.o cmds-filesystem.o cmds-device.o >>> cmds-scrub.o \ >>> cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \ >>> cmds-quota.o cmds-qgroup.o cmds-replace.o check/main.o \ >>> diff --git a/undelete-subvol.c b/undelete-subvol.c >>> new file mode 100644 >>> index 000000000000..00fcc4895778 >>> --- /dev/null >>> +++ b/undelete-subvol.c >>> @@ -0,0 +1,53 @@ >>> +/* >>> + * Copyright (C) 2018 Fujitsu. All rights reserved. >> >> IIRC David will remove all such copy right line. >> Is there some principle about this, David? > > Are you referring to this patchset that replace GPL boilerplate by SPDX? > https://patchwork.kernel.org/patch/10321621/ > > However, I haven't seen a similar patch in btrfs-progs. >
Nope, I mean David will remove the Copyright (C) line when applying.
Although I'm not completely sure.
Thanks,
Qu
>>
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU General Public
>>> + * License v2 as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>> + * General Public License for more details.
>>> + */
>>> +
>>> +#include "ctree.h"
>>> +
>>> +/*
>>> + * Determines whether the subvolume is intact, according to the
>>> drop_progress
>>> + * of the root item specified by subvol_id.
>>> + *
>>> + * Return true if the subvolume is intact, otherwise return false.
>>> + */
>>> +static bool is_subvol_intact(struct btrfs_root *root, u64 subvol_id)
>>
>> Here we have 2 parameters which can represent a root, so please explain
>> the meaning of each parameter.
>>
>> And after looking into the code, @root should be tree root, and in that
>> case, I prefer passing @fs_info here to get rid of any confusion.
>
> Make sense.
>
>>
>>> +{
>>> + struct btrfs_path path;
>>> + struct btrfs_key key;
>>> + struct extent_buffer *leaf;
>>> + struct btrfs_root_item root_item;
>>> + u64 offset;
>>> + int ret;
>>> +
>>> + key.objectid =ubvol_id;
>>> + key.type =TRFS_ROOT_ITEM_KEY;
>>> + key.offset =;
>>> +
>>> + btrfs_init_path(&path);
>>> + ret =trfs_search_slot(NULL, root, &key, &path, 0, 0);
>>
>> Instead of setting up keys and search manually, what about using
>> btrfs_read_fs_root()?
>
> Indeed looks better.
>
>>
>>> + if (ret) {
>>> + ret =alse;
>>> + goto out;
>>> + }
>>> +
>>> + leaf =ath.nodes[0];
>>> + offset =trfs_item_ptr_offset(leaf, path.slots[0]);
>>> +
>>> + read_extent_buffer(leaf, &root_item, offset, sizeof(root_item));
>>> +
>>> + /* the subvolume is intact if the objectid of drop_progress =0 */
>>> + ret =trfs_disk_key_objectid(&root_item.drop_progress) ? false :
>>> true;
>>> +
>>> +out:
>>> + btrfs_release_path(&path);
>>> + return ret;
>>> +}
>>> diff --git a/undelete-subvol.h b/undelete-subvol.h
>>> new file mode 100644
>>> index 000000000000..7cfd100cce37
>>> --- /dev/null
>>> +++ b/undelete-subvol.h
>>> @@ -0,0 +1,17 @@
>>> +/*
>>> + * Copyright (C) 2018 Fujitsu. All rights reserved.
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU General Public
>>> + * License v2 as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>> + * General Public License for more details.
>>> + */
>>> +
>>> +#ifndef __BTRFS_UNDELETE_SUBVOLUME_H__
>> I think the empty header is not really needed here.
>> What about introducing it when we have something here?
>
> That's OK.
>
> Thanks for your review.
>
> --
> Thanks,
> Lu
>
>>
>> Thanks,
>> Qu
>>
>>> +#define __BTRFS_UNDELETE_SUBVOLUME_H__
>>> +
>>> +#endif
>>>
>>
>
>
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
signature.asc
Description: OpenPGP digital signature
