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

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



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

Attachment: signature.asc
Description: OpenPGP digital signature


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