On 06/16/2014 01:54 AM, Peter Krempa wrote:
On 06/14/14 14:50, Eric Blake wrote:
We have a policy of avoiding enum types in structs in our public API, because it is possible for a client to choose compiler options that can change the in-memory ABI of that struct based on whether the enum value occupies an int or a minimal size. But we missed this for virDomainBlockJobInfo. We got lucky on little-endian machines - if the enum fits minimal size (a char), we still end up padding to the next long before the next field; but on big-endian, a client interpreting the enum as a char would always see 0 when the server supplies contents as an int.
While at it, I noticed that the web page lacked documentation: http://libvirt.org/html/libvirt-libvirt.html#virDomainBlockJobType not only for the recently added active commit, but also for all the other job types.
"While at it" is often an indication that a patch does too much at once.
VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT = 4, + /* Active Block Commit (virDomainBlockCommit with flags), job + * exists as long as sync is active */
#ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_BLOCK_JOB_TYPE_LAST
ACK to the above hunk,
So I'll split this into 2, and apply the doc patches separately from...
@@ -2537,7 +2544,7 @@ typedef unsigned long long virDomainBlockJobCursor;
typedef struct _virDomainBlockJobInfo virDomainBlockJobInfo; struct _virDomainBlockJobInfo { - virDomainBlockJobType type; + int type; /* virDomainBlockJobType */ unsigned long bandwidth; /* * The following fields provide an indication of block job progress. @cur
I don't object to this one, but I'd like to see a second opinion.
...the ABI change. But as Dan has agreed that we are not breaking ABI on little-endian, and fixing a real bug on big-endian, I'll go ahead and push both patches shortly. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org