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

Re: [libvirt] [PATCH 2.5/7] maint: prefer newer API names internally



On Wed, May 18, 2011 at 09:51:44AM -0600, Eric Blake wrote:
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 7cd6e13..2174cfd 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -308,6 +308,10 @@ typedef enum {
>   * virTypedParameter:
>   *
>   * A named parameter, including a type and value.
> + *
> + * The types virSchedParameter, virBlkioParameter, and
> + * virMemoryParameter are aliases of this type, for use when
> + * targetting libvirt earlier than 0.9.2.
>   */
>  typedef struct _virTypedParameter virTypedParameter;
> 
> @@ -331,21 +335,8 @@ struct _virTypedParameter {
>   */
>  typedef virTypedParameter *virTypedParameterPtr;
> 
> -/* Management of scheduler parameters */
> 
> -/**
> - * virDomainSchedParameterType:
> - *
> - * A scheduler parameter field type
> - */
> -typedef enum {
> -    VIR_DOMAIN_SCHED_FIELD_INT     = VIR_TYPED_PARAM_INT,
> -    VIR_DOMAIN_SCHED_FIELD_UINT    = VIR_TYPED_PARAM_UINT,
> -    VIR_DOMAIN_SCHED_FIELD_LLONG   = VIR_TYPED_PARAM_LLONG,
> -    VIR_DOMAIN_SCHED_FIELD_ULLONG  = VIR_TYPED_PARAM_ULLONG,
> -    VIR_DOMAIN_SCHED_FIELD_DOUBLE  = VIR_TYPED_PARAM_DOUBLE,
> -    VIR_DOMAIN_SCHED_FIELD_BOOLEAN = VIR_TYPED_PARAM_BOOLEAN,
> -} virSchedParameterType;
> +/* Management of scheduler parameters */
> 
>  typedef enum {
>      VIR_DOMAIN_SCHEDPARAM_CURRENT = 0,        /* affect current domain state */
> @@ -353,49 +344,25 @@ typedef enum {
>      VIR_DOMAIN_SCHEDPARAM_CONFIG  = (1 << 1), /* Affect next boot */
>  } virDomainSchedParameterFlags;
> 
> -/**
> - * VIR_DOMAIN_SCHED_FIELD_LENGTH:
> - *
> - * Macro providing the field length of virSchedParameter
> - */
> -
> -#define VIR_DOMAIN_SCHED_FIELD_LENGTH VIR_TYPED_PARAM_FIELD_LENGTH
> -
> -/**
> - * virDomainSchedParameter:
> - *
> - * a virDomainSchedParameter is the set of scheduler parameters
> - */
> -
> -typedef struct _virTypedParameter virSchedParameter;
> -
> -/**
> - * virSchedParameterPtr:
> - *
> - * a virSchedParameterPtr is a pointer to a virSchedParameter structure.
> - */
> -
> -typedef virSchedParameter *virSchedParameterPtr;
> -
>  /*
>   * Fetch scheduler parameters, caller allocates 'params' field of size 'nparams'
>   */
>  int     virDomainGetSchedulerParameters (virDomainPtr domain,
> -                                         virSchedParameterPtr params,
> +                                         virTypedParameterPtr params,
>                                           int *nparams);
> 
>  /*
>   * Change scheduler parameters
>   */
>  int     virDomainSetSchedulerParameters (virDomainPtr domain,
> -                                         virSchedParameterPtr params,
> +                                         virTypedParameterPtr params,
>                                           int nparams);
> 
>  /*
>   * Change scheduler parameters
>   */
>  int     virDomainSetSchedulerParametersFlags (virDomainPtr domain,
> -                                              virSchedParameterPtr params,
> +                                              virTypedParameterPtr params,
>                                                int nparams,
>                                                unsigned int flags);
> 
> @@ -798,29 +765,8 @@ int                     virDomainGetState       (virDomainPtr domain,
>  char *                  virDomainGetSchedulerType(virDomainPtr domain,
>                                                   int *nparams);
> 
> -/* Manage blkio parameters.  */
> -
> -/**
> - * virDomainBlkioParameterType:
> - *
> - * A blkio parameter field type
> - */
> -typedef enum {
> -    VIR_DOMAIN_BLKIO_PARAM_INT     = VIR_TYPED_PARAM_INT,
> -    VIR_DOMAIN_BLKIO_PARAM_UINT    = VIR_TYPED_PARAM_UINT,
> -    VIR_DOMAIN_BLKIO_PARAM_LLONG   = VIR_TYPED_PARAM_LLONG,
> -    VIR_DOMAIN_BLKIO_PARAM_ULLONG  = VIR_TYPED_PARAM_ULLONG,
> -    VIR_DOMAIN_BLKIO_PARAM_DOUBLE  = VIR_TYPED_PARAM_DOUBLE,
> -    VIR_DOMAIN_BLKIO_PARAM_BOOLEAN = VIR_TYPED_PARAM_BOOLEAN,
> -} virBlkioParameterType;
> 
> -/**
> - * VIR_DOMAIN_BLKIO_FIELD_LENGTH:
> - *
> - * Macro providing the field length of virBlkioParameter
> - */
> -
> -#define VIR_DOMAIN_BLKIO_FIELD_LENGTH VIR_TYPED_PARAM_FIELD_LENGTH
> +/* Manage blkio parameters.  */
> 
>  /**
>   * VIR_DOMAIN_BLKIO_WEIGHT:
> @@ -831,55 +777,17 @@ typedef enum {
> 
>  #define VIR_DOMAIN_BLKIO_WEIGHT "weight"
> 
> -/**
> - * virDomainBlkioParameter:
> - *
> - * a virDomainBlkioParameter is the set of blkio parameters
> - */
> -
> -typedef struct _virTypedParameter virBlkioParameter;
> -
> -/**
> - * virBlkioParameterPtr:
> - *
> - * a virBlkioParameterPtr is a pointer to a virBlkioParameter structure.
> - */
> -
> -typedef virBlkioParameter *virBlkioParameterPtr;
> -
>  /* Set Blkio tunables for the domain*/
>  int     virDomainSetBlkioParameters(virDomainPtr domain,
> -                                     virBlkioParameterPtr params,
> +                                     virTypedParameterPtr params,
>                                       int nparams, unsigned int flags);
>  int     virDomainGetBlkioParameters(virDomainPtr domain,
> -                                     virBlkioParameterPtr params,
> +                                     virTypedParameterPtr params,
>                                       int *nparams, unsigned int flags);
> 
>  /* Manage memory parameters.  */
> 
>  /**
> - * virDomainMemoryParameterType:
> - *
> - * A memory parameter field type
> - */
> -typedef enum {
> -    VIR_DOMAIN_MEMORY_PARAM_INT     = VIR_TYPED_PARAM_INT,
> -    VIR_DOMAIN_MEMORY_PARAM_UINT    = VIR_TYPED_PARAM_UINT,
> -    VIR_DOMAIN_MEMORY_PARAM_LLONG   = VIR_TYPED_PARAM_LLONG,
> -    VIR_DOMAIN_MEMORY_PARAM_ULLONG  = VIR_TYPED_PARAM_ULLONG,
> -    VIR_DOMAIN_MEMORY_PARAM_DOUBLE  = VIR_TYPED_PARAM_DOUBLE,
> -    VIR_DOMAIN_MEMORY_PARAM_BOOLEAN = VIR_TYPED_PARAM_BOOLEAN,
> -} virMemoryParameterType;
> -
> -/**
> - * VIR_DOMAIN_MEMORY_FIELD_LENGTH:
> - *
> - * Macro providing the field length of virMemoryParameter
> - */
> -
> -#define VIR_DOMAIN_MEMORY_FIELD_LENGTH VIR_TYPED_PARAM_FIELD_LENGTH
> -
> -/**
>   * VIR_DOMAIN_MEMORY_PARAM_UNLIMITED:
>   *
>   * Macro providing the virMemoryParameter value that indicates "unlimited"
> @@ -924,28 +832,12 @@ typedef enum {
> 
>  #define VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT "swap_hard_limit"
> 
> -/**
> - * virDomainMemoryParameter:
> - *
> - * a virDomainMemoryParameter is the set of scheduler parameters
> - */
> -
> -typedef struct _virTypedParameter virMemoryParameter;
> -
> -/**
> - * virMemoryParameterPtr:
> - *
> - * a virMemoryParameterPtr is a pointer to a virMemoryParameter structure.
> - */
> -
> -typedef virMemoryParameter *virMemoryParameterPtr;
> -
>  /* Set memory tunables for the domain*/
>  int     virDomainSetMemoryParameters(virDomainPtr domain,
> -                                     virMemoryParameterPtr params,
> +                                     virTypedParameterPtr params,
>                                       int nparams, unsigned int flags);
>  int     virDomainGetMemoryParameters(virDomainPtr domain,
> -                                     virMemoryParameterPtr params,
> +                                     virTypedParameterPtr params,
>                                       int *nparams, unsigned int flags);
> 
>  /* Memory size modification flags. */
> @@ -2617,6 +2509,136 @@ int virDomainOpenConsole(virDomainPtr dom,
> 
>  int virDomainInjectNMI(virDomainPtr domain, unsigned int flags);
> 
> +
> +/**
> + * virDomainSchedParameterType:
> + *
> + * A scheduler parameter field type.  Provided for backwards
> + * compatibility; virTypedParameterType is the preferred enum since
> + * 0.9.2.
> + */
> +typedef enum {
> +    VIR_DOMAIN_SCHED_FIELD_INT     = VIR_TYPED_PARAM_INT,
> +    VIR_DOMAIN_SCHED_FIELD_UINT    = VIR_TYPED_PARAM_UINT,
> +    VIR_DOMAIN_SCHED_FIELD_LLONG   = VIR_TYPED_PARAM_LLONG,
> +    VIR_DOMAIN_SCHED_FIELD_ULLONG  = VIR_TYPED_PARAM_ULLONG,
> +    VIR_DOMAIN_SCHED_FIELD_DOUBLE  = VIR_TYPED_PARAM_DOUBLE,
> +    VIR_DOMAIN_SCHED_FIELD_BOOLEAN = VIR_TYPED_PARAM_BOOLEAN,
> +} virSchedParameterType;
> +
> +/**
> + * VIR_DOMAIN_SCHED_FIELD_LENGTH:
> + *
> + * Macro providing the field length of virSchedParameter.  Provided
> + * for backwards compatibility; VIR_TYPED_PARAM_FIELD_LENGTH is the
> + * preferred value since 0.9.2.
> + */
> +#define VIR_DOMAIN_SCHED_FIELD_LENGTH VIR_TYPED_PARAM_FIELD_LENGTH
> +
> +/**
> + * virDomainSchedParameter:
> + *
> + * a virDomainSchedParameter is the set of scheduler parameters.
> + * Provided for backwards compatibility; virTypedParameter is the
> + * preferred alias since 0.9.2.
> + */
> +typedef struct _virTypedParameter virSchedParameter;
> +
> +/**
> + * virSchedParameterPtr:
> + *
> + * a virSchedParameterPtr is a pointer to a virSchedParameter structure.
> + * Provided for backwards compatibility; virTypedParameterPtr is the
> + * preferred alias since 0.9.2.
> + */
> +typedef virSchedParameter *virSchedParameterPtr;
> +
> +/**
> + * virDomainBlkioParameterType:
> + *
> + * A blkio parameter field type.  Provided for backwards
> + * compatibility; virTypedParameterType is the preferred enum since
> + * 0.9.2.
> + */
> +typedef enum {
> +    VIR_DOMAIN_BLKIO_PARAM_INT     = VIR_TYPED_PARAM_INT,
> +    VIR_DOMAIN_BLKIO_PARAM_UINT    = VIR_TYPED_PARAM_UINT,
> +    VIR_DOMAIN_BLKIO_PARAM_LLONG   = VIR_TYPED_PARAM_LLONG,
> +    VIR_DOMAIN_BLKIO_PARAM_ULLONG  = VIR_TYPED_PARAM_ULLONG,
> +    VIR_DOMAIN_BLKIO_PARAM_DOUBLE  = VIR_TYPED_PARAM_DOUBLE,
> +    VIR_DOMAIN_BLKIO_PARAM_BOOLEAN = VIR_TYPED_PARAM_BOOLEAN,
> +} virBlkioParameterType;
> +
> +/**
> + * VIR_DOMAIN_BLKIO_FIELD_LENGTH:
> + *
> + * Macro providing the field length of virBlkioParameter.  Provided
> + * for backwards compatibility; VIR_TYPED_PARAM_FIELD_LENGTH is the
> + * preferred value since 0.9.2.
> + */
> +#define VIR_DOMAIN_BLKIO_FIELD_LENGTH VIR_TYPED_PARAM_FIELD_LENGTH
> +
> +/**
> + * virDomainBlkioParameter:
> + *
> + * a virDomainBlkioParameter is the set of blkio parameters.
> + * Provided for backwards compatibility; virTypedParameter is the
> + * preferred alias since 0.9.2.
> + */
> +typedef struct _virTypedParameter virBlkioParameter;
> +
> +/**
> + * virBlkioParameterPtr:
> + *
> + * a virBlkioParameterPtr is a pointer to a virBlkioParameter structure.
> + * Provided for backwards compatibility; virTypedParameterPtr is the
> + * preferred alias since 0.9.2.
> + */
> +typedef virBlkioParameter *virBlkioParameterPtr;
> +
> +/**
> + * virDomainMemoryParameterType:
> + *
> + * A memory parameter field type.  Provided for backwards
> + * compatibility; virTypedParameterType is the preferred enum since
> + * 0.9.2.
> + */
> +typedef enum {
> +    VIR_DOMAIN_MEMORY_PARAM_INT     = VIR_TYPED_PARAM_INT,
> +    VIR_DOMAIN_MEMORY_PARAM_UINT    = VIR_TYPED_PARAM_UINT,
> +    VIR_DOMAIN_MEMORY_PARAM_LLONG   = VIR_TYPED_PARAM_LLONG,
> +    VIR_DOMAIN_MEMORY_PARAM_ULLONG  = VIR_TYPED_PARAM_ULLONG,
> +    VIR_DOMAIN_MEMORY_PARAM_DOUBLE  = VIR_TYPED_PARAM_DOUBLE,
> +    VIR_DOMAIN_MEMORY_PARAM_BOOLEAN = VIR_TYPED_PARAM_BOOLEAN,
> +} virMemoryParameterType;
> +
> +/**
> + * VIR_DOMAIN_MEMORY_FIELD_LENGTH:
> + *
> + * Macro providing the field length of virMemoryParameter.  Provided
> + * for backwards compatibility; VIR_TYPED_PARAM_FIELD_LENGTH is the
> + * preferred value since 0.9.2.
> + */
> +#define VIR_DOMAIN_MEMORY_FIELD_LENGTH VIR_TYPED_PARAM_FIELD_LENGTH
> +
> +/**
> + * virDomainMemoryParameter:
> + *
> + * a virDomainMemoryParameter is the set of scheduler parameters.
> + * Provided for backwards compatibility; virTypedParameter is the
> + * preferred alias since 0.9.2.
> + */
> +typedef struct _virTypedParameter virMemoryParameter;
> +
> +/**
> + * virMemoryParameterPtr:
> + *
> + * a virMemoryParameterPtr is a pointer to a virMemoryParameter structure.
> + * Provided for backwards compatibility; virTypedParameterPtr is the
> + * preferred alias since 0.9.2.
> + */
> +typedef virMemoryParameter *virMemoryParameterPtr;
> +
>  #ifdef __cplusplus
>  }
>  #endif

While the structs are ABI compatible, this still constitutes a change
in API. ie apps doing this

  int np = 5;
  struct _virMemoryParameter p[np];

  virDomainGetSchedulerParameters(dom, p, &np);

will get a compile warning/error

  demo.c: In function ‘main’:
  demo.c:35:3: warning: passing argument 2 of ‘virDomainGetSchedulerParameters’ from incompatible pointer type
  demo.c:25:12: note: expected ‘virTypedParameterPtr’ but argument is of type ‘struct _virMemoryParameter *’

so NACK to this change.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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