[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 19:14, Osier Yang wrote:
> 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. :-)
> 

Yes, it's my fault.. thanks for your advice :)

>> +virNumaSetupMemoryPolicy;
>>
> 
> Others look good, ACK.
> 

Thanks!


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