Hello Bernard,

Thanks for this submission. I have just reviewed it and found a few
problems, which have to be addressed before we merge this code.

 review needs-fixing

> === modified file 'lava_dispatcher/client/base.py'
> --- lava_dispatcher/client/base.py	2013-05-15 23:15:31 +0000
> +++ lava_dispatcher/client/base.py	2013-05-16 07:03:30 +0000
> @@ -426,7 +426,7 @@
>          attempts = 0
>          in_linaro_image = False
>          while (attempts < boot_attempts) and (not in_linaro_image):
> -            logging.info("Booting the test image. Attempt: %d" % attempts + 1)
> +            logging.info("Booting the test image. Attempt: %d" % int(attempts + 1))
>              try:
>                  self._boot_linaro_image()
>              except (OperationFailed, pexpect.TIMEOUT) as e:

This is not needed, as attempts is already an integer.

> @@ -435,8 +435,8 @@
>                  attempts += 1
>                  continue
>  
> -                timeout = self.config.boot_linaro_timeout
> -                TESTER_PS1_PATTERN = self.target_device.deployment_data['TESTER_PS1_PATTERN']
> +            TESTER_PS1_PATTERN = self.target_device.deployment_data['TESTER_PS1_PATTERN']
> +            timeout = self.config.boot_linaro_timeout
>              try:
>                  wait_for_prompt(self.proc, TESTER_PS1_PATTERN, timeout=timeout)
>              except (pexpect.TIMEOUT) as e:

This change does not apply anymore, please undo it.

> === modified file 'lava_dispatcher/config.py'
> --- lava_dispatcher/config.py	2013-05-16 01:53:21 +0000
> +++ lava_dispatcher/config.py	2013-05-16 07:03:30 +0000
> @@ -192,7 +192,7 @@
>          if os.path.exists(path):
>              config_files.append(path)
>      if not config_files:
> -        raise Exception("no config files named %r found" % (name + ".conf"))
> +        raise Exception("no config files named %r found on %s" % (name + ".conf", config_dir))
>      config_files.reverse()
>      logging.debug("About to read %s", str(config_files))
>      for path in config_files:

The dispatcher will look for config file in several directories, not
only in `config_dir`. So saying that the file was not found in
`config_dir` is not correct.

> === added file 'lava_dispatcher/device/gauss.py'

gauss.py is an exact copy of highbank.py

This is good because it means you could make it work without any change
there.

But it is bad from a maintaince point of view because we don't want to
maintain two copies of the same code.

So, I created a new merge proposal generalizing highbank.py to
ipmi_pxe.py, which basically only changes a couple of names:
https://code.launchpad.net/~terceiro/lava-dispatcher/generalize-ipmi-pxe/+merge/164781

This code will be merged in trunk in a few days, after one of the other
LAVA team members reviews it, and you should be able to use it then.

> === modified file 'lava_dispatcher/ipmi.py'
> --- lava_dispatcher/ipmi.py	2013-04-12 10:01:48 +0000
> +++ lava_dispatcher/ipmi.py	2013-05-16 07:03:30 +0000
> @@ -53,7 +53,10 @@
>          self.__ipmi("chassis power on")
>  
>      def reset(self):
> -        self.__ipmi("chassis power reset")
> +        if self.host.find('gauss') == -1:
> +                self.__ipmi("chassis power reset")
> +        else :
> +            self.__ipmi("chassis power cycle")

this block has a broken indentation.

More important, you cannot assume that the device host name is
associated with a given device type.

Is there a reason why `power cycle` does not work for you?

OTOH reading the ipmitool(1) manpage¹, maybe we actually want `reset` in
there for all cases?

¹ "cycle

    Provides a power off interval of at least 1 second.  No action
    should occur if chassis power is in S4/S5 state, but  it is
    recommended to check power state first and only issue a power cycle
    command if the system  power is on or in lower sleep state than
    S4/S5."

-- 
Antonio Terceiro
Software Engineer - Linaro
http://www.linaro.org