LGTM

Thanks,

Guido


On Fri, Jan 18, 2013 at 3:30 PM, Michael Hanselmann <[email protected]> wrote:
> Add a unittest for a function formatting CPU pinning information for
> Xen's configuration.
> ---
>  lib/hypervisor/hv_xen.py                     |   69 
> +++++++++++++-------------
>  test/py/ganeti.hypervisor.hv_xen_unittest.py |   19 +++++++
>  2 files changed, 53 insertions(+), 35 deletions(-)
>
> diff --git a/lib/hypervisor/hv_xen.py b/lib/hypervisor/hv_xen.py
> index 1afc1d9..20c7230 100644
> --- a/lib/hypervisor/hv_xen.py
> +++ b/lib/hypervisor/hv_xen.py
> @@ -43,6 +43,38 @@ VIF_BRIDGE_SCRIPT = 
> utils.PathJoin(pathutils.XEN_CONFIG_DIR,
>  _DOM0_NAME = "Domain-0"
>
>
> +def _CreateConfigCpus(cpu_mask):
> +  """Create a CPU config string for Xen's config file.
> +
> +  """
> +  # Convert the string CPU mask to a list of list of int's
> +  cpu_list = utils.ParseMultiCpuMask(cpu_mask)
> +
> +  if len(cpu_list) == 1:
> +    all_cpu_mapping = cpu_list[0]
> +    if all_cpu_mapping == constants.CPU_PINNING_OFF:
> +      # If CPU pinning has 1 entry that's "all", then remove the
> +      # parameter from the config file
> +      return None
> +    else:
> +      # If CPU pinning has one non-all entry, mapping all vCPUS (the entire
> +      # VM) to one physical CPU, using format 'cpu = "C"'
> +      return "cpu = \"%s\"" % ",".join(map(str, all_cpu_mapping))
> +  else:
> +
> +    def _GetCPUMap(vcpu):
> +      if vcpu[0] == constants.CPU_PINNING_ALL_VAL:
> +        cpu_map = constants.CPU_PINNING_ALL_XEN
> +      else:
> +        cpu_map = ",".join(map(str, vcpu))
> +      return "\"%s\"" % cpu_map
> +
> +    # build the result string in format 'cpus = [ "c", "c", "c" ]',
> +    # where each c is a physical CPU number, a range, a list, or any
> +    # combination
> +    return "cpus = [ %s ]" % ", ".join(map(_GetCPUMap, cpu_list))
> +
> +
>  class XenHypervisor(hv_base.BaseHypervisor):
>    """Xen generic hypervisor interface
>
> @@ -119,39 +151,6 @@ class XenHypervisor(hv_base.BaseHypervisor):
>      """
>      utils.RemoveFile(XenHypervisor._ConfigFileName(instance_name))
>
> -  @classmethod
> -  def _CreateConfigCpus(cls, cpu_mask):
> -    """Create a CPU config string that's compatible with Xen's
> -    configuration file.
> -
> -    """
> -    # Convert the string CPU mask to a list of list of int's
> -    cpu_list = utils.ParseMultiCpuMask(cpu_mask)
> -
> -    if len(cpu_list) == 1:
> -      all_cpu_mapping = cpu_list[0]
> -      if all_cpu_mapping == constants.CPU_PINNING_OFF:
> -        # If CPU pinning has 1 entry that's "all", then remove the
> -        # parameter from the config file
> -        return None
> -      else:
> -        # If CPU pinning has one non-all entry, mapping all vCPUS (the entire
> -        # VM) to one physical CPU, using format 'cpu = "C"'
> -        return "cpu = \"%s\"" % ",".join(map(str, all_cpu_mapping))
> -    else:
> -
> -      def _GetCPUMap(vcpu):
> -        if vcpu[0] == constants.CPU_PINNING_ALL_VAL:
> -          cpu_map = constants.CPU_PINNING_ALL_XEN
> -        else:
> -          cpu_map = ",".join(map(str, vcpu))
> -        return "\"%s\"" % cpu_map
> -
> -      # build the result string in format 'cpus = [ "c", "c", "c" ]',
> -      # where each c is a physical CPU number, a range, a list, or any
> -      # combination
> -      return "cpus = [ %s ]" % ", ".join(map(_GetCPUMap, cpu_list))
> -
>    @staticmethod
>    def _RunXmList(xmlist_errors):
>      """Helper function for L{_GetXMList} to run "xm list".
> @@ -676,7 +675,7 @@ class XenPvmHypervisor(XenHypervisor):
>      config.write("memory = %d\n" % startup_memory)
>      config.write("maxmem = %d\n" % instance.beparams[constants.BE_MAXMEM])
>      config.write("vcpus = %d\n" % instance.beparams[constants.BE_VCPUS])
> -    cpu_pinning = cls._CreateConfigCpus(hvp[constants.HV_CPU_MASK])
> +    cpu_pinning = _CreateConfigCpus(hvp[constants.HV_CPU_MASK])
>      if cpu_pinning:
>        config.write("%s\n" % cpu_pinning)
>      cpu_cap = hvp[constants.HV_CPU_CAP]
> @@ -779,7 +778,7 @@ class XenHvmHypervisor(XenHypervisor):
>      config.write("memory = %d\n" % startup_memory)
>      config.write("maxmem = %d\n" % instance.beparams[constants.BE_MAXMEM])
>      config.write("vcpus = %d\n" % instance.beparams[constants.BE_VCPUS])
> -    cpu_pinning = cls._CreateConfigCpus(hvp[constants.HV_CPU_MASK])
> +    cpu_pinning = _CreateConfigCpus(hvp[constants.HV_CPU_MASK])
>      if cpu_pinning:
>        config.write("%s\n" % cpu_pinning)
>      cpu_cap = hvp[constants.HV_CPU_CAP]
> diff --git a/test/py/ganeti.hypervisor.hv_xen_unittest.py 
> b/test/py/ganeti.hypervisor.hv_xen_unittest.py
> index dc86839..824a562 100755
> --- a/test/py/ganeti.hypervisor.hv_xen_unittest.py
> +++ b/test/py/ganeti.hypervisor.hv_xen_unittest.py
> @@ -44,5 +44,24 @@ class TestConsole(unittest.TestCase):
>        self.assertEqual(cons.command[-1], instance.name)
>
>
> +class TestCreateConfigCpus(unittest.TestCase):
> +  def testEmpty(self):
> +    for cpu_mask in [None, ""]:
> +      self.assertEqual(hv_xen._CreateConfigCpus(cpu_mask),
> +                       "cpus = [  ]")
> +
> +  def testAll(self):
> +    self.assertEqual(hv_xen._CreateConfigCpus(constants.CPU_PINNING_ALL),
> +                     None)
> +
> +  def testOne(self):
> +    self.assertEqual(hv_xen._CreateConfigCpus("9"), "cpu = \"9\"")
> +
> +  def testMultiple(self):
> +    self.assertEqual(hv_xen._CreateConfigCpus("0-2,4,5-5:3:all"),
> +                     ("cpus = [ \"0,1,2,4,5\", \"3\", \"%s\" ]" %
> +                      constants.CPU_PINNING_ALL_XEN))
> +
> +
>  if __name__ == "__main__":
>    testutils.GanetiTestProgram()
> --
> 1.7.7.3
>



-- 
Guido Trotter
Ganeti engineering
Google Germany

Reply via email to