unit test exec_command and friends w.r.t. migration device state (#346)

Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
Reviewed-by: Swapnil Ingle <swapnil.ingle@nutanix.com>

4 files changed, 160 insertions, 95 deletions

diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c
index ab6f4a3..8aeb572 100644
--- a/lib/libvfio-user.c
+++ b/lib/libvfio-user.c
@@ -730,6 +730,38 @@ get_next_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, int *fds,
 UNIT_TEST_SYMBOL(get_next_command);
 #define get_next_command __wrap_get_next_command
 
+bool
+cmd_allowed_when_stopped_and_copying(uint16_t cmd)
+{
+    return cmd == VFIO_USER_REGION_READ ||
+           cmd == VFIO_USER_REGION_WRITE ||
+           cmd == VFIO_USER_DIRTY_PAGES;
+}
+UNIT_TEST_SYMBOL(cmd_allowed_when_stopped_and_copying);
+#define cmd_allowed_when_stopped_and_copying __wrap_cmd_allowed_when_stopped_and_copying
+
+bool
+should_exec_command(vfu_ctx_t *vfu_ctx, uint16_t cmd)
+{
+    assert(vfu_ctx != NULL);
+
+    if (device_is_stopped_and_copying(vfu_ctx->migration)) {
+        if (!cmd_allowed_when_stopped_and_copying(cmd)) {
+            vfu_log(vfu_ctx, LOG_ERR,
+                    "bad command %d while device in stop-and-copy state", cmd);
+            return false;
+        }
+    } else if (device_is_stopped(vfu_ctx->migration) &&
+               cmd != VFIO_USER_DIRTY_PAGES) {
+        vfu_log(vfu_ctx, LOG_ERR,
+               "bad command %d while device in stopped state", cmd);
+        return false;
+    }
+    return true;
+}
+UNIT_TEST_SYMBOL(should_exec_command);
+#define should_exec_command __wrap_should_exec_command
+
 int
 exec_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size,
              int *fds, size_t nr_fds, int **fds_out, size_t *nr_fds_out,
@@ -755,6 +787,10 @@ exec_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size,
         return ret;
     }
 
+    if (!should_exec_command(vfu_ctx, hdr->cmd)) {
+        return -EINVAL;
+    }
+
     cmd_data_size = hdr->msg_size - sizeof (*hdr);
 
     if (cmd_data_size > 0) {
@@ -765,22 +801,6 @@ exec_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size,
         }
     }
 
-    if (device_is_stopped_and_copying(vfu_ctx->migration)
-        && !(hdr->cmd == VFIO_USER_REGION_READ ||
-             hdr->cmd == VFIO_USER_REGION_WRITE ||
-             hdr->cmd == VFIO_USER_DIRTY_PAGES)) {
-        vfu_log(vfu_ctx, LOG_ERR,
-               "bad command %d while device in stop-and-copy state", hdr->cmd);
-        ret = -EINVAL;
-        goto out;
-    } else if (device_is_stopped(vfu_ctx->migration) &&
-               hdr->cmd != VFIO_USER_DIRTY_PAGES) {
-        vfu_log(vfu_ctx, LOG_ERR,
-               "bad command %d while device in stopped state", hdr->cmd);
-        ret = -EINVAL;
-        goto out;
-    }
-
     switch (hdr->cmd) {
     case VFIO_USER_DMA_MAP:
     case VFIO_USER_DMA_UNMAP:
@@ -879,7 +899,6 @@ exec_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size,
         break;
     }
 
-out:
     free(cmd_data);
     return ret;
 }
diff --git a/lib/migration.c b/lib/migration.c
index ec07961..234366c 100644
--- a/lib/migration.c
+++ b/lib/migration.c
@@ -37,87 +37,10 @@
 #include "common.h"
 #include "migration.h"
 #include "private.h"
+#include "migration_priv.h"
 
 /* FIXME no need to use __u32 etc., use uint32_t etc */
 
-/*
- * FSM to simplify saving device state.
- */
-enum migr_iter_state {
-    VFIO_USER_MIGR_ITER_STATE_INITIAL,
-    VFIO_USER_MIGR_ITER_STATE_STARTED,
-    VFIO_USER_MIGR_ITER_STATE_DATA_PREPARED,
-    VFIO_USER_MIGR_ITER_STATE_FINISHED
-};
-
-struct migration {
-    /*
-     * TODO if the user provides an FD then should mmap it and use the migration
-     * registers in the file
-     */
-    struct vfio_device_migration_info info;
-    size_t pgsize;
-    vfu_migration_callbacks_t callbacks;
-    uint64_t data_offset;
-
-    /*
-     * This is only for the saving state. The resuming state is simpler so we
-     * don't need it.
-     */
-    struct {
-        enum migr_iter_state state;
-        __u64 pending_bytes;
-        __u64 offset;
-        __u64 size;
-    } iter;
-};
-
-struct migr_state_data {
-    __u32 state;
-    const char *name;
-};
-
-#define VFIO_DEVICE_STATE_ERROR (VFIO_DEVICE_STATE_SAVING | VFIO_DEVICE_STATE_RESUMING)
-
-/* valid migration state transitions */
-static const struct migr_state_data migr_states[(VFIO_DEVICE_STATE_MASK + 1)] = {
-    [VFIO_DEVICE_STATE_STOP] = {
-        .state = 1 << VFIO_DEVICE_STATE_STOP,
-        .name = "stopped"
-    },
-    [VFIO_DEVICE_STATE_RUNNING] = {
-        .state =
-            (1 << VFIO_DEVICE_STATE_STOP) |
-            (1 << VFIO_DEVICE_STATE_RUNNING) |
-            (1 << VFIO_DEVICE_STATE_SAVING) |
-            (1 << (VFIO_DEVICE_STATE_RUNNING | VFIO_DEVICE_STATE_SAVING)) |
-            (1 << VFIO_DEVICE_STATE_RESUMING) |
-            (1 << VFIO_DEVICE_STATE_ERROR),
-        .name = "running"
-    },
-    [VFIO_DEVICE_STATE_SAVING] = {
-        .state =
-            (1 << VFIO_DEVICE_STATE_STOP) |
-            (1 << VFIO_DEVICE_STATE_SAVING) |
-            (1 << VFIO_DEVICE_STATE_ERROR),
-        .name = "stop-and-copy"
-    },
-    [VFIO_DEVICE_STATE_RUNNING | VFIO_DEVICE_STATE_SAVING] = {
-        .state =
-            (1 << VFIO_DEVICE_STATE_STOP) |
-            (1 << VFIO_DEVICE_STATE_SAVING) |
-            (1 << VFIO_DEVICE_STATE_RUNNING | VFIO_DEVICE_STATE_SAVING) |
-            (1 << VFIO_DEVICE_STATE_ERROR),
-        .name = "pre-copy"
-    },
-    [VFIO_DEVICE_STATE_RESUMING] = {
-        .state =
-            (1 << VFIO_DEVICE_STATE_RUNNING) |
-            (1 << VFIO_DEVICE_STATE_RESUMING) |
-            (1 << VFIO_DEVICE_STATE_ERROR),
-        .name = "resuming"
-    }
-};
 
 bool
 vfio_migr_state_transition_is_valid(__u32 from, __u32 to)
diff --git a/lib/migration_priv.h b/lib/migration_priv.h
new file mode 100644
index 0000000..b8a8734
--- /dev/null
+++ b/lib/migration_priv.h
@@ -0,0 +1,117 @@
+/*
+ * Copyright (c) 2021 Nutanix Inc. All rights reserved.
+ *
+ * Authors: Thanos Makatos <thanos@nutanix.com>
+ *
+ *  Redistribution and use in source and binary forms, with or without
+ *  modification, are permitted provided that the following conditions are met:
+ *      * Redistributions of source code must retain the above copyright
+ *        notice, this list of conditions and the following disclaimer.
+ *      * Redistributions in binary form must reproduce the above copyright
+ *        notice, this list of conditions and the following disclaimer in the
+ *        documentation and/or other materials provided with the distribution.
+ *      * Neither the name of Nutanix nor the names of its contributors may be
+ *        used to endorse or promote products derived from this software without
+ *        specific prior written permission.
+ *
+ *  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ *  AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ *  IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ *  ARE DISCLAIMED. IN NO EVENT SHALL <COPYRIGHT HOLDER> BE LIABLE FOR ANY
+ *  DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ *  (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ *  SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ *  CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ *  LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ *  OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+ *  DAMAGE.
+ *
+ */
+
+#ifndef LIB_VFIO_USER_MIGRATION_PRIV_H
+#define LIB_VFIO_USER_MIGRATION_PRIV_H
+
+#include <linux/vfio.h>
+
+/*
+ * FSM to simplify saving device state.
+ */
+enum migr_iter_state {
+    VFIO_USER_MIGR_ITER_STATE_INITIAL,
+    VFIO_USER_MIGR_ITER_STATE_STARTED,
+    VFIO_USER_MIGR_ITER_STATE_DATA_PREPARED,
+    VFIO_USER_MIGR_ITER_STATE_FINISHED
+};
+
+struct migration {
+    /*
+     * TODO if the user provides an FD then should mmap it and use the migration
+     * registers in the file
+     */
+    struct vfio_device_migration_info info;
+    size_t pgsize;
+    vfu_migration_callbacks_t callbacks;
+    uint64_t data_offset;
+
+    /*
+     * This is only for the saving state. The resuming state is simpler so we
+     * don't need it.
+     */
+    struct {
+        enum migr_iter_state state;
+        __u64 pending_bytes;
+        __u64 offset;
+        __u64 size;
+    } iter;
+};
+
+struct migr_state_data {
+    __u32 state;
+    const char *name;
+};
+
+#define VFIO_DEVICE_STATE_ERROR (VFIO_DEVICE_STATE_SAVING | VFIO_DEVICE_STATE_RESUMING)
+
+/* valid migration state transitions */
+static const struct migr_state_data migr_states[(VFIO_DEVICE_STATE_MASK + 1)] = {
+    [VFIO_DEVICE_STATE_STOP] = {
+        .state = 1 << VFIO_DEVICE_STATE_STOP,
+        .name = "stopped"
+    },
+    [VFIO_DEVICE_STATE_RUNNING] = {
+        .state =
+            (1 << VFIO_DEVICE_STATE_STOP) |
+            (1 << VFIO_DEVICE_STATE_RUNNING) |
+            (1 << VFIO_DEVICE_STATE_SAVING) |
+            (1 << (VFIO_DEVICE_STATE_RUNNING | VFIO_DEVICE_STATE_SAVING)) |
+            (1 << VFIO_DEVICE_STATE_RESUMING) |
+            (1 << VFIO_DEVICE_STATE_ERROR),
+        .name = "running"
+    },
+    [VFIO_DEVICE_STATE_SAVING] = {
+        .state =
+            (1 << VFIO_DEVICE_STATE_STOP) |
+            (1 << VFIO_DEVICE_STATE_SAVING) |
+            (1 << VFIO_DEVICE_STATE_ERROR),
+        .name = "stop-and-copy"
+    },
+    [VFIO_DEVICE_STATE_RUNNING | VFIO_DEVICE_STATE_SAVING] = {
+        .state =
+            (1 << VFIO_DEVICE_STATE_STOP) |
+            (1 << VFIO_DEVICE_STATE_SAVING) |
+            (1 << VFIO_DEVICE_STATE_RUNNING | VFIO_DEVICE_STATE_SAVING) |
+            (1 << VFIO_DEVICE_STATE_ERROR),
+        .name = "pre-copy"
+    },
+    [VFIO_DEVICE_STATE_RESUMING] = {
+        .state =
+            (1 << VFIO_DEVICE_STATE_RUNNING) |
+            (1 << VFIO_DEVICE_STATE_RESUMING) |
+            (1 << VFIO_DEVICE_STATE_ERROR),
+        .name = "resuming"
+    }
+};
+
+#endif
+
+/* ex: set tabstop=4 shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/lib/private.h b/lib/private.h
index edd7b68..8e068c6 100644
--- a/lib/private.h
+++ b/lib/private.h
@@ -184,6 +184,12 @@ long
 dev_get_reginfo(vfu_ctx_t *vfu_ctx, uint32_t index, uint32_t argsz,
                 struct vfio_region_info **vfio_reg, int **fds, size_t *nr_fds);
 
+bool
+cmd_allowed_when_stopped_and_copying(uint16_t cmd);
+
+bool
+should_exec_command(vfu_ctx_t *vfu_ctx, uint16_t cmd);
+
 #endif /* LIB_VFIO_USER_PRIVATE_H */
 
 /* ex: set tabstop=4 shiftwidth=4 softtabstop=4 expandtab: */