Re: [PATCH] xfsdump: add locks around the inventory put

Rich Johnston
2013-10-01T22:19:10Z

On 10/01/2013 04:06 PM, Eric Sandeen wrote:
> hi Rich -
>
> On 10/1/13 11:30 AM, Rich Johnston wrote:
>> From: Phil White <cerise-xfs@l.armory.com>
>>
>> Add locks around the inventory put to prevent inventory
>> corruption.
>>
>> Signed-off-by: Phil White <cerise-xfs@l.armory.com>
>
> Similar questions here; it says it's thread-safe, but apparently
> not. But why not? what happens? Is there a testcase?

Phil worked on this for a long time. To my knowledge the corruption
occured with multistream dumps only. I did not change his code but we
have had reports from the field on corruption when using multistream
dumps which does not occur with this fix.

Not sure what else Phil did to reproduce this other than a multistream
dump/restore (20 streams), which fails.

Testcase?? Good point I need to submit a testcase for both patches.

>
> (you guys probably have longer history in ptools, maybe you
> can see what if anything changed since the original comment
> was added - or maybe when it was added, etc?)
>
I did not look into this further, I will see if there is a better
explaination in our bug writeup.

> Thanks,
> -Eric
>
>> diff --git a/dump/content.c b/dump/content.c
>> index ac19021..b8977bb 100644
>> --- a/dump/content.c
>> +++ b/dump/content.c
>> @@ -2550,8 +2550,11 @@ decision_more:
>> scwhdrp->cih_startpt.sp_offset );
>> }
>>
>> - /* already thread-safe, don't need to lock
>> + /* Supposedly already thread-safe, according to the
>> + * previous revisions, but corruption of inventory
>> + * objects can occur.
>> */
>> */
>> + lock();
>> ok = inv_put_mediafile( inv_stmt,
>> &mwhdrp->mh_mediaid,
>> mwhdrp->mh_medialabel,
>> @@ -2565,6 +2568,7 @@ decision_more:
>> &&
>> ! empty_mediafile,
>> BOOL_FALSE );
>> + unlock();
>> if ( ! ok ) {
>> mlog( MLOG_NORMAL, _(
>> "inventory media file put failed\n") );
>>
>> _______________________________________________
>> xfs mailing list
>> xfs@oss.sgi.com
>> http://oss.sgi.com/mailman/listinfo/xfs
>>