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

[Peter Krempa]

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.
> 
> * include/libvirt/libvirt.h.in (virDomainBlockJobInfo): Enforce
> particular sizing.
> (virDomainBlockJobType): Document recent addition.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  include/libvirt/libvirt.h.in | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 127de11..dc88c40 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -2501,19 +2501,26 @@ int virDomainUpdateDeviceFlags(virDomainPtr domain,
>  /**
>   * virDomainBlockJobType:
>   *
> - * VIR_DOMAIN_BLOCK_JOB_TYPE_PULL: Block Pull (virDomainBlockPull, or
> - * virDomainBlockRebase without flags), job ends on completion
> - * VIR_DOMAIN_BLOCK_JOB_TYPE_COPY: Block Copy (virDomainBlockRebase with
> - * flags), job exists as long as mirroring is active
> - * VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT: Block Commit (virDomainBlockCommit),
> - * job ends on completion
> + * Describes various possible block jobs.
>   */
>  typedef enum {
> -    VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN = 0,
> +    VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN = 0, /* Placeholder */
> +
>      VIR_DOMAIN_BLOCK_JOB_TYPE_PULL = 1,
> +    /* Block Pull (virDomainBlockPull, or virDomainBlockRebase without
> +     * flags), job ends on completion */
> +
>      VIR_DOMAIN_BLOCK_JOB_TYPE_COPY = 2,
> +    /* Block Copy (virDomainBlockRebase with flags), job exists as
> +     * long as mirroring is active */
> +
>      VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT = 3,
> +    /* Block Commit (virDomainBlockCommit without flags), job ends on
> +     * completion */
> +
>      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,

> @@ -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.

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140616/7992b358/attachment-0001.sig>