On Tue, Jul 24, 2012 at 7:55 AM, Arne Jansen <sensi...@gmx.net> wrote:
> On 23.07.2012 21:41, Alexander Block wrote:
>> On Mon, Jul 16, 2012 at 4:56 PM, Arne Jansen <sensi...@gmx.net> wrote:
>>> On 04.07.2012 15:38, Alexander Block wrote:
>>>> +
>>>> + ret = btrfs_update_root(trans, root->fs_info->tree_root,
>>>> + &root->root_key, &root->root_item);
>>>> + if (ret < 0) {
>>>> + goto out;
>>>
>>> are you leaking a trans handle here?
>>>
>> btrfs_update_root is aborting the transaction in case of failure. Do I
>> still need to call end_transaction?
>
> It's your handle, you should free it.
Ahh...I thought abort_transaction already frees the handle. Fixed.
>
> ...
>
>>>>
>>>> +struct btrfs_ioctl_received_subvol_args {
>>>> + char uuid[BTRFS_UUID_SIZE]; /* in */
>>>> + __u64 stransid; /* in */
>>>> + __u64 rtransid; /* out */
>>>> + struct timespec stime; /* in */
>>>> + struct timespec rtime; /* out */
>>>> + __u64 reserved[16];
>>>
>>> What is this reserved used for? I don't see a mechanism that could be
>>> used to signal that there are useful information here, other than
>>> using a different ioctl.
>>>
>> The reserved is a result of a suggestion made by David. I can remove
>> it again if you want...
>
> I don't argue against some reserved space, I only have problems to
> see how you can make use of them in the future when there's no way
> to signal that they contain valid information. I should be sufficient
> to define the reserved values to be 0 at the moment.
Misunderstood that. Now I see the problem :) I've added a flags field
before the reserved fields. It's unused for now but can later be used
to signal that new fields are used.
>
>>>> +};
>>>> +
>>>> #define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
>>>> struct btrfs_ioctl_vol_args)
>>>> #define BTRFS_IOC_DEFRAG _IOW(BTRFS_IOCTL_MAGIC, 2, \
>>>> @@ -359,6 +368,10 @@ struct btrfs_ioctl_get_dev_stats {
>>>> struct btrfs_ioctl_ino_path_args)
>>>> #define BTRFS_IOC_LOGICAL_INO _IOWR(BTRFS_IOCTL_MAGIC, 36, \
>>>> struct btrfs_ioctl_ino_path_args)
>>>> +
>>>> +#define BTRFS_IOC_SET_RECEIVED_SUBVOL _IOWR(BTRFS_IOCTL_MAGIC, 37, \
>>>> + struct btrfs_ioctl_received_subvol_args)
>>>> +
>>>> #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \
>>>> struct btrfs_ioctl_get_dev_stats)
>>>> #define BTRFS_IOC_GET_AND_RESET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 53, \
>>>> diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
>>>> index 24fb8ce..17d638e 100644
>>>> --- a/fs/btrfs/root-tree.c
>>>> +++ b/fs/btrfs/root-tree.c
>>>> @@ -16,6 +16,7 @@
>>>> * Boston, MA 021110-1307, USA.
>>>> */
>>>>
>>>> +#include <linux/uuid.h>
>>>> #include "ctree.h"
>>>> #include "transaction.h"
>>>> #include "disk-io.h"
>>>> @@ -25,6 +26,9 @@
>>>> * lookup the root with the highest offset for a given objectid. The key
>>>> we do
>>>> * find is copied into 'key'. If we find something return 0, otherwise
>>>> 1, < 0
>>>> * on error.
>>>> + * We also check if the root was once mounted with an older kernel. If we
>>>> detect
>>>> + * this, the new fields coming after 'level' get overwritten with zeros
>>>> so to
>>>> + * invalidate the fields.
>>>
>>> ... "This is detected by a mismatch of the 2 generation fields" ... or
>>> something
>>> like that.
>>>
>> The current version (found in git only) has this new function which is
>> called in find_last_root:
>> void btrfs_read_root_item(struct btrfs_root *root,
>> struct extent_buffer *eb, int slot,
>> struct btrfs_root_item *item)
>>
>> The comment above this function explains what happens.
>
> ok. Please regard most of my comments as an expression of my thoughts while
> reading it. So they mark places where it might be useful to add comments
> to make it easier for the next reader :)
--
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
