[ovs-dev] [PATCH] windows: Crash when the handle communication device cannot be found

[Shashank Ram]

________________________________
From: ovs-dev-bounces at openvswitch.org <ovs-dev-bounces at openvswitch.org> on behalf of Alin Serdean <aserdean at cloudbasesolutions.com>
Sent: Thursday, March 9, 2017 1:58 AM
To: Ben Pfaff
Cc: dev at openvswitch.org
Subject: Re: [ovs-dev] [PATCH] windows: Crash when the handle communication device cannot be found


> On Fri, Feb 17, 2017 at 12:35:08PM +0000, Alin Serdean wrote:
> > When trying to uninstall/disable the OVS extension the driver will
> > fail to unload properly(require reboot)/hang until ovs-vswitchd is closed.
> >
> > The root cause of this behavior is because the handles from
> > ovs-vswitchd to the kernel communication devices are still opened
> > although the actual device was removed from the kernel.
> >
> > Trying to close the handles will also fail because they do not exist.
> >
> > The remaining option is to cause a crash and rely on the service
> > manager to restart ovs-vswitchd.
> >
> > Reported-at: https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_27&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=6OuVHk-mnufSWzkKa74UkQ&m=Qs6sW_toEOsD2wL_kBa4oDCU-aaQKUthrM4uzIzdViI&s=eJ7_ShFGcVZOY6rswSe0TB1D-IPhDiHA55Xn_0e96nA&e=
> > Reported-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
> > Signed-off-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
>
> I suggest using ovs_abort() instead of ovs_fatal() in this case, because of the
> intent of ovs_abort():
>
>  * This function is preferred to ovs_fatal() in a situation where it would make
>  * sense for a monitoring process to restart the daemon.
>
> On Linux, using ovs_abort() actually causes the monitoring process to
> automatically restart the daemon.  On Windows, I don't know whether it has
> that effect, but it at least makes the intent clear in the source code.
>
> I'd still like to see a review from a Windows developer.
[Alin Serdean] Thanks for the review Ben. Both `ovs_fatal()` and `ovs_abort()` will have the same outcome(restarting the service).
[SR] Thanks for the patch Alin. Causing a crash using either of ovs_fatal() and ovs_abort() will result in the service manager restarting the service. But like Ben suggested, maybe it would be better to use ovs_abort() for the sake of consistency with Linux in this case?
_______________________________________________
dev mailing list
dev at openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=6OuVHk-mnufSWzkKa74UkQ&m=Qs6sW_toEOsD2wL_kBa4oDCU-aaQKUthrM4uzIzdViI&s=dcrYnHY7KbeFFkzIhmi2cnGvRTw6Mij5ppikIcVxEMs&e=