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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list