Re: chain_fsetxattr extra chunk removal

[Loic Dachary <loic@dachary.org>]

[-- Attachment #1: Type: text/plain, Size: 2277 bytes --]

Hi,

I amended the unit tests ( https://github.com/ceph/ceph/pull/40/files ) to cover the code below. A review would be much appreciated :-)

Cheers

On 02/11/2013 09:08 PM, Loic Dachary wrote:
> 
> 
> On 02/11/2013 06:13 AM, Yehuda Sadeh wrote:
>> On Thu, Feb 7, 2013 at 12:59 PM, Loic Dachary <loic@dachary.org> wrote:
>>> Hi,
>>>
>>> While writing unit tests for chain_xattr.cc I tried to understand how to create the conditions to trigger this part of the chain_fsetxattr function:
>>>
>>>   /* if we're exactly at a chunk size, remove the next one (if wasn't removed
>>>      before) */
>>>   if (ret >= 0 && chunk_size == CHAIN_XATTR_MAX_BLOCK_LEN) {
>>>     get_raw_xattr_name(name, i, raw_name, sizeof(raw_name));
>>>     int r = sys_fremovexattr(fd, raw_name);
>>>     if (r < 0 && r != -ENODATA)
>>>       ret = r;
>>>   }
>>>
>>> I suspect this cleans up extra empty attributes created as a side effect of a previous version of the function. Or I just don't understand the case it addresses.
>>>
>>> I'd very much appreciate a hint :-)
>>>
>>
>> Well, the code has changed a bit, but originally when a chain was
>> overwritten we didn't bother to remove the xattrs tail. When we read
>> the chain we stop either when we got a short xattr, or when the next
>> xattr in the chain didn't exist.  So when writing an xattr that was
>> perfectly aligned with the block len we had to remove the next xattr
>> in order make sure that readers will not over-read. I'm not too sure
>> whether that still the case, Sam might have a better idea.
>> In any case, it might be a good idea to test the case where we have a
>> big xattr that spans across multiple blocks (e.g., > 3) and being
>> overwritten by a short xattr. Probably also need to test it with
>> different combinations of aligned and non-aligned block sizes.
> 
> I understand now and I'll modify the pull request https://github.com/ceph/ceph/pull/40 accordingly.
> 
> Thanks :-)
> 
>>
>> Thanks,
>> Yehuda
>> --
>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Loïc Dachary, Artisan Logiciel Libre


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]