On 09/06/2017 at 02:01 PM, Dave Young wrote:
On 09/06/17 at 01:44pm, Xunlei Pang wrote:
> On 09/06/2017 at 12:52 PM, Dave Young wrote:
>> On 09/06/17 at 12:41pm, Dave Young wrote:
>>> On 09/06/17 at 09:01am, Xunlei Pang wrote:
>>>> On 09/05/2017 at 04:55 PM, Dave Young wrote:
>>>>> Hi Xunlei,
>>>>> On 08/30/17 at 04:45pm, Xunlei Pang wrote:
>>>>>> After adding "--hostonly-cmdline", besides 99kdump,
95iscsi
>>>>>> also generates iscsi related cmdline. IOW, we have duplicate
>>>>>> software iscsi cmdlines.
>>>>>>
>>>>>> Considering we need to setup special
"kdump-eth*"(moreover,
>>>>>> 95iscsi generated software iscsi cmdline doesn't work), we
>>>>>> remove that of 95iscsi and use that of 99kdump.
>>>>> kdump- prefix is another issue, it is probably one obstable for us
to
>>>>> remove kdump specific soft iscsi code. But it is not the reason why
we
>>>>> need remove duplicate iscsi cmdline genereated by dracut. How about
>>>>> remove the above paragraph for less confusion?
>>>> ok
>>>>
>>>>>> Signed-off-by: Xunlei Pang <xlpang(a)redhat.com>
>>>>>> ---
>>>>>> dracut-module-setup.sh | 5 +++++
>>>>>> 1 file changed, 5 insertions(+)
>>>>>>
>>>>>> diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
>>>>>> index ae13337..3eeda66 100755
>>>>>> --- a/dracut-module-setup.sh
>>>>>> +++ b/dracut-module-setup.sh
>>>>>> @@ -612,6 +612,11 @@ kdump_check_iscsi_targets () {
>>>>>> # If our prerequisites are not met, fail anyways.
>>>>>> type -P iscsistart >/dev/null || return 1
>>>>>>
>>>>>> + # Ugly: Remove software iscsi cmdline generated by dracut,
>>>>>> + # and regenerate here mainly due to kdump_setup_ifname().
>>>>>> + grep "ip=ibft"
${initdir}/etc/cmdline.d/95iscsi.conf &>/dev/null
>>>>>> + [ $? -ne 0 ] && rm -rf
${initdir}/etc/cmdline.d/95iscsi.conf
>>>>>> +
>>>>> Checked the dracut code, in dracut install_softiscsi will do nothin
in
>>>>> case /sys/firmware/ibft, so how about do it like below?
>>>>>
>>>>> [ ! -d /sys/firmware/ibft ] && rm -rf
${initdir}/etc/cmdline.d/95iscsi.conf
>>>> Oh, yes, the original code did have some issue, after some iscsi
knowledge recall, let's focus on firmware iscsi:
>>>> a) for cxgb4i(iBFT) the cmdline is: "rd.iscsi.firmware=1
ip=ibft"
>>>> b) for be2iscsi(firmware behave like iBFT, but not iBFT) the cmdline is:
"rd.iscsi.firmware=1", for such case
>>>> there will not be "/sys/firmware/ibft".
>>>> I.E. for all firmware cases, "rd.iscsi.firmware" always
exists.
>>>>
>>>> So I think we should change to use the following to do the correct
thing:
>>>> grep "rd.iscsi.firmware" ${initdir}/etc/cmdline.d/95iscsi.conf
&>/dev/null
>>> Hmm, if so it sounds better to check the soft iscsi strings instead of
>>> depends on the complex ibft conditions, we can check rd.iscsi.initiator
>>> because for software iscsi setup we must add it.
>>>
>>> How about use below condition?
>>>
>>> grep "rd.iscsi.initiator" ${initdir}/etc/cmdline.d/95iscsi.conf
&>/dev/null
>> Since dracut possibly add soft iscsi and rd.iscsi.firmware at same time, maybe
add
> It won't, see 95iscsi, it's if else condition:
> cmdline() {
> local _iscsiconf=$(install_ibft)
> {
> if [ "$_iscsiconf" ] ; then
> echo ${_iscsiconf}
> else
> install_softiscsi
> fi
> } | sort | uniq
> }
>
>
>> above condition should in below place
>> kdump_setup_iscsi_device() {
>> if is_ibft ${path}; then
>> return
>> fi
>>
>> grep "rd.iscsi.initiator" ${initdir}/etc/cmdline.d/95iscsi.conf
&>/dev/null
>> [ $? -ne 0 ] && rm -f ${initdir}/etc/cmdline.d/95iscsi.conf
> Agree, it's a better place, actually I think we can simply add one line below:
>
> rm -f ${initdir}/etc/cmdline.d/95iscsi.conf
>
> because for firmware(is_ibft) cases, it will return.
Ok, good catch, let's go this way before we dropping our own softiscsi
code..
Just update this patch should be enough.
Ok, sent v3, thanks for your review!
Regards,
Xunlei