-
Notifications
You must be signed in to change notification settings - Fork 379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[netkvm] does netkvm can support NDIS_MINIPORT_ATTRIBUTES_SURPRISE_REMOVE_OK? #1002
Comments
@vrozenfe I think we handle it on the host side for the storage devices, right? |
Hi Yan, Cheers, |
@zjmletang please check Vadim's comment. |
For further discussion: IMO, it is good to add it if it does not break anything including HCK and hot plug/unplug functionality (IMO it should not). For netkvm adding this attribute effectively removes it from the "eject" list. IMO again we do not need to add any code except of this attribute declaration as we anyway support the surprise removal. |
@YanVugenfirer ok, thanks. |
@ybendito ,If that's the case, may I submit a PR (just to add this attribute) directly? |
@YanVugenfirer @vrozenfe I see for storage drivers it was discussed earlier #366 |
I think it was breaking HCK. So before doing it, please run full HCK test and check that it is OK |
@zjmletang Yes, you welcome to submit such PR. First of all we'll see that the HCK is ok with it. Later we'll need to check that this does not affect hot unplugging flow. |
Signed-off-by: Zhang JianMing <zhangjianming.zjm@alibaba-inc.com> for the problem description please visit virtio-win#1002
Signed-off-by: Zhang JianMing <zhangjianming.zjm@alibaba-inc.com> for the problem description please visit virtio-win#1002
Signed-off-by: Zhang JianMing <zhangjianming.zjm@alibaba-inc.com> for the problem description please visit #1002
@ybendito Over the past two days, while retesting, I've discovered an issue: after adding this feature, the network adapter does disappear from the "eject" list following the installation of a new driver. However, after each VM restart, the network adapter reappears in the "eject" list. At this point, if I reinstall the driver, or first disable and then enable the driver, the network adapter once again disappears from the list. I am unable to explain this point at the moment. (´;д;`) |
Probably the first appearance after VM start depends on data collected before the driver starts (on root port hotplug support). If you disable the driver in the device manager and restart the VM - the adapter appears in the list, enable the driver - disappears, disable the driver - does not appear... I agree, this OS behavior is kind of inconsistent, but we did what we're able to. |
Is your feature request related to a problem? Please describe.
For the current version, Windows displays a pop-up for devices, and sometimes users accidentally click on eject, making the device invisible. I'm considering whether it's possible to prevent network card devices from appearing in the eject device list, so users won't accidentally click on them.
on the other hand, as the msdn(https://learn.microsoft.com/en-us/windows-hardware/drivers/network/handling-the-surprise-removal-of-a-nic) says:
Miniport drivers for Windows Vista and later versions of the operating system should be able to handle surprise removals. In particular, NDIS miniport drivers with a Windows Driver Model (WDM) lower edge should be able to handle such events. If an NDIS-WDM miniport driver does not handle a surprise removal, any pending IRPs that the miniport driver sent to the underlying bus driver before the surprise removal cannot be completed.
Describe the solution you'd like
solution: netkvm support NDIS_MINIPORT_ATTRIBUTES_SURPRISE_REMOVE_OK
One area that needs to be modified is as follows,But I haven't studied yet whether there are other places that need to be modified.
miniportAttributes.RegistrationAttributes.AttributeFlags =
// actual for USB
NDIS_MINIPORT_ATTRIBUTES_SURPRISE_REMOVE_OK|
NDIS_MINIPORT_ATTRIBUTES_HARDWARE_DEVICE |
NDIS_MINIPORT_ATTRIBUTES_BUS_MASTER;
#ifndef NO_VISTA_POWER_MANAGEMENT
miniportAttributes.RegistrationAttributes.AttributeFlags |= NDIS_MINIPORT_ATTRIBUTES_NO_HALT_ON_SUSPEND;
#endif
#if NDIS_SUPPORT_NDIS630
miniportAttributes.RegistrationAttributes.AttributeFlags |= NDIS_MINIPORT_ATTRIBUTES_NO_PAUSE_ON_SUSPEND;
#endif
The text was updated successfully, but these errors were encountered: