Viostor: Introduce action-on-reset feature from vioscsi

[MartinDrab]

The action-on-reset determines what the driver should do when a bus reset request arrives. It can be configured to:

• complete pending requests,
• do nothing,
• bug check.

This feature is already present in vioscsi but not in viostor.

Needs to be merged after #1297.

[benyamin-codez]

👍

More registry love.
Nice..!

It looks like clang-format is upset.
You can find usage for the Tools/clang-format-helper.sh script here.
Probably line 490 wanting to look something like:

VioStorReadRegistryParameter(DeviceExtension,
                             REGISTRY_ACTION_ON_RESET,
                             FIELD_OFFSET(ADAPTER_EXTENSION, action_on_reset));

...but other lines look close to the column boundary too, so there might be more.

P.S.:
Just checked and it reports lines 490 & 1213 in virtio_stor.c and lines 209 and 267 in virtio_stor.h.
That mod for line 490 above is correct.

[MartinDrab]

👍

More registry love. Nice..!

It looks like clang-format is upset. You can find usage for the Tools/clang-format-helper.sh script here. Probably line 490 wanting to look something like:

VioStorReadRegistryParameter(DeviceExtension,
                             REGISTRY_ACTION_ON_RESET,
                             FIELD_OFFSET(ADAPTER_EXTENSION, action_on_reset));

...but other lines look close to the column boundary too, so there might be more.

P.S.: Just checked and it reports lines 490 & 1213 in virtio_stor.c and lines 209 and 267 in virtio_stor.h. That mod for line 490 above is correct.

Yeah, I expected clang-format-check to complain and plan to fix that when #1297 gets merged (and I rebase above it).

[lixianming19951001]

I would like to ask another question: We control the behavior of action_on_reset through the registry, but by default, it still completes all pending requests. Does this not pose a risk of memory access when the host processes requests that have already been completed?

[benyamin-codez]

I would like to ask another question: We control the behavior of action_on_reset through the registry, but by default, it still completes all pending requests. Does this not pose a risk of memory access when the host processes requests that have already been completed?

Thanks Li.

Again, Martin might know best, but viostor has the additional function CompleteRequestWithStatus() (as pointed out in my comment here - not that I'm saying here that it does need to be removed - just that it is different to vioscsi), which will complete the requests using the specified status. StorPort is eventually notified in CompleteSRB(). I would have thought that if StorPort has been notified, provided the host process makes the request via StorPort, then it should be safe.

[MartinDrab]

I would like to ask another question: We control the behavior of action_on_reset through the registry, but by default, it still completes all pending requests. Does this not pose a risk of memory access when the host processes requests that have already been completed?

Hello,

I apologize for long delay. The risk is there in case the pending requests get completed as a result of bus reset request from the OS (and the host may complete some of them at the same moment -- which means access to memory no longer belonging to the requests).

I will add commit setting the action_on_reset on DoNothing during the installation.

[YanVugenfirer]

ok to test