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

Re: [libvirt] [PATCH RESEND 1/2] NUMA: cleanup for numa related codes



On 2013年03月20日 11:35, Gao feng wrote:
> Intend to reduce the redundant code,use virNumaSetupMemoryPolicy
> to replace virLXCControllerSetupNUMAPolicy and
> qemuProcessInitNumaMemoryPolicy.
> 
> This patch also moves the numa related codes to the
> file virnuma.c and virnuma.h
> 
> Signed-off-by: Gao feng<gaofeng cn fujitsu com>
> ---
>   src/conf/domain_conf.c   |  31 ++++--------
>   src/conf/domain_conf.h   |  25 +---------
>   src/libvirt_private.syms |   9 ++--
>   src/lxc/lxc_controller.c | 116 +------------------------------------------
>   src/qemu/qemu_cgroup.c   |   4 +-
>   src/qemu/qemu_driver.c   |   6 +--
>   src/qemu/qemu_process.c  | 123 +--------------------------------------------
>   src/util/virnuma.c       | 126 +++++++++++++++++++++++++++++++++++++++++++++++
>   src/util/virnuma.h       |  30 +++++++++++
>   9 files changed, 182 insertions(+), 288 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a1cfc76..fa70329 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -690,11 +690,6 @@ VIR_ENUM_IMPL(virDomainTimerMode, VIR_DOMAIN_TIMER_MODE_LAST,
>                 "paravirt",
>                 "smpsafe");
> 
> -VIR_ENUM_IMPL(virDomainNumatuneMemMode, VIR_DOMAIN_NUMATUNE_MEM_LAST,
> -              "strict",
> -              "preferred",
> -              "interleave");
> -
>   VIR_ENUM_IMPL(virDomainStartupPolicy, VIR_DOMAIN_STARTUP_POLICY_LAST,
>                 "default",
>                 "mandatory",
> @@ -709,12 +704,6 @@ VIR_ENUM_IMPL(virDomainDiskTray, VIR_DOMAIN_DISK_TRAY_LAST,
>                 "closed",
>                 "open");
> 
> -VIR_ENUM_IMPL(virDomainNumatuneMemPlacementMode,
> -              VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_LAST,
> -              "default",
> -              "static",
> -              "auto");
> -
>   VIR_ENUM_IMPL(virDomainRNGModel,
>                 VIR_DOMAIN_RNG_MODEL_LAST,
>                 "virtio");
> @@ -9852,7 +9841,7 @@ virDomainDefParseXML(virCapsPtr caps,
>                       int placement_mode = 0;
>                       if (placement) {
>                           if ((placement_mode =
> -                             virDomainNumatuneMemPlacementModeTypeFromString(placement))<  0) {
> +                             virNumaTuneMemPlacementModeTypeFromString(placement))<  0) {
>                               virReportError(VIR_ERR_XML_ERROR,
>                                              _("Unsupported memory placement "
>                                                "mode '%s'"), placement);
> @@ -9862,18 +9851,18 @@ virDomainDefParseXML(virCapsPtr caps,
>                           VIR_FREE(placement);
>                       } else if (def->numatune.memory.nodemask) {
>                           /* Defaults to "static" if nodeset is specified. */
> -                        placement_mode = VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC;
> +                        placement_mode = VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_STATIC;
>                       } else {
>                           /* Defaults to "placement" of<vcpu>  if nodeset is
>                            * not specified.
>                            */
>                           if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC)
> -                            placement_mode = VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC;
> +                            placement_mode = VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_STATIC;
>                           else
> -                            placement_mode = VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO;
> +                            placement_mode = VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO;
>                       }
> 
> -                    if (placement_mode == VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC&&
> +                    if (placement_mode == VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_STATIC&&
>                           !def->numatune.memory.nodemask) {
>                           virReportError(VIR_ERR_XML_ERROR, "%s",
>                                          _("nodeset for NUMA memory tuning must be set "
> @@ -9882,13 +9871,13 @@ virDomainDefParseXML(virCapsPtr caps,
>                       }
> 
>                       /* Ignore 'nodeset' if 'placement' is 'auto' finally */
> -                    if (placement_mode == VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO)
> +                    if (placement_mode == VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO)
>                           virBitmapFree(def->numatune.memory.nodemask);
> 
>                       /* Copy 'placement' of<numatune>  to<vcpu>  if its 'placement'
>                        * is not specified and 'placement' of<numatune>  is specified.
>                        */
> -                    if (placement_mode == VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO&&
> +                    if (placement_mode == VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO&&
>                           !def->cpumask)
>                           def->placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO;
> 
> @@ -9907,7 +9896,7 @@ virDomainDefParseXML(virCapsPtr caps,
>            * and 'placement' of<vcpu>  is 'auto'.
>            */
>           if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) {
> -            def->numatune.memory.placement_mode = VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO;
> +            def->numatune.memory.placement_mode = VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO;
>               def->numatune.memory.mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
>           }
>       }
> @@ -14818,7 +14807,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>           virBufferAsprintf(buf, "<memory mode='%s' ", mode);
> 
>           if (def->numatune.memory.placement_mode ==
> -            VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC) {
> +            VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_STATIC) {
>               nodemask = virBitmapFormat(def->numatune.memory.nodemask);
>               if (nodemask == NULL) {
>                   virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -14829,7 +14818,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>               virBufferAsprintf(buf, "nodeset='%s'/>\n", nodemask);
>               VIR_FREE(nodemask);
>           } else if (def->numatune.memory.placement_mode) {
> -            placement = virDomainNumatuneMemPlacementModeTypeToString(def->numatune.memory.placement_mode);
> +            placement = virNumaTuneMemPlacementModeTypeToString(def->numatune.memory.placement_mode);
>               virBufferAsprintf(buf, "placement='%s'/>\n", placement);
>           }
>           virBufferAddLit(buf, "</numatune>\n");
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index bfc37a0..6d856a3 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -47,6 +47,7 @@
>   # include "device_conf.h"
>   # include "virbitmap.h"
>   # include "virstoragefile.h"
> +# include "virnuma.h"
> 
>   /* forward declarations of all device types, required by
>    * virDomainDeviceDef
> @@ -1605,14 +1606,6 @@ enum virDomainCpuPlacementMode {
>       VIR_DOMAIN_CPU_PLACEMENT_MODE_LAST
>   };
> 
> -enum virDomainNumatuneMemPlacementMode {
> -    VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_DEFAULT = 0,
> -    VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC,
> -    VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO,
> -
> -    VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_LAST
> -};
> -
>   typedef struct _virDomainTimerCatchupDef virDomainTimerCatchupDef;
>   typedef virDomainTimerCatchupDef *virDomainTimerCatchupDefPtr;
>   struct _virDomainTimerCatchupDef {
> @@ -1701,18 +1694,6 @@ virDomainVcpuPinDefPtr virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def,
>                                                     int nvcpupin,
>                                                     int vcpu);
> 
> -typedef struct _virDomainNumatuneDef virDomainNumatuneDef;
> -typedef virDomainNumatuneDef *virDomainNumatuneDefPtr;
> -struct _virDomainNumatuneDef {
> -    struct {
> -        virBitmapPtr nodemask;
> -        int mode;
> -        int placement_mode; /* enum virDomainNumatuneMemPlacementMode */
> -    } memory;
> -
> -    /* Future NUMA tuning related stuff should go here. */
> -};
> -
>   typedef struct _virBlkioDeviceWeight virBlkioDeviceWeight;
>   typedef virBlkioDeviceWeight *virBlkioDeviceWeightPtr;
>   struct _virBlkioDeviceWeight {
> @@ -1802,7 +1783,7 @@ struct _virDomainDef {
>           virDomainVcpuPinDefPtr emulatorpin;
>       } cputune;
> 
> -    virDomainNumatuneDef numatune;
> +    virNumaTuneDef numatune;
> 
>       /* These 3 are based on virDomainLifeCycleAction enum flags */
>       int onReboot;
> @@ -2397,8 +2378,6 @@ VIR_ENUM_DECL(virDomainGraphicsSpicePlaybackCompression)
>   VIR_ENUM_DECL(virDomainGraphicsSpiceStreamingMode)
>   VIR_ENUM_DECL(virDomainGraphicsSpiceClipboardCopypaste)
>   VIR_ENUM_DECL(virDomainGraphicsSpiceMouseMode)
> -VIR_ENUM_DECL(virDomainNumatuneMemMode)
> -VIR_ENUM_DECL(virDomainNumatuneMemPlacementMode)
>   VIR_ENUM_DECL(virDomainHyperv)
>   VIR_ENUM_DECL(virDomainRNGModel)
>   VIR_ENUM_DECL(virDomainRNGBackend)
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index dc01bfa..8890859 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -252,10 +252,6 @@ virDomainNetRemove;
>   virDomainNetTypeToString;
>   virDomainNostateReasonTypeFromString;
>   virDomainNostateReasonTypeToString;
> -virDomainNumatuneMemModeTypeFromString;
> -virDomainNumatuneMemModeTypeToString;
> -virDomainNumatuneMemPlacementModeTypeFromString;
> -virDomainNumatuneMemPlacementModeTypeToString;
>   virDomainObjAssignDef;
>   virDomainObjCopyPersistentDef;
>   virDomainObjGetPersistentDef;
> @@ -1557,7 +1553,12 @@ virNodeSuspendGetTargetMask;
> 
> 
>   # util/virnuma.h
> +virDomainNumatuneMemModeTypeFromString;
> +virDomainNumatuneMemModeTypeToString;
> +virNumaTuneMemPlacementModeTypeFromString;
> +virNumaTuneMemPlacementModeTypeToString;
>   virNumaGetAutoPlacementAdvice;

Not alphabetically sorted. Just for your reference, I have an
alias like below to build after every changing:

alias $name="make && make syntax-check && make check"

This helps me a lot as  I'm careless enough to make mistakes
just like what you do. :-)

> +virNumaSetupMemoryPolicy;
> 

Others look good, ACK.


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