Fix unplugging of emulated devices on resume from suspend

Due to a mis-ordering of the interface initialization calls in FdoCreate(),
the SUSPEND interface never gets hold of the UNPLUG interface and thus,
on resume from suspend, emulated device unplug is not done and the
emulated network and disk devices re-appear in the VM.
This patch re-orders the initialization code to fix this problem and also
makes SuspendTrigger() fail if the UNPLUG interface is not available.

NOTE: The change of type of SuspendTrigger() does not require a new
      interface revision since it is a change from a void function to a
      non-void function, so older client code will simply ignore the return
      value.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

diff --git a/include/suspend_interface.h b/include/suspend_interface.h
index df1b4b0d329defba64dd216f7c379e7c8f8b2cbe..cbe11ab6d9e4f4ad21ac54f0edbc6483728a577c 100644 (file)
--- a/include/suspend_interface.h
+++ b/include/suspend_interface.h
@@ -126,7 +126,7 @@ typedef VOID
 
     This method must always be invoked with IRQL == PASSIVE_LEVEL
 */
-typedef VOID
+typedef NTSTATUS
 (*XENBUS_SUSPEND_TRIGGER)(
     IN  PINTERFACE  Interface
     );

diff --git a/src/xenbus/fdo.c b/src/xenbus/fdo.c
index 3c3e9a091268737f0e52b8bb4e25bc66675673dc..cbd2d9aa7c8a260f1b1a7d6a24e35020634db7b0 100644 (file)
--- a/src/xenbus/fdo.c
+++ b/src/xenbus/fdo.c
@@ -1272,7 +1272,7 @@ FdoSuspend(
                             "control",
                             "shutdown");
 
-        XENBUS_SUSPEND(Trigger, &Fdo->SuspendInterface);
+        (VOID) XENBUS_SUSPEND(Trigger, &Fdo->SuspendInterface);
 
         __FdoSuspendClearActive(Fdo);
 
@@ -4581,42 +4581,51 @@ FdoCreate(
     if (!__FdoIsActive(Fdo))
         goto done;
 
-    status = DebugInitialize(Fdo, &Fdo->DebugContext);
+    status = FDO_QUERY_INTERFACE(Fdo,
+                                 XENFILT,
+                                 UNPLUG,
+                                 (PINTERFACE)&Fdo->UnplugInterface,
+                                 sizeof (Fdo->UnplugInterface),
+                                 TRUE);
     if (!NT_SUCCESS(status))
         goto fail7;
 
-    status = SuspendInitialize(Fdo, &Fdo->SuspendContext);
+    status = DebugInitialize(Fdo, &Fdo->DebugContext);
     if (!NT_SUCCESS(status))
         goto fail8;
 
-    status = SharedInfoInitialize(Fdo, &Fdo->SharedInfoContext);
+    status = SuspendInitialize(Fdo, &Fdo->SuspendContext);
     if (!NT_SUCCESS(status))
         goto fail9;
 
-    status = EvtchnInitialize(Fdo, &Fdo->EvtchnContext);
+    status = SharedInfoInitialize(Fdo, &Fdo->SharedInfoContext);
     if (!NT_SUCCESS(status))
         goto fail10;
 
-    status = StoreInitialize(Fdo, &Fdo->StoreContext);
+    status = EvtchnInitialize(Fdo, &Fdo->EvtchnContext);
     if (!NT_SUCCESS(status))
         goto fail11;
 
-    status = RangeSetInitialize(Fdo, &Fdo->RangeSetContext);
+    status = StoreInitialize(Fdo, &Fdo->StoreContext);
     if (!NT_SUCCESS(status))
         goto fail12;
 
-    status = CacheInitialize(Fdo, &Fdo->CacheContext);
+    status = RangeSetInitialize(Fdo, &Fdo->RangeSetContext);
     if (!NT_SUCCESS(status))
         goto fail13;
 
-    status = GnttabInitialize(Fdo, &Fdo->GnttabContext);
+    status = CacheInitialize(Fdo, &Fdo->CacheContext);
     if (!NT_SUCCESS(status))
         goto fail14;
 
+    status = GnttabInitialize(Fdo, &Fdo->GnttabContext);
+    if (!NT_SUCCESS(status))
+        goto fail15;
+
     if (FdoIsBalloonEnabled(Fdo)) {
         status = BalloonInitialize(Fdo, &Fdo->BalloonContext);
         if (!NT_SUCCESS(status))
-            goto fail15;
+            goto fail16;
     }
 
     status = DebugGetInterface(__FdoGetDebugContext(Fdo),
@@ -4660,15 +4669,6 @@ FdoCreate(
                                  sizeof (Fdo->BalloonInterface));
     ASSERT(NT_SUCCESS(status));
 
-    status = FDO_QUERY_INTERFACE(Fdo,
-                                 XENFILT,
-                                 UNPLUG,
-                                 (PINTERFACE)&Fdo->UnplugInterface,
-                                 sizeof (Fdo->UnplugInterface),
-                                 TRUE);
-    if (!NT_SUCCESS(status))
-        goto fail16;
-
 done:
     InitializeMutex(&Fdo->Mutex);
     InitializeListHead(&Dx->ListEntry);
@@ -4687,76 +4687,56 @@ done:
 fail16:
     Error("fail16\n");
 
-    RtlZeroMemory(&Fdo->BalloonInterface,
-                  sizeof (XENBUS_BALLOON_INTERFACE));
-
-    RtlZeroMemory(&Fdo->RangeSetInterface,
-                  sizeof (XENBUS_RANGE_SET_INTERFACE));
-
-    RtlZeroMemory(&Fdo->StoreInterface,
-                  sizeof (XENBUS_STORE_INTERFACE));
-
-    RtlZeroMemory(&Fdo->EvtchnInterface,
-                  sizeof (XENBUS_EVTCHN_INTERFACE));
-
-    RtlZeroMemory(&Fdo->SuspendInterface,
-                  sizeof (XENBUS_SUSPEND_INTERFACE));
-
-    RtlZeroMemory(&Fdo->DebugInterface,
-                  sizeof (XENBUS_DEBUG_INTERFACE));
-
-    if (Fdo->BalloonContext != NULL) {
-        BalloonTeardown(Fdo->BalloonContext);
-        Fdo->BalloonContext = NULL;
-    }
+    GnttabTeardown(Fdo->GnttabContext);
+    Fdo->GnttabContext = NULL;
 
 fail15:
     Error("fail15\n");
 
-    GnttabTeardown(Fdo->GnttabContext);
-    Fdo->GnttabContext = NULL;
+    CacheTeardown(Fdo->CacheContext);
+    Fdo->CacheContext = NULL;
 
 fail14:
     Error("fail14\n");
 
-    CacheTeardown(Fdo->CacheContext);
-    Fdo->CacheContext = NULL;
+    RangeSetTeardown(Fdo->RangeSetContext);
+    Fdo->RangeSetContext = NULL;
 
 fail13:
     Error("fail13\n");
 
-    RangeSetTeardown(Fdo->RangeSetContext);
-    Fdo->RangeSetContext = NULL;
+    StoreTeardown(Fdo->StoreContext);
+    Fdo->StoreContext = NULL;
 
 fail12:
     Error("fail12\n");
 
-    StoreTeardown(Fdo->StoreContext);
-    Fdo->StoreContext = NULL;
+    EvtchnTeardown(Fdo->EvtchnContext);
+    Fdo->EvtchnContext = NULL;
 
 fail11:
     Error("fail11\n");
 
-    EvtchnTeardown(Fdo->EvtchnContext);
-    Fdo->EvtchnContext = NULL;
+    SharedInfoTeardown(Fdo->SharedInfoContext);
+    Fdo->SharedInfoContext = NULL;
 
 fail10:
     Error("fail10\n");
 
-    SharedInfoTeardown(Fdo->SharedInfoContext);
-    Fdo->SharedInfoContext = NULL;
+    SuspendTeardown(Fdo->SuspendContext);
+    Fdo->SuspendContext = NULL;
 
 fail9:
     Error("fail9\n");
 
-    SuspendTeardown(Fdo->SuspendContext);
-    Fdo->SuspendContext = NULL;
+    DebugTeardown(Fdo->DebugContext);
+    Fdo->DebugContext = NULL;
 
 fail8:
     Error("fail8\n");
 
-    DebugTeardown(Fdo->DebugContext);
-    Fdo->DebugContext = NULL;
+    RtlZeroMemory(&Fdo->UnplugInterface,
+                  sizeof (XENFILT_UNPLUG_INTERFACE));
 
 fail7:
     Error("fail7\n");
@@ -4831,9 +4811,6 @@ FdoDestroy(
     RtlZeroMemory(&Fdo->Mutex, sizeof (MUTEX));
 
     if (__FdoIsActive(Fdo)) {
-        RtlZeroMemory(&Fdo->UnplugInterface,
-                      sizeof (XENFILT_UNPLUG_INTERFACE));
-
         RtlZeroMemory(&Fdo->BalloonInterface,
                       sizeof (XENBUS_BALLOON_INTERFACE));
 
@@ -4881,6 +4858,9 @@ FdoDestroy(
         DebugTeardown(Fdo->DebugContext);
         Fdo->DebugContext = NULL;
 
+        RtlZeroMemory(&Fdo->UnplugInterface,
+                      sizeof (XENFILT_UNPLUG_INTERFACE));
+
         __FdoSetActive(Fdo, FALSE);
     }
 

diff --git a/src/xenbus/suspend.c b/src/xenbus/suspend.c
index 663ed8e7753386f8bd3dd61d683e352eb6d7da8d..b8267951b92036a79efb734afc2c939492667c94 100644 (file)
--- a/src/xenbus/suspend.c
+++ b/src/xenbus/suspend.c
@@ -143,7 +143,7 @@ SuspendDeregister(
     __SuspendFree(Callback);
 }
 
-VOID
+NTSTATUS
 #pragma prefast(suppress:28167) // Function changes IRQL
 SuspendTrigger(
     IN  PINTERFACE          Interface
@@ -153,6 +153,10 @@ SuspendTrigger(
     KIRQL                   Irql;
     NTSTATUS                status;
 
+    status = STATUS_NOT_SUPPORTED;
+    if (Context->UnplugInterface.Interface.Context == NULL)
+        goto fail1;
+
     KeRaiseIrql(DISPATCH_LEVEL, &Irql);
 
     LogPrintf(LOG_LEVEL_INFO,
@@ -175,8 +179,7 @@ SuspendTrigger(
 
         HypercallPopulate();
 
-        if (Context->UnplugInterface.Interface.Context != NULL)
-            XENFILT_UNPLUG(Replay, &Context->UnplugInterface);
+        XENFILT_UNPLUG(Replay, &Context->UnplugInterface);
 
         for (ListEntry = Context->EarlyList.Flink;
              ListEntry != &Context->EarlyList;
@@ -211,6 +214,13 @@ SuspendTrigger(
     LogPrintf(LOG_LEVEL_INFO, "SUSPEND: <====\n");
 
     KeLowerIrql(Irql);
+
+    return STATUS_SUCCESS;
+
+fail1:
+    Error("fail1 (%08x)\n", status);
+
+    return status;
 }
 
 static ULONG

diff --git a/src/xenfilt/unplug.c b/src/xenfilt/unplug.c
index 37562c55c42cd52ac57488efe05e3ee4bdef6cd1..0c0948db70dbf48af7fda260e4b2b24fbd2d4db3 100644 (file)
--- a/src/xenfilt/unplug.c
+++ b/src/xenfilt/unplug.c
@@ -183,8 +183,6 @@ done:
     return STATUS_SUCCESS;
 
 fail1:
-    Error("fail1 (%08x)\n", status);
-
     return status;
 }