[libvirt] [PATCH] fix libvirt alignment on arm platforms

Michal Privoznik mprivozn at redhat.com
Fri Dec 13 07:54:03 UTC 2013


On 12.12.2013 20:27, Eric Blake wrote:
> On 12/12/2013 11:55 AM, Michal Privoznik wrote:
>>>      int detail;
>>> -};
>>> +} ATTRIBUTE_PACKED;
>>
>> (What an ancient e-mail :-) )
> 
> 1970 was how many years before libvirt was even started?
> 
>>
>> I've raised the problem here:
>>
>> https://www.redhat.com/archives/libvir-list/2013-December/msg00635.html
>>
>> And Eric replied suggesting a fix:
>>
>> https://www.redhat.com/archives/libvir-list/2013-December/msg00662.html
>>
>> But I must say I like your approach more. Eric?
> 
> I still think ATTRIBUTE_PACKED in a parent class is wrong; it forces the
> children to be packed, and may make it impossible to implement atomic
> operations on a child member that was supposed to be aligned.  I'd much
> rather fix the alignment in the parent class using portable constructs
> than a compiler-specific non-alignment directive.
> 

Oh right. The locking could be impossible, because if two struct members
are packed in a word, e.g.:

struct S {
  bool a;
  bool b;
  ...
};

then modifying S.a requires whole struct to be fetched from mem,
modifying S.a a and storing it back. However, even if our locking code
to protect say S.b was written well, S.b can still get changed without
lock. As S.a is being changed, another thread changes S.b, but storing
S.a means overwriting S.b without lock held.

I have not thought of that. So NACK then - we need Eric's approach.

Michal




More information about the libvir-list mailing list