From 7481e4d959c5ee12c5bc7577fc0c70b29bf0c054 Mon Sep 17 00:00:00 2001
From: Rob Bradford <robert.bradford@intel.com>
Date: Fri, 24 Apr 2020 14:46:48 +0100
Subject: [PATCH] vmm: config: Validate that shared memory is enabled if using
 vhost-user

Check that if any device using vhost-user (net & disk with
vhost_user=true) or virtio-fs is enabled then check shared memory is
also enabled.

Fixes: #848

Signed-off-by: Rob Bradford <robert.bradford@intel.com>
---
 src/main.rs       | 31 +++++++++++++++++++++-
 vmm/src/config.rs | 65 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/src/main.rs b/src/main.rs
index 4e8249a0e..bc2213b1e 100755
--- a/src/main.rs
+++ b/src/main.rs
@@ -749,12 +749,15 @@ mod unit_tests {
                     "cloud-hypervisor",
                     "--kernel",
                     "/path/to/kernel",
+                    "--memory",
+                    "shared=true",
                     "--disk",
                     "vhost_user=true,socket=/tmp/socket1",
                     "path=/path/to/disk/2",
                 ],
                 r#"{
                     "kernel": {"path": "/path/to/kernel"},
+                    "memory" : { "shared": true, "size": 536870912 },
                     "disks": [
                         {"vhost_user":true, "vhost_socket":"/tmp/socket1"},
                         {"path": "/path/to/disk/2"}
@@ -767,12 +770,15 @@ mod unit_tests {
                     "cloud-hypervisor",
                     "--kernel",
                     "/path/to/kernel",
+                    "--memory",
+                    "shared=true",
                     "--disk",
                     "vhost_user=true,socket=/tmp/socket1,wce=true",
                     "path=/path/to/disk/2",
                 ],
                 r#"{
                     "kernel": {"path": "/path/to/kernel"},
+                    "memory" : { "shared": true, "size": 536870912 },
                     "disks": [
                         {"vhost_user":true, "vhost_socket":"/tmp/socket1", "wce":true},
                         {"path": "/path/to/disk/2"}
@@ -953,9 +959,10 @@ mod unit_tests {
                 true,
             ),
             (
-                vec!["cloud-hypervisor", "--kernel", "/path/to/kernel", "--net", "mac=12:34:56:78:90:ab,vhost_user=true,socket=/tmp/socket"],
+                vec!["cloud-hypervisor", "--kernel", "/path/to/kernel", "--memory", "shared=true", "--net", "mac=12:34:56:78:90:ab,vhost_user=true,socket=/tmp/socket"],
                 r#"{
                     "kernel": {"path": "/path/to/kernel"},
+                    "memory" : { "shared": true, "size": 536870912 },
                     "net": [
                         {"mac": "12:34:56:78:90:ab", "vhost_user": true, "vhost_socket": "/tmp/socket"}
                     ]
@@ -997,12 +1004,14 @@ mod unit_tests {
             (
                 vec![
                     "cloud-hypervisor", "--kernel", "/path/to/kernel",
+                    "--memory", "shared=true",
                     "--fs",
                     "tag=virtiofs1,sock=/path/to/sock1",
                     "tag=virtiofs2,sock=/path/to/sock2",
                 ],
                 r#"{
                     "kernel": {"path": "/path/to/kernel"},
+                    "memory" : { "shared": true, "size": 536870912 },
                     "fs": [
                         {"tag": "virtiofs1", "sock": "/path/to/sock1"},
                         {"tag": "virtiofs2", "sock": "/path/to/sock2"}
@@ -1013,12 +1022,14 @@ mod unit_tests {
             (
                 vec![
                     "cloud-hypervisor", "--kernel", "/path/to/kernel",
+                    "--memory", "shared=true",
                     "--fs",
                     "tag=virtiofs1,sock=/path/to/sock1",
                     "tag=virtiofs2,sock=/path/to/sock2",
                 ],
                 r#"{
                     "kernel": {"path": "/path/to/kernel"},
+                    "memory" : { "shared": true, "size": 536870912 },
                     "fs": [
                         {"tag": "virtiofs1", "sock": "/path/to/sock1"}
                     ]
@@ -1028,11 +1039,13 @@ mod unit_tests {
             (
                 vec![
                     "cloud-hypervisor", "--kernel", "/path/to/kernel",
+                    "--memory", "shared=true",
                     "--fs",
                     "tag=virtiofs1,sock=/path/to/sock1,num_queues=4",
                 ],
                 r#"{
                     "kernel": {"path": "/path/to/kernel"},
+                    "memory" : { "shared": true, "size": 536870912 },
                     "fs": [
                         {"tag": "virtiofs1", "sock": "/path/to/sock1", "num_queues": 4}
                     ]
@@ -1042,11 +1055,13 @@ mod unit_tests {
             (
                 vec![
                     "cloud-hypervisor", "--kernel", "/path/to/kernel",
+                    "--memory", "shared=true",
                     "--fs",
                     "tag=virtiofs1,sock=/path/to/sock1,num_queues=4,queue_size=128"
                 ],
                 r#"{
                     "kernel": {"path": "/path/to/kernel"},
+                    "memory" : { "shared": true, "size": 536870912 },
                     "fs": [
                         {"tag": "virtiofs1", "sock": "/path/to/sock1", "num_queues": 4, "queue_size": 128}
                     ]
@@ -1056,11 +1071,13 @@ mod unit_tests {
             (
                 vec![
                     "cloud-hypervisor", "--kernel", "/path/to/kernel",
+                    "--memory", "shared=true",
                     "--fs",
                     "tag=virtiofs1,sock=/path/to/sock1,num_queues=4,queue_size=128,dax=on"
                 ],
                 r#"{
                     "kernel": {"path": "/path/to/kernel"},
+                    "memory" : { "shared": true, "size": 536870912 },
                     "fs": [
                         {"tag": "virtiofs1", "sock": "/path/to/sock1", "num_queues": 4, "queue_size": 128}
                     ]
@@ -1070,11 +1087,13 @@ mod unit_tests {
             (
                 vec![
                     "cloud-hypervisor", "--kernel", "/path/to/kernel",
+                    "--memory", "shared=true",
                     "--fs",
                     "tag=virtiofs1,sock=/path/to/sock1,num_queues=4,queue_size=128,dax=on"
                 ],
                 r#"{
                     "kernel": {"path": "/path/to/kernel"},
+                    "memory" : { "shared": true, "size": 536870912 },
                     "fs": [
                         {"tag": "virtiofs1", "sock": "/path/to/sock1", "num_queues": 4, "queue_size": 128, "dax": true}
                     ]
@@ -1084,11 +1103,13 @@ mod unit_tests {
             (
                 vec![
                     "cloud-hypervisor", "--kernel", "/path/to/kernel",
+                    "--memory", "shared=true",
                     "--fs",
                     "tag=virtiofs1,sock=/path/to/sock1,num_queues=4,queue_size=128"
                 ],
                 r#"{
                     "kernel": {"path": "/path/to/kernel"},
+                    "memory" : { "shared": true, "size": 536870912 },
                     "fs": [
                         {"tag": "virtiofs1", "sock": "/path/to/sock1", "num_queues": 4, "queue_size": 128, "dax": true}
                     ]
@@ -1098,11 +1119,13 @@ mod unit_tests {
             (
                 vec![
                     "cloud-hypervisor", "--kernel", "/path/to/kernel",
+                    "--memory", "shared=true",
                     "--fs",
                     "tag=virtiofs1,sock=/path/to/sock1,num_queues=4,queue_size=128,cache_size=8589934592"
                 ],
                 r#"{
                     "kernel": {"path": "/path/to/kernel"},
+                    "memory" : { "shared": true, "size": 536870912 },
                     "fs": [
                         {"tag": "virtiofs1", "sock": "/path/to/sock1", "num_queues": 4, "queue_size": 128}
                     ]
@@ -1112,11 +1135,13 @@ mod unit_tests {
             (
                 vec![
                     "cloud-hypervisor", "--kernel", "/path/to/kernel",
+                    "--memory", "shared=true",
                     "--fs",
                     "tag=virtiofs1,sock=/path/to/sock1,num_queues=4,queue_size=128"
                 ],
                 r#"{
                     "kernel": {"path": "/path/to/kernel"},
+                    "memory" : { "shared": true, "size": 536870912 },
                     "fs": [
                         {"tag": "virtiofs1", "sock": "/path/to/sock1", "num_queues": 4, "queue_size": 128, "cache_size": 8589934592}
                     ]
@@ -1126,11 +1151,13 @@ mod unit_tests {
             (
                 vec![
                     "cloud-hypervisor", "--kernel", "/path/to/kernel",
+                    "--memory", "shared=true",
                     "--fs",
                     "tag=virtiofs1,sock=/path/to/sock1,num_queues=4,queue_size=128,cache_size=4294967296"
                 ],
                 r#"{
                     "kernel": {"path": "/path/to/kernel"},
+                    "memory" : { "shared": true, "size": 536870912 },
                     "fs": [
                         {"tag": "virtiofs1", "sock": "/path/to/sock1", "num_queues": 4, "queue_size": 128, "cache_size": 4294967296}
                     ]
@@ -1140,11 +1167,13 @@ mod unit_tests {
             (
                 vec![
                     "cloud-hypervisor", "--kernel", "/path/to/kernel",
+                    "--memory", "shared=true",
                     "--fs",
                     "tag=virtiofs1,sock=/path/to/sock1,num_queues=4,queue_size=128,cache_size=4294967296"
                 ],
                 r#"{
                     "kernel": {"path": "/path/to/kernel"},
+                    "memory" : { "shared": true, "size": 536870912 },
                     "fs": [
                         {"tag": "virtiofs1", "sock": "/path/to/sock1", "num_queues": 4, "queue_size": 128}
                     ]
diff --git a/vmm/src/config.rs b/vmm/src/config.rs
index a24b0cfe8..10221b53d 100644
--- a/vmm/src/config.rs
+++ b/vmm/src/config.rs
@@ -84,6 +84,8 @@ pub enum ValidationError {
     CpusMaxLowerThanBoot,
     /// Both socket and path specified
     DiskSocketAndPath,
+    /// Using vhost user requires shared memory
+    VhostUserRequiresSharedMemory,
 }
 
 type ValidationResult<T> = std::result::Result<T, ValidationError>;
@@ -97,6 +99,9 @@ impl fmt::Display for ValidationError {
             ConsoleFileMissing => write!(f, "Path missing when using file console mode"),
             CpusMaxLowerThanBoot => write!(f, "Max CPUs greater than boot CPUs"),
             DiskSocketAndPath => write!(f, "Disk path and vhost socket both provided"),
+            VhostUserRequiresSharedMemory => {
+                write!(f, "Using vhost-user requires using shared memory")
+            }
         }
     }
 }
@@ -1252,6 +1257,23 @@ impl VmConfig {
                 if disk.vhost_socket.as_ref().and(disk.path.as_ref()).is_some() {
                     return Err(ValidationError::DiskSocketAndPath);
                 }
+                if disk.vhost_user && !self.memory.shared {
+                    return Err(ValidationError::VhostUserRequiresSharedMemory);
+                }
+            }
+        }
+
+        if let Some(nets) = &self.net {
+            for net in nets {
+                if net.vhost_user && !self.memory.shared {
+                    return Err(ValidationError::VhostUserRequiresSharedMemory);
+                }
+            }
+        }
+
+        if let Some(fses) = &self.fs {
+            if !fses.is_empty() && !self.memory.shared {
+                return Err(ValidationError::VhostUserRequiresSharedMemory);
             }
         }
 
@@ -1955,6 +1977,49 @@ mod tests {
         }]);
         assert!(invalid_config.validate().is_err());
 
+        let mut invalid_config = valid_config.clone();
+        invalid_config.disks = Some(vec![DiskConfig {
+            vhost_user: true,
+            ..Default::default()
+        }]);
+        assert!(invalid_config.validate().is_err());
+
+        let mut still_valid_config = valid_config.clone();
+        still_valid_config.disks = Some(vec![DiskConfig {
+            vhost_user: true,
+            ..Default::default()
+        }]);
+        still_valid_config.memory.shared = true;
+        assert!(still_valid_config.validate().is_ok());
+
+        let mut invalid_config = valid_config.clone();
+        invalid_config.net = Some(vec![NetConfig {
+            vhost_user: true,
+            ..Default::default()
+        }]);
+        assert!(invalid_config.validate().is_err());
+
+        let mut still_valid_config = valid_config.clone();
+        still_valid_config.net = Some(vec![NetConfig {
+            vhost_user: true,
+            ..Default::default()
+        }]);
+        still_valid_config.memory.shared = true;
+        assert!(still_valid_config.validate().is_ok());
+
+        let mut invalid_config = valid_config.clone();
+        invalid_config.fs = Some(vec![FsConfig {
+            ..Default::default()
+        }]);
+        assert!(invalid_config.validate().is_err());
+
+        let mut still_valid_config = valid_config.clone();
+        invalid_config.fs = Some(vec![FsConfig {
+            ..Default::default()
+        }]);
+        still_valid_config.memory.shared = true;
+        assert!(still_valid_config.validate().is_ok());
+
         Ok(())
     }
 }