[ovs-dev] [PATCH 4/4] daemon-windows: Add users for windows services.

[Gurucharan Shetty]

On Fri, Jan 17, 2014 at 1:37 PM, Ben Pfaff <blp at nicira.com> wrote:
> On Fri, Jan 17, 2014 at 01:30:27PM -0800, Gurucharan Shetty wrote:
>> On Fri, Jan 17, 2014 at 1:13 PM, Ben Pfaff <blp at nicira.com> wrote:
>> > On Fri, Jan 17, 2014 at 12:26:23PM -0800, Gurucharan Shetty wrote:
>> >> Start with ovs-vswitchd and ovsdb-server.
>> >>
>> >> Signed-off-by: Gurucharan Shetty <gshetty at nicira.com>
>> >
>> > Will poll_block() wake up if a service stop has been requested?
>> > Ideally, it would.
>> I did not think a lot about it. I suppose we have a time limit in ms
>> after which we wake up every time correct?
>
> No.  In ovs-vswitchd, for example, if nothing is happening, and no
> bridges are configured, then poll_block() can block indefinitely.

Okay.
>
>> I haven't seen (during my tests) a negative consequence of a small
>> delay in stopping the service after a request. If that is indeed a
>> problem, I will investigate more.
>
> I assume a small delay is OK, too, but I don't think that the delay
> here is bounded.
>
I see that now. I think the following incremental that uses a
CreateEvent and SetEvent in daemon-windows.c (on top of patch3 of the
series) will fix the problem. The patch makes an assumption that
poll_block() will only be called at one place in the main thread's
loop. Otherwise, between two poll_block(), I will have to figure out a
way to call a poll_fd_wait() to re-register a event. Is my assumption
correct?


Incremental:
+/* Handle to an event object used to wakeup from poll_block(). */
+static HANDLE wevent;
+
 /* Hold the arguments sent to the main function. */
 static int sargc;
 static char ***sargvp;
@@ -82,6 +86,14 @@
     if (detached) {
         init_service_status();

+        wevent = CreateEvent(NULL, TRUE, FALSE, NULL);
+        if (!wevent) {
+            ovs_lasterror_to_string((LPTSTR)&msg_buf);
+            VLOG_FATAL("Failed to create a event (%s).", msg_buf);
+        }
+
+        poll_fd_wait(0, wevent, POLLIN);
+
         /* Register the control handler. This function is called by the service
          * manager to stop the service. */
         hstatus = RegisterServiceCtrlHandler(program_name,
@@ -162,6 +174,7 @@
     case SERVICE_CONTROL_SHUTDOWN:
         service_status.dwCurrentState = SERVICE_STOPPED;
         service_status.dwWin32ExitCode = NO_ERROR;
+        SetEvent(wevent);
         break;

     default:
@@ -174,8 +187,12 @@
 bool
 should_service_stop(void)
 {
-    if (detached && service_status.dwCurrentState != SERVICE_RUNNING) {
-        return true;
+    if (detached) {
+        if (service_status.dwCurrentState != SERVICE_RUNNING) {
+            return true;
+        } else {
+            poll_fd_wait(0, wevent, POLLIN);
+        }
     }
     return false;
 }
@@ -186,6 +203,9 @@
 void
 service_stop()
 {
+    ResetEvent(wevent);
+    CloseHandle(wevent);
+
     service_status.dwCurrentState = SERVICE_STOPPED;
     service_status.dwWin32ExitCode = NO_ERROR;
     SetServiceStatus(hstatus, &service_status);