Re: [PATCH][linux 2.6.18] remove pointless error handling in scsiback

[Juergen Gross <jgross@suse.com>]

On 07/02/2014 08:52 AM, Jan Beulich wrote:
>>>> On 01.07.14 at 20:51, <JGross@suse.com> wrote:
>> On 07/01/2014 05:39 PM, Jan Beulich wrote:
>>>>>> On 01.07.14 at 17:27, <"jgross@suse.com".non-mime.internet> wrote:
>>>> From: Juergen Gross <jgross@suse.com>
>>>>
>>>> It makes no sense to try repeating traversing the lun list in case of a
>>>> possible mismatch between the original counting of luns and the real
>>>> processing.
>>>
>>> Perhaps the retry logic isn't correct, but with the lock being dropped
>>> between the two loops I'm not sure the retry makes no sense. Can
>>> you explain?
>>
>> In theory the number of LUNs could have just changed between
>> counting them and evaluating them. The counting isn't redone,
>> so what's the point of retrying? What are the chances a matching
>> LUN will appear and disappear in just the correct timing?
>>
>> You could argue the counting should be included in the retry loop.
>> But then doing a limited number of retries is only decreasing the
>> chance of a problem, not eliminating it.
>>
>> The real fix would be to process the LUNs in the first run without
>> using an intermediate buffer by filling the domU's buffer directly.
>> This would avoid all race conditions.
>
> I did a little archaeology and managed to spot where the retry (which
> was added after the initial introduction of pvSCSI) came from. See
> http://lists.xenproject.org/archives/html/xen-devel/2008-07/msg00915.html
> - effectively you're requesting to revert changeset 610:3bcc901cbd7a,
> i.e. you would need to (a) say so in the description and (b) explain
> why the change (and the reasoning having led to it) was wrong. And
> _if_ we indeed decide to undo this, the file should then also be pruned
> of the VSCSI_REPORT_LUNS_RETRY definition.

Okay. I think the reasoning was wrong indeed. After thinking about this
some more time I came to the conclusion that multiple failures are not
completely unlikely when a target with many LUNs is added while the domU
is trying to probe the target (e.g. when the first LUN was detected).
The single LUNs will be added to the backend one after another, so the
count of matching LUNs will increase rather fast.

I'll respin the patch with a proper description.

Juergen