Re: [PATCH] blockjob: avoid compiler uncertainty in info sizing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]