[Ipmitool-devel] Code Review - ID: 289 - bmx-snmp-proxy: PEF alerting does not work for multiple destinations

[Zdenek Styblik]

On Mon, Dec 9, 2013 at 10:08 AM, Zdenek Styblik

Hello Charles,
I'm a bit confused about this patch and patch#84 I've reviewed just a
minute ago. Looking at it, it seems to me like iteration of previous.
Therefore, why are there two tickets and two patches? Why isn't former
closed as obsolete, if nothing else? Or should patches be applied
consecutively?
~~~
+ rm -f ${SNMPD_BMC_CONF}
[ $? -ne 0 ] && RETVAL=5
~~~
-> ``rm -f ${SNMPD_BMC_CONF} || RETVAL = 5''
~~~
+ temp_ping=$(ping -c 1 -I ${dev} ${BMC_IPv4})
+ [ $? -ne 0 ] && continue
+ echo $temp_ping| awk '{print $5}' && break
~~~
What exactly is purpose of this? What is the expected output here?
~~~
+ echo $temp_ping| awk '{print $5}' && break
~~~
-> ``printf -- "%s\n" "${temp_ping}" | ...''
Best regards,
Z.

I forgot to add ipmitool-devel@ on CC.

Z.

--
Zdenek Styblik

[C***@Dell.com]

Post by Zdenek Styblik
On Mon, Dec 9, 2013 at 10:08 AM, Zdenek Styblik

Hello Charles,
I'm a bit confused about this patch and patch#84 I've reviewed just a
minute ago. Looking at it, it seems to me like iteration of previous.
Therefore, why are there two tickets and two patches? Why isn't former
closed as obsolete, if nothing else? Or should patches be applied
consecutively?

Z,
Thanks for your review. And sorry for the confusion. #289 is newer to #84. Request you to close #84
(I do not seem to have any option to close it).

Post by Zdenek Styblik

~~~
+ rm -f ${SNMPD_BMC_CONF}
[ $? -ne 0 ] && RETVAL=5
~~~
-> ``rm -f ${SNMPD_BMC_CONF} || RETVAL = 5''

Agree. Will include this.

Post by Zdenek Styblik

~~~
+ temp_ping=$(ping -c 1 -I ${dev} ${BMC_IPv4})
+ [ $? -ne 0 ] && continue
+ echo $temp_ping| awk '{print $5}' && break
~~~
What exactly is purpose of this? What is the expected output here?

We use it for TRAPD_IP=$(get_host_ip)

Post by Zdenek Styblik

~~~
+ echo $temp_ping| awk '{print $5}' && break
~~~
-> ``printf -- "%s\n" "${temp_ping}" | ...''

Agree. Will include this change.

Post by Zdenek Styblik

Best regards,
Z.

Z.

--
Zdenek Styblik

--
Charles Rose
Linux Engineering
Dell Inc.