Re: [PATCH 2/2] dm-mpath: Add element for suspended state.

[Kiyoshi Ueda <k-ueda@ct.jp.nec.com>]

Hi Alasdair, Mike,

On 11/16/2009 10:54 PM +0900, Alasdair G Kergon wrote:
> On Sun, Nov 15, 2009 at 11:38:29PM -0800, Mike Anderson wrote:
>> Add element to multipath structure for indication of suspended state.
>  
> Is there a way to avoid this?
> It seems redundant for a target to need to track whether or not
> it is suspended.  Core dm should be capable of that.
> 
> dm_suspend has:
>         dm_table_postsuspend_targets(map);
> 
>         set_bit(DMF_SUSPENDED, &md->flags);
> 
> Can we reorder those two?

For multipath and dm-core, yes.
I'm not sure whether other targets care about the ordering.

But from semantics point of view, is it confusing if dm_suspended()
returns true but a target is doing something in postsuspend()?

If we take the approach instead of this patch, the patch-set is like:
    1/4: http://patchwork.kernel.org/patch/61588/
    2/4: http://patchwork.kernel.org/patch/61589/
    3/4: http://patchwork.kernel.org/patch/61594/
    4/4: http://patchwork.kernel.org/patch/61595/

Please review.


> What about dm_resume?
>         clear_bit(DMF_SUSPENDED, &md->flags);
> Can that move a little higher up the function?
> - preresume, clear DMF_SUSPENDED, resume perhaps?

I think we shouldn't move the clear_bit in resume.

It doesn't make sense to clear the flag before I/Os start flowing.
If targets want to do any preparation before I/Os flowing through,
it can do in the resume() callback.

Thanks,
Kiyoshi Ueda