[Stgt-devel] [PATCH] Make changes so that VTL stuff works properly ...

[Mark Harvey]

On Wed, Jul 30, 2008 at 1:28 PM, Mark Harvey <markh794 at gmail.com> wrote:
> My concern is with
>
> tgt_device_path_update()
> {
>    if(lu->path) {
>        ....
>        lu->bst->bs_close(lu);
>        ....
>    }
> }
>
> bs_close()
> {
>    ....
>    bs_thread_close();
>    ....
> }
>
>
> What will occur if there are multiple LU attached to this target and
> one LU is activately being read/written to while another LU has its
> backing file path updated ?
>
> I assume - multithreaded code is new experence for me - that we will
> result in a core dump as the backing store threads have been torn
> down.
>
> Cheers
> Mark

Sorry - forgot to add. If this is the case, Richards patch will not
fix the over-all problem.

The dtd_load_unload() only opens/closes the backing file. It does not
attempt to initialize the threads.


I feel the correct fix for this is:

To add  bs_init()/bs_exit() ?? hook to initialize/shutdown the threads.
And the bs_open()/bs_close() to open/close the backing store file.

The bs_init() should be called at setup time and bs_open()/bs_close()
when media loaded or unloaded.

I'll attempt to put something in code this afternoon and post a POC patch.

Cheers
Mark
>
> On Wed, Jul 30, 2008 at 12:52 PM, FUJITA Tomonori
> <fujita.tomonori at lab.ntt.co.jp> wrote:
>> On Wed, 30 Jul 2008 12:45:05 +1000
>> "Mark Harvey" <markh794 at gmail.com> wrote:
>>
>>> OK, I've reproduced the core dump as well.
>>>
>>> All my testing involved running the scripts/tgt-core-setup script.
>>>
>>> This script (incorrectly) adds a backing file - so I never experienced
>>> this core dump. When configured to startup with a backing file - loads
>>> and unloads work without error.
>>>
>>> Setting up the device without an initial backing file results in the
>>> core dump once a write is attempted.
>>
>> Should I apply Richard's patch (Make changes so that VTL stuff works
>> properly)? Or we need cleanups before applying it?
>>
>> Sorry, I have not had a chance to reproduce it and read the patch yet.
>>
>