Page MenuHomeFreeBSD
Differential D25756

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

Authored by siderop1_netapp.com on Jul 21 2020, 8:38 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Oct 22, 2:45 PM
Unknown Object (File)
Oct 3 2024, 12:11 PM
Unknown Object (File)
Oct 2 2024, 11:59 PM
Unknown Object (File)
Oct 2 2024, 11:16 PM
Unknown Object (File)
Oct 1 2024, 10:04 AM
Unknown Object (File)
Sep 27 2024, 2:14 AM
Unknown Object (File)
Sep 12 2024, 7:48 PM
Unknown Object (File)
Sep 11 2024, 10:16 AM
View All 56 Files
Subscribers
allanjude
imp
lwhsu

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

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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.
whu added a comment.Jul 28 2020, 5:13 AM

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

siderop1_netapp.com added a comment.Jul 29 2020, 4:21 PM
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.

whu added a comment.Jul 30 2020, 6:07 AM

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 ↗(On Diff #74760)

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?

trasz accepted this revision.Aug 1 2020, 3:02 PM

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 ↗(On Diff #74760)

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.
allanjude accepted this revision.Aug 6 2020, 1:53 PM

With that change, looks good to me.

sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c
2292 ↗(On Diff #74760)

Yes, this is exactly what I had in mind.

siderop1_netapp.com updated this revision to Diff 75512.Aug 6 2020, 5:18 PM

update error message based on review feedback

This revision now requires review to proceed.Aug 6 2020, 5:18 PM
Harbormaster completed remote builds in B32815: Diff 75512.Aug 6 2020, 5:18 PM
allanjude accepted this revision.Aug 11 2020, 7:14 PM
This revision is now accepted and ready to land.Aug 11 2020, 7:14 PM
allanjude added a comment.Aug 11 2020, 7:15 PM

@whu Will you commit this? Or would you like someone else to?

allanjude added a reviewer: avg.Aug 24 2020, 6:06 PM
whu added a comment.Aug 31 2020, 7:00 AM
In D25756#577469, @allanjude wrote:

@whu Will you commit this? Or would you like someone else to?

@allanjude Sorry I was OOTO for a while for personal reason and just came back. Yes, I will commit this.

Closed by commit rS364984: Hyper-V: storvsc: Enhance srb_status code handling. (authored by whu). · Explain WhyAug 31 2020, 9:05 AM
This revision was automatically updated to reflect the committed changes.
whu added a commit: rS364984: Hyper-V: storvsc: Enhance srb_status code handling..
whu added a commit: rG919a160fc622: Hyper-V: storvsc: Enhance srb_status code handling..Mar 15 2021, 5:11 AM

Revision Contents
Changeset List

PathSize
head/
sys/
dev/
hyperv/
storvsc/
158 lines
34 lines

Diff 76389

View Options

head/sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c

Loading...
View Options

head/sys/dev/hyperv/storvsc/hv_vstorage.h

Loading...