vmm, tests: Make specifying a size optional for virtio-pmem

If a size is specified use it (in particular this is required if the
destination is a directory) otherwise seek in the file to get the size
of the file.

Add a new check that the size is a multiple of 2MiB otherwise the kernel
will reject it.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>

tests/integration.rs

@@ -1987,15 +1987,20 @@ mod tests {
 
     #[cfg_attr(not(feature = "mmio"), test)]
     fn test_virtio_pmem_persist_writes() {
-        test_virtio_pmem(false)
+        test_virtio_pmem(false, false)
     }
 
     #[cfg_attr(not(feature = "mmio"), test)]
     fn test_virtio_pmem_discard_writes() {
-        test_virtio_pmem(true)
+        test_virtio_pmem(true, false)
     }
 
-    fn test_virtio_pmem(discard_writes: bool) {
+    #[cfg_attr(not(feature = "mmio"), test)]
+    fn test_virtio_pmem_with_size() {
+        test_virtio_pmem(true, true)
+    }
+
+    fn test_virtio_pmem(discard_writes: bool, specify_size: bool) {
         test_block!(tb, "", {
             let mut clear = ClearDiskConfig::new();
             let guest = Guest::new(&mut clear);
@@ -2020,9 +2025,9 @@ mod tests {
                 .args(&[
                     "--pmem",
                     format!(
-                        "file={},size={}{}",
+                        "file={}{}{}",
                         pmem_temp_file.path().to_str().unwrap(),
-                        "128M",
+                        if specify_size { ",size=128M" } else { "" },
                         if discard_writes {
                             ",discard_writes=on"
                         } else {
@@ -3836,7 +3841,7 @@ mod tests {
                     &api_socket,
                     "add-pmem",
                     Some(&format!(
-                        "file={},size=128M,id=test0",
+                        "file={},id=test0",
                         pmem_temp_file.path().to_str().unwrap()
                     ))
                 )

vmm/src/api/openapi/cloud-hypervisor.yaml

@@ -514,7 +514,6 @@ components:
     PmemConfig:
       required:
       - file
-      - size
       type: object
       properties:
         file:

vmm/src/config.rs

@@ -34,8 +34,6 @@ pub enum Error {
     InvalidCacheSizeWithDaxOff,
     /// Missing persistant memory file parameter.
     ParsePmemFileMissing,
-    /// Missing persistant memory size parameter.
-    ParsePmemSizeMissing,
     /// Missing vsock socket path parameter.
     ParseVsockSockMissing,
     /// Missing vsock cid parameter.
@@ -126,8 +124,6 @@ impl fmt::Display for Error {
             }
             ParsePersistentMemory(o) => write!(f, "Error parsing --pmem: {}", o),
             ParsePmemFileMissing => write!(f, "Error parsing --pmem: file missing"),
-            ParsePmemSizeMissing => write!(f, "Error parsing --pmem: size missing"),
-
             ParseVsock(o) => write!(f, "Error parsing --vsock: {}", o),
             ParseVsockCidMissing => write!(f, "Error parsing --vsock: cid missing"),
             ParseVsockSockMissing => write!(f, "Error parsing --vsock: sock missing"),
@@ -959,7 +955,8 @@ impl FsConfig {
 #[derive(Clone, Debug, PartialEq, Deserialize, Serialize, Default)]
 pub struct PmemConfig {
     pub file: PathBuf,
-    pub size: u64,
+    #[serde(default)]
+    pub size: Option<u64>,
     #[serde(default)]
     pub iommu: bool,
     #[serde(default)]
@@ -988,9 +985,8 @@ impl PmemConfig {
         let file = PathBuf::from(parser.get("file").ok_or(Error::ParsePmemFileMissing)?);
         let size = parser
             .convert::<ByteSized>("size")
-            .map_err(Error::ParseMemory)?
-            .ok_or(Error::ParsePmemSizeMissing)?
-            .0;
+            .map_err(Error::ParsePersistentMemory)?
+            .map(|v| v.0);
         let mergeable = parser
             .convert::<Toggle>("mergeable")
             .map_err(Error::ParsePersistentMemory)?
@@ -1756,13 +1752,12 @@ mod tests {
     fn test_pmem_parsing() -> Result<()> {
         // Must always give a file and size
         assert!(PmemConfig::parse("").is_err());
-        assert!(PmemConfig::parse("file=/tmp/pmem").is_err());
         assert!(PmemConfig::parse("size=128M").is_err());
         assert_eq!(
             PmemConfig::parse("file=/tmp/pmem,size=128M")?,
             PmemConfig {
                 file: PathBuf::from("/tmp/pmem"),
-                size: 128 << 20,
+                size: Some(128 << 20),
                 ..Default::default()
             }
         );
@@ -1770,7 +1765,7 @@ mod tests {
             PmemConfig::parse("file=/tmp/pmem,size=128M,id=mypmem0")?,
             PmemConfig {
                 file: PathBuf::from("/tmp/pmem"),
-                size: 128 << 20,
+                size: Some(128 << 20),
                 id: Some("mypmem0".to_owned()),
                 ..Default::default()
             }
@@ -1779,7 +1774,7 @@ mod tests {
             PmemConfig::parse("file=/tmp/pmem,size=128M,iommu=on,mergeable=on,discard_writes=on")?,
             PmemConfig {
                 file: PathBuf::from("/tmp/pmem"),
-                size: 128 << 20,
+                size: Some(128 << 20),
                 mergeable: true,
                 discard_writes: true,
                 iommu: true,

vmm/src/device_manager.rs

@@ -39,7 +39,7 @@ use qcow::{self, ImageType, QcowFile};
 use std::any::Any;
 use std::collections::HashMap;
 use std::fs::{File, OpenOptions};
-use std::io::{self, sink, stdout};
+use std::io::{self, sink, stdout, Seek, SeekFrom};
 #[cfg(feature = "pci_support")]
 use std::num::Wrapping;
 use std::os::unix::fs::OpenOptionsExt;
@@ -285,6 +285,12 @@ pub enum DeviceManagerError {
     /// Failed updating guest memory for VFIO PCI device.
     #[cfg(feature = "pci_support")]
     UpdateMemoryForVfioPciDevice(VfioPciError),
+
+    /// Trying to use a directory for pmem but no size specified
+    PmemWithDirectorySizeMissing,
+
+    /// Trying to use a size that is not multiple of 2MiB
+    PmemSizeNotAligned,
 }
 pub type DeviceManagerResult<T> = result::Result<T, DeviceManagerError>;
 
@@ -1522,7 +1528,36 @@ impl DeviceManager {
             pmem_cfg.id = self.next_device_name(PMEM_DEVICE_NAME_PREFIX)?;
         }
 
-        let size = pmem_cfg.size;
+        let (custom_flags, set_len) = if pmem_cfg.file.is_dir() {
+            if pmem_cfg.size.is_none() {
+                return Err(DeviceManagerError::PmemWithDirectorySizeMissing);
+            }
+            (O_TMPFILE, true)
+        } else {
+            (0, false)
+        };
+
+        let mut file = OpenOptions::new()
+            .read(true)
+            .write(!pmem_cfg.discard_writes)
+            .custom_flags(custom_flags)
+            .open(&pmem_cfg.file)
+            .map_err(DeviceManagerError::PmemFileOpen)?;
+
+        let size = if let Some(size) = pmem_cfg.size {
+            if set_len {
+                file.set_len(size)
+                    .map_err(DeviceManagerError::PmemFileSetLen)?;
+            }
+            size
+        } else {
+            file.seek(SeekFrom::End(0))
+                .map_err(DeviceManagerError::PmemFileSetLen)?
+        };
+
+        if size % 0x20_0000 != 0 {
+            return Err(DeviceManagerError::PmemSizeNotAligned);
+        }
 
         // The memory needs to be 2MiB aligned in order to support
         // hugepages.
@@ -1534,24 +1569,6 @@ impl DeviceManager {
             .allocate_mmio_addresses(None, size as GuestUsize, Some(0x0020_0000))
             .ok_or(DeviceManagerError::PmemRangeAllocation)?;
 
-        let (custom_flags, set_len) = if pmem_cfg.file.is_dir() {
-            (O_TMPFILE, true)
-        } else {
-            (0, false)
-        };
-
-        let file = OpenOptions::new()
-            .read(true)
-            .write(!pmem_cfg.discard_writes)
-            .custom_flags(custom_flags)
-            .open(&pmem_cfg.file)
-            .map_err(DeviceManagerError::PmemFileOpen)?;
-
-        if set_len {
-            file.set_len(size)
-                .map_err(DeviceManagerError::PmemFileSetLen)?;
-        }
-
         let cloned_file = file.try_clone().map_err(DeviceManagerError::CloneFile)?;
         let mmap_region = MmapRegion::build(
             Some(FileOffset::new(cloned_file, 0)),