Re: [patch v2 2/3] Add for_each_host_dev_and_slaves for device only checking

[Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On 08/23/2012 11:26 PM, Vivek Goyal wrote:

> On Thu, Aug 23, 2012 at 11:02:23AM +0800, dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org wrote:
>> For lvm, multipath, iscsi modules they do not care about the filesystem,
>> Also there could be devcie in host_devs but it does not get formated.
>>
>> For these kind of modules, use for_each_host_dev_and_slaves will be better than use
>> for_each_host_dev_fs, here add a new function to iterate the host_devs and
>> their slave devices.
>>
>> In original for_each_host_dev_fs, it will call check_block_and_slaves which
>> will return once helper function return 0, but this is not enough for kdump
>> iscsi setup. For kdump iscsi case, it need setup each slave devices so that
>> the iscsi target can be properly setuped in initramfs.


Firstly, fix myself, check_block_and_slaves is not called in
for_each_host_dev_fs. It's called when dracut.sh collect and push
host_fs_types, so because there's no enough slave devices in
host_fs_types is_iscsi will only check the target in host_fs_types.

>>
>> Thus, this patch also add new functions check_block_and_slaves_all and
>> for_each_host_dev_and_slaves_all.
> 
> I think this patch should be broken in two parts for more clarity.


Will do

> 
> - Fix the kdump iscsi issue where we don't expect to break out of the
>   loop the moment first iscsi device is found.
> 
> - Start using host_dev instead of host_dev_fs for modules which don't
>   care about fs. Also why to have host_devs[] and host_fs_types[] both.
>   Looks like host_fs_types contains both device and fs information. I
>   think that includes strings like LVM etc.
> 
>   So can we merge both into one and those modules who don't require fs
>   info will ignore it. Or, keep one data structure host_dev_fs_types, 
>   and provide two helper functions. One which provdes on $dev in $1
>   and other which provides both dev and fs as argument to the function.


Will try the host_fs_types with optional fs type like "dev|[fs]"

-- 
Thanks
Dave