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&lt;gaofeng(a)cn.fujitsu.com&gt; > --- > 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.