[PATCH] Add checking num-threads of makedumpfile

[Minfei Huang]

On 08/25/15 at 03:21pm, "Zhou, Wenjian/周文剑" wrote:
> On 08/25/2015 02:57 PM, Minfei Huang wrote:
> >On 08/25/15 at 02:17pm, "Zhou, Wenjian/周文剑" wrote:
> >>>>>>+	if [ $? -eq 0 ];then
> >>>>>>+		local nr_cpus=1
> >>>>>>+		local num_threads=0
> >>>>>>+		local core_collector=`grep -v "^#" $KDUMP_CONFIG_FILE | grep "^core_collector"`
> >>>>>>+
> >>>>>>+		num_threads=`echo ${core_collector#*--num-threads} | awk '{print $1}'`
> >>>>>>+		nr_cpus=`echo ${KDUMP_COMMANDLINE_APPEND#*nr_cpus=} | awk '{print $1}'`
> >>>>>>+
> >>>>>>+		test $num_threads -ge $nr_cpus &> /dev/null
> >>>>>>+		if [ $? -eq 0 ];then
> >>>>>
> >>>>>To keep the style, it is better to use "if [ $a -ge $b ]".
> >>>>>
> >>>>
> >>>>Using "test" is to handle the situation that the $num_threads or $nr_cpus is not an integer.
> >>>>
> >>>
> >>>Is it allowed for $num_threads or $nr_cpus, if the value is not the
> >>>integer?
> >>>
> >>
> >>Of course not integer is invalid. But the value of num_threads won't be checked
> >>until makedumpfile is executed.
> >>If user sets "core_collector makedumpfile --num-threads a" in kdump.conf, it will
> >>lead to a fault here.
> >>
> >
> >I think it is fine without validating the type of num-threads, since
> >makedumpfile will do this work.
> >
> >kdump does not check other usages of makedumpfile as well. It is better
> >to wrap a new function to check it in kdumpctl, if you want to try to
> >validate the usage value.
> >
> 
> I agree with you that we should not validate the type of num-threads.
> But using "test" is not to validate the type. It is just for avoiding the validating.
> If don't avoid it, the fault will occur here. And user may feel strange about the
> error message.

If we can detect the error as early as we can, the rate we dump the
vmcore successfully is higher.

Thanks
Minfei