On Fri, Apr 17, 2015 at 7:54 AM, Noah Massey <noah.mas...@gmail.com> wrote:
> On Thu, Apr 16, 2015 at 7:33 PM, Dan Merillat <dan.meril...@gmail.com> wrote:
>> The inode is already found, use the data and make restore friendlier.
>>
>> Signed-off-by: Dan Merillat <dan.meril...@gmail.com>
>> ---
>>  cmds-restore.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/cmds-restore.c b/cmds-restore.c
>> index d2fc951..95ac487 100644
>> --- a/cmds-restore.c
>> +++ b/cmds-restore.c
>> @@ -567,12 +567,22 @@ static int copy_file(struct btrfs_root *root, int
>> fd, struct btrfs_key *key,
>>                 fprintf(stderr, "Ran out of memory\n");
>>                 return -ENOMEM;
>>         }
>> +       struct timespec times[2];
>> +       int times_ok=0;
>>
>>         ret = btrfs_lookup_inode(NULL, root, path, key, 0);
>>         if (ret == 0) {
>>                 inode_item = btrfs_item_ptr(path->nodes[0], path->slots[0],
>>                                     struct btrfs_inode_item);
>>                 found_size = btrfs_inode_size(path->nodes[0], inode_item);
>> +               struct btrfs_timespec bts;
>> +               read_eb_member(path->nodes[0], inode_item, struct 
>> btrfs_inode_item, atime, &bts);
>> +               times[0].tv_sec=bts.sec;
>> +               times[0].tv_nsec=bts.nsec;
>> +               read_eb_member(path->nodes[0], inode_item, struct 
>> btrfs_inode_item, atime, &bts);
>
> I think you mean 'mtime' here

I absolutely do, whoops.  This is probably a good time to mention how
much I dislike the fake pointers being used everywhere, coupled with
the partially-implemented macro magic to get fields out of them.  Is
there a good reason why btrfs_item_ptr isn't just a type-pun, with the
understanding that you'll need to copy it to keep it?

>> +       if (times_ok)
>> +               futimens(fd, times);
>
> return value isn't checked here.

What could we do with the error if it occurred?  Restoring times is a
nice bonus if it works, but if it gets lost while the data was
restored successfully, that shouldn't be an error condition.  I can
add a comment to that effect to make it clearer why it's being ignored
though, or perhaps something like a warn_once if the filesystem being
restored to doesn't support changing the times.

On the subject of errors - is it possible for read_eb_member to fail
the way I'm using it?  It's defined, but never used anywhere else, so
I have nothing to compare it to.  My feeling is that if btrfs_item_ptr
works the data in the structure returned is going to be there, but I'm
not sure.
--
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

Reply via email to