Code review comment for lp:~nick-schutt/lava-dispatcher/nicks-highbank-support

On Thu, Apr 11, 2013 at 01:13:31PM -0000, Nicholas Schutt wrote:
>
> > > === modified file 'lava_dispatcher/client/base.py'
> > > --- lava_dispatcher/client/base.py 2013-03-27 11:22:07 +0000
> > > +++ lava_dispatcher/client/base.py 2013-04-08 13:59:23 +0000
> > > @@ -154,7 +154,7 @@
> > > lava_server_ip = self._client.context.config.lava_server_ip
> > > self.run(
> > > "LC_ALL=C ping -W4 -c1 %s" % lava_server_ip,
> > > - ["1 received", "0 received", "Network is unreachable"],
> > > + ["1 received|1 packets* received", "0 received|0 packets
> > received", "Network is unreachable"],
> > > timeout=5, failok=True)
> > > if self.match_id == 0:
> > > return True
> > >
> >
> > Do you really need this? Did ping had a different output on the rootfs
> > you tested with?
>
> >
>
> The output from ping in the initrd seems to match what I see on my ubuntu machine,
> which contains the word "packets":
>
>
> nick@neptune:/data/linaro/calxeda/code/lava-dispatcher/nicks-highbank-support/lava_dispatcher$ ping -W4 -c1 validation.linaro.org
> PING validation.linaro.org (88.98.47.97) 56(84) bytes of data.
> 64 bytes from validation.linaro.org (88.98.47.97): icmp_req=1 ttl=52 time=33.8 ms
>
> --- validation.linaro.org ping statistics ---
> 1 packets transmitted, 1 received, 0% packet loss, time 0ms
> rtt min/avg/max/mdev = 33.813/33.813/33.813/0.000 ms

yes, but note that "packets" are mentioned in the "transmitted" part,
but we really only care about the "received" part, which in this case
will say exactly either "1 received" or "0 received".

my point is to keep the diff minimal, without changes that are unrelated
with the purpose of this MP.

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