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

Re: [libvirt] [PATCH] Deprecate several CURRENT/LIVE/CONFIG enums



On Mon, Jun 13, 2011 at 02:06:49PM -0600, Eric Blake wrote:
> On 06/08/2011 12:33 AM, Hu Tao wrote:
> > This patch deprecates following enums:
> > 
> > VIR_DOMAIN_MEM_CURRENT
> > VIR_DOMAIN_MEM_LIVE
> > VIR_DOMAIN_MEM_CONFIG
> > 
> > VIR_DOMAIN_VCPU_LIVE
> > VIR_DOMAIN_VCPU_CONFIG
> > 
> > VIR_DOMAIN_DEVICE_MODIFY_CURRENT
> > VIR_DOMAIN_DEVICE_MODIFY_LIVE
> > VIR_DOMAIN_DEVICE_MODIFY_CONFIG
> > 
> > And modify internal codes to use virDomainModifycationImpact.
> 
> s/Modifycation/Modification/
> 
> > +++ b/include/libvirt/libvirt.h.in
> > @@ -863,15 +863,6 @@ int     virDomainGetMemoryParameters(virDomainPtr domain,
> >                                       virTypedParameterPtr params,
> >                                       int *nparams, unsigned int flags);
> >  
> > -/* Memory size modification flags. */
> > -typedef enum {
> > -    VIR_DOMAIN_MEM_CURRENT = 0,        /* affect current domain state */
> > -    VIR_DOMAIN_MEM_LIVE    = (1 << 0), /* affect active domain */
> > -    VIR_DOMAIN_MEM_CONFIG  = (1 << 1), /* affect next boot */
> > -    VIR_DOMAIN_MEM_MAXIMUM = (1 << 2), /* affect Max rather than current */
> > -} virDomainMemoryModFlags;
> 
> We can deprecate VIR_DOMAIN_MEM_{CURRENT,LIVE,CONFIG}, but
> VIR_DOMAIN_MEM_MAXIMUM is still a useful value.  Therefore, sinking this
> particular enum to the bottom of the file is not useful, because it
> hides a non-deprecated enum name from the user.
> 
> > -
> > -
> >  /*
> >   * Dynamic control of domains
> >   */
> > @@ -1027,16 +1018,6 @@ struct _virVcpuInfo {
> >  };
> >  typedef virVcpuInfo *virVcpuInfoPtr;
> >  
> > -/* Flags for controlling virtual CPU hot-plugging.  */
> > -typedef enum {
> > -    /* Must choose at least one of these two bits; SetVcpus can choose both */
> > -    VIR_DOMAIN_VCPU_LIVE    = (1 << 0), /* Affect active domain */
> > -    VIR_DOMAIN_VCPU_CONFIG  = (1 << 1), /* Affect next boot */
> > -
> > -    /* Additional flags to be bit-wise OR'd in */
> > -    VIR_DOMAIN_VCPU_MAXIMUM = (1 << 2), /* Max rather than current count */
> > -} virDomainVcpuFlags;
> 
> Same story - VIR_DOMAIN_VCPU_MAXIMUM is independently useful.
> 
> > -
> >  int                     virDomainSetVcpus       (virDomainPtr domain,
> >                                                   unsigned int nvcpus);
> >  int                     virDomainSetVcpusFlags  (virDomainPtr domain,
> > @@ -1136,15 +1117,6 @@ int                     virDomainGetVcpus       (virDomainPtr domain,
> >  #define VIR_GET_CPUMAP(cpumaps,maplen,vcpu)     &(cpumaps[(vcpu)*(maplen)])
> >  
> >  
> > -typedef enum {
> > -
> > -   VIR_DOMAIN_DEVICE_MODIFY_CURRENT = 0, /* Modify device allocation based on current domain state */
> > -   VIR_DOMAIN_DEVICE_MODIFY_LIVE = (1 << 0), /* Modify live device allocation */
> > -   VIR_DOMAIN_DEVICE_MODIFY_CONFIG = (1 << 1), /* Modify persisted device allocation */
> > -   VIR_DOMAIN_DEVICE_MODIFY_FORCE = (1 << 2), /* Forcibly modify device
> > -                                                 (ex. force eject a cdrom) */
> > -} virDomainDeviceModifyFlags;
> 
> As is VIR_DOMAIN_DEVICE_MODIFY_FORCE.
> 
> But the rest of the patch looks good.  I went ahead and pushed with this
> change to libvirt.h.in instead of your change:
> 
> diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in
> index a38ba81..76ad908 100644
> --- i/include/libvirt/libvirt.h.in
> +++ w/include/libvirt/libvirt.h.in
> @@ -865,9 +865,12 @@ int     virDomainGetMemoryParameters(virDomainPtr
> domain,
> 
>  /* Memory size modification flags. */
>  typedef enum {
> -    VIR_DOMAIN_MEM_CURRENT = 0,        /* affect current domain state */
> -    VIR_DOMAIN_MEM_LIVE    = (1 << 0), /* affect active domain */
> -    VIR_DOMAIN_MEM_CONFIG  = (1 << 1), /* affect next boot */
> +    /* See virDomainModificationImpact for these flags.  */
> +    VIR_DOMAIN_MEM_CURRENT = VIR_DOMAIN_AFFECT_CURRENT,
> +    VIR_DOMAIN_MEM_LIVE    = VIR_DOMAIN_AFFECT_LIVE,
> +    VIR_DOMAIN_MEM_CONFIG  = VIR_DOMAIN_AFFECT_CONFIG,
> +
> +    /* Additionally, these flags may be bitwise-OR'd in.  */
>      VIR_DOMAIN_MEM_MAXIMUM = (1 << 2), /* affect Max rather than current */
>  } virDomainMemoryModFlags;
> 
> @@ -1029,11 +1032,12 @@ typedef virVcpuInfo *virVcpuInfoPtr;
> 
>  /* Flags for controlling virtual CPU hot-plugging.  */
>  typedef enum {
> -    /* Must choose at least one of these two bits; SetVcpus can choose
> both */
> -    VIR_DOMAIN_VCPU_LIVE    = (1 << 0), /* Affect active domain */
> -    VIR_DOMAIN_VCPU_CONFIG  = (1 << 1), /* Affect next boot */
> +    /* Must choose at least one of these two bits; SetVcpus can choose
> both;
> +       see virDomainModificationImpact for details.  */
> +    VIR_DOMAIN_VCPU_LIVE    = VIR_DOMAIN_AFFECT_LIVE,
> +    VIR_DOMAIN_VCPU_CONFIG  = VIR_DOMAIN_AFFECT_CONFIG,
> 
> -    /* Additional flags to be bit-wise OR'd in */
> +    /* Additionally, these flags may be bitwise-OR'd in.  */
>      VIR_DOMAIN_VCPU_MAXIMUM = (1 << 2), /* Max rather than current count */
>  } virDomainVcpuFlags;
> 
> @@ -1142,12 +1146,14 @@ int                     virDomainGetVcpus
> (virDomainPtr domain,
> 
> 
>  typedef enum {
> -
> -   VIR_DOMAIN_DEVICE_MODIFY_CURRENT = 0, /* Modify device allocation
> based on current domain state */
> -   VIR_DOMAIN_DEVICE_MODIFY_LIVE = (1 << 0), /* Modify live device
> allocation */
> -   VIR_DOMAIN_DEVICE_MODIFY_CONFIG = (1 << 1), /* Modify persisted
> device allocation */
> -   VIR_DOMAIN_DEVICE_MODIFY_FORCE = (1 << 2), /* Forcibly modify device
> -                                                 (ex. force eject a
> cdrom) */
> +    /* See virDomainModificationImpact for these flags.  */
> +    VIR_DOMAIN_DEVICE_MODIFY_CURRENT = VIR_DOMAIN_AFFECT_CURRENT,
> +    VIR_DOMAIN_DEVICE_MODIFY_LIVE    = VIR_DOMAIN_AFFECT_LIVE,
> +    VIR_DOMAIN_DEVICE_MODIFY_CONFIG  = VIR_DOMAIN_AFFECT_CONFIG,
> +
> +    /* Additionally, these flags may be bitwise-OR'd in.  */
> +    VIR_DOMAIN_DEVICE_MODIFY_FORCE = (1 << 2), /* Forcibly modify device
> +                                                  (ex. force eject a
> cdrom) */
>  } virDomainDeviceModifyFlags;
> 
>  int virDomainAttachDevice(virDomainPtr domain, const char *xml);
> 
> 
> -- 
> Eric Blake   eblake redhat com    +1-801-349-2682
> Libvirt virtualization library http://libvirt.org
> 

Thanks.

-- 
Thanks,
Hu Tao


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