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