fix: more fixes and responses to comments

Signed-off-by: William Henderson <william.henderson@nutanix.com>

4 files changed, 23 insertions, 13 deletions

diff --git a/include/vfio-user.h b/include/vfio-user.h
index 77b3c43..532e353 100644
--- a/include/vfio-user.h
+++ b/include/vfio-user.h
@@ -229,7 +229,7 @@ struct vfio_user_bitmap_range {
 struct vfio_user_device_feature {
 	uint32_t	argsz;
 	uint32_t	flags;
-#ifndef VFIO_DEVICE_FEATURE_MASK
+#ifndef VFIO_REGION_TYPE_MIGRATION_DEPRECATED
 #define VFIO_DEVICE_FEATURE_MASK	(0xffff)  /* 16-bit feature index */
 #define VFIO_DEVICE_FEATURE_GET		(1 << 16) /* Get feature into data[] */
 #define VFIO_DEVICE_FEATURE_SET		(1 << 17) /* Set feature from data[] */
@@ -241,13 +241,13 @@ struct vfio_user_device_feature {
 /* Analogous to vfio_device_feature_migration */
 struct vfio_user_device_feature_migration {
     uint64_t flags;
-#ifndef VFIO_MIGRATION_STOP_COPY
+#ifndef VFIO_REGION_TYPE_MIGRATION_DEPRECATED
 #define VFIO_MIGRATION_STOP_COPY    (1 << 0)
 #define VFIO_MIGRATION_P2P          (1 << 1)
 #define VFIO_MIGRATION_PRE_COPY     (1 << 2)
 #endif
 } __attribute__((packed));
-#ifndef VFIO_DEVICE_FEATURE_MIGRATION
+#ifndef VFIO_REGION_TYPE_MIGRATION_DEPRECATED
 #define VFIO_DEVICE_FEATURE_MIGRATION 1
 #endif
 _Static_assert(sizeof(struct vfio_user_device_feature_migration) == 8,
diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c
index 6927942..3f2094f 100644
--- a/lib/libvfio-user.c
+++ b/lib/libvfio-user.c
@@ -1044,7 +1044,8 @@ handle_device_feature(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg)
     uint32_t supported_flags =
         migration_feature_flags(req->flags & VFIO_DEVICE_FEATURE_MASK);
 
-    if ((req->flags & supported_flags) != req->flags || supported_flags == 0) {
+    if ((req->flags & supported_flags) !=
+        (req->flags & ~VFIO_DEVICE_FEATURE_MASK) || supported_flags == 0) {
         return -EINVAL;
     }
 
@@ -1086,7 +1087,8 @@ handle_device_feature(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg)
         if (ret < 0) {
             msg->out.iov.iov_len = sizeof(struct vfio_user_device_feature);
         } else {
-            res->argsz = sizeof(struct vfio_user_device_feature) + ret;
+            res->argsz = sizeof(struct vfio_user_device_feature)
+                + sizeof(struct vfio_user_device_feature_migration);
         }
     } else if (req->flags & VFIO_DEVICE_FEATURE_SET) {
         msg->out.iov.iov_base = malloc(msg->in.iov.iov_len);
diff --git a/lib/migration.c b/lib/migration.c
index 80af2e0..5995677 100644
--- a/lib/migration.c
+++ b/lib/migration.c
@@ -198,10 +198,10 @@ migration_feature_get(vfu_ctx_t *vfu_ctx, uint32_t feature, void *buf)
         case VFIO_DEVICE_FEATURE_MIGRATION:
             res = buf;
             // FIXME are these always supported? Can we consider to be
-            //   "supported" if said support is just an empty callback?
+            // "supported" if said support is just an empty callback?
             //
             // We don't need to return RUNNING or ERROR since they are always
-            //   supported.
+            // supported.
             res->flags = VFIO_MIGRATION_STOP_COPY | VFIO_MIGRATION_PRE_COPY;
 
             return 0;
@@ -245,11 +245,14 @@ handle_mig_data_read(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg)
 
     if (migr->state != VFIO_DEVICE_STATE_PRE_COPY
         && migr->state != VFIO_DEVICE_STATE_STOP_COPY) {
-        vfu_log(vfu_ctx, LOG_ERR, "bad state to read data: %d", migr->state);
+        vfu_log(vfu_ctx, LOG_ERR, "bad migration state to read data: %d",
+                migr->state);
         return -EINVAL;
     }
 
     if (req->size > vfu_ctx->client_max_data_xfer_size) {
+        vfu_log(vfu_ctx, LOG_ERR, "transfer size exceeds limit (%ld > %ld)",
+                req->size, vfu_ctx->client_max_data_xfer_size);
         return -EINVAL;
     }
 
@@ -289,7 +292,8 @@ handle_mig_data_write(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg)
     }
 
     if (migr->state != VFIO_DEVICE_STATE_RESUMING) {
-        vfu_log(vfu_ctx, LOG_ERR, "bad state to write data: %d", migr->state);
+        vfu_log(vfu_ctx, LOG_ERR, "bad migration state to write data: %d",
+                migr->state);
         return -EINVAL;
     }
 
diff --git a/lib/migration_priv.h b/lib/migration_priv.h
index c20982c..2566b39 100644
--- a/lib/migration_priv.h
+++ b/lib/migration_priv.h
@@ -45,10 +45,14 @@ struct migration {
    if the bit is set then the transition is allowed
    the indices of each state are those in the vfio_user_device_mig_state enum */
 static const char transitions[8] = {
-    // ERROR    STOP        RUNNING     STOP_COPY
-    0b00000000, 0b00111000, 0b01000010, 0b01000000,
-    // RESUMING RUNNING_P2P PRE_COPY    PRE_COPY_P2P 
-    0b01000000, 0b00000000, 0b00110000, 0b00000000
+    0b00000000, // ERROR        -> {}
+    0b00011100, // STOP         -> {RUNNING, STOP_COPY, RESUMING}
+    0b01000010, // RUNNING      -> {STOP, PRE_COPY}
+    0b00000010, // STOP_COPY    -> {STOP}
+    0b00000010, // RESUMING     -> {STOP}
+    0b00000000, // RUNNING_P2P  -> {}
+    0b00001100, // PRE_COPY     -> {RUNNING, STOP_COPY}
+    0b00000000  // PRE_COPY_P2P -> {}
 };
 
 MOCK_DECLARE(vfu_migr_state_t, migr_state_vfio_to_vfu, uint32_t device_state);