[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

On 07/02/2014 08:52 AM, Jan Beulich wrote:
On 01.07.14 at 20:51, <JGross@xxxxxxxx> wrote:
On 07/01/2014 05:39 PM, Jan Beulich wrote:
On 01.07.14 at 17:27, <"jgross@xxxxxxxx".non-mime.internet> wrote:
From: Juergen Gross <jgross@xxxxxxxx>

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

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
- 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.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.