Upstream Hyper-V Storage VSC Driver Fixes from NetApp
ClosedPublic
Actions

Authored by siderop1_netapp.com on Jul 21 2020, 8:38 PM.

Details

Reviewers

trasz
whu
decui_microsoft.com
sepherosa_gmail.com
allanjude
avg

Commits

rG919a160fc622: Hyper-V: storvsc: Enhance srb_status code handling.
rS364984: Hyper-V: storvsc: Enhance srb_status code handling.

Summary

In hv_storvsc_io_request() when coring, prevent changing of the send channel from the base channel to another one. storvsc_poll always probes on the base channel.

Based upon conversations with Microsoft, changed the handling of srb_status codes. Most we should never get, others yes. All are treated as retry-able except for two. We should not get these statuses, but if we ever do, the I/O state is not known.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 32460
Build 29936: arc lint + arc unit

Event Timeline

siderop1_netapp.com created this revision.Jul 21 2020, 8:38 PM

siderop1_netapp.com held this revision as a draft.

Harbormaster completed remote builds in B32460: Diff 74760.Jul 21 2020, 8:38 PM

siderop1_netapp.com added reviewers: trasz, whu.Jul 21 2020, 8:46 PM

siderop1_netapp.com set the repository for this revision to rS FreeBSD src repository - subversion.

Herald added a subscriber: imp. · View Herald TranscriptJul 21 2020, 8:46 PM

siderop1_netapp.com requested review of this revision.Jul 21 2020, 8:51 PM

allanjude added a subscriber: lwhsu.Jul 21 2020, 10:28 PM

lwhsu added a reviewer: decui_microsoft.com.Jul 22 2020, 10:24 AM

lwhsu added a reviewer: sepherosa_gmail.com.

#ifdef DIAGNOSTIC
Is this only for test purpose? When will this be defined? Thanks.

In D25756#572421, @whu wrote:

#ifdef DIAGNOSTIC
Is this only for test purpose? When will this be defined? Thanks.

Yes, to force set the srb_status value for testing. We use DIAGNOSTIC in all our internal debug (non-production) builds.

Looks good to me. Thanks for fixing this!

whu accepted this revision.Jul 30 2020, 6:08 AM

This revision is now accepted and ready to land.Jul 30 2020, 6:08 AM

allanjude added a subscriber: allanjude.Jul 31 2020, 11:46 PM

allanjude added inline comments.

sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c
2292	This error message doesn't really read like an error message, maybe: "The hypervisor's I/O adapter driver received an unexpected response code 0x%x for operation: %s" But is there some way we can expand this message to inform the user what they are expected to do about this? Or what side effects this might cause?

Looks good, apart from the wording pointed out by Allan.

siderop1_netapp.com added inline comments.Aug 3 2020, 3:11 PM

sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c

2292

Yeah, that's a good idea. Below's what we are thinking. Comments on this before I update the diff?

The hypervisor's I/O adapter driver received an unexpected response code 0x%x for operation: %s. If this continues to occur, report the condition to your hypervisor vendor so they can rectify the issue.

With that change, looks good to me.