qemu: Move 'shmem' device size validation to qemu_validate

The 'size' of a 'shmem' device is parsed and formatted as a "scaled"
value, stored in bytes, but the formatting scale is mebibytes. This
precission loss combined with the fact that the value was validated only
when starting and the size is formatted only when non-zero meant that
on first parse a value < 1 MiB would be accepted, but would be formatted
to the XML as 0 MiB as it was non-zero but truncated and a subsequent
parse would parse of such XML would parse it as 0 bytes, which in turn
would be interpreted as 'default' size.

Fix the issue by moving the validator, which ensures that the number is
a power of two and more than 1 MiB to the validator code so that it'll
be rejected at XML parsing time.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 712feb7b81c400fddf38d18b7d76835c64f8f8fb..15a0ea4081564976c6e73f3db7a0dc434afb2d0b 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9086,25 +9086,6 @@ qemuBuildShmemCommandLine(virCommand *cmd,
     g_autoptr(virJSONValue) memProps = NULL;
     g_autoptr(virJSONValue) devProps = NULL;
 
-    if (shmem->size) {
-        /*
-         * Thanks to our parsing code, we have a guarantee that the
-         * size is power of two and is at least a mebibyte in size.
-         * But because it may change in the future, the checks are
-         * doubled in here.
-         */
-        if (shmem->size & (shmem->size - 1)) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("shmem size must be a power of two"));
-            return -1;
-        }
-        if (shmem->size < 1024 * 1024) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("shmem size must be at least 1 MiB (1024 KiB)"));
-            return -1;
-        }
-    }
-
     if (shmem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                        _("only 'pci' addresses are supported for the shared memory device"));

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index b22d3618feb7fe2f66dced277cffa8d9fea2776d..01f65c0866ab4f7b75014370eaaae6bba23801ec 100644 (file)
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -5118,6 +5118,15 @@ static int
 qemuValidateDomainDeviceDefShmem(virDomainShmemDef *shmem,
                                  virQEMUCaps *qemuCaps)
 {
+    if (shmem->size > 0) {
+        if (shmem->size < 1024 * 1024 ||
+            !VIR_IS_POW2(shmem->size)) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("shmem size must be a power of 2 and at least 1 MiB (1024 KiB)"));
+            return -1;
+        }
+    }
+
     switch (shmem->model) {
     case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM:
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",

diff --git a/tests/qemuxml2argvdata/shmem-invalid-size.x86_64-latest.err b/tests/qemuxml2argvdata/shmem-invalid-size.x86_64-latest.err
index 623bd8e5dd257c1318c31d7a51ea86b460a89626..5409cb73c26531e13ae9796575f1b41a980eb7b5 100644 (file)
--- a/tests/qemuxml2argvdata/shmem-invalid-size.x86_64-latest.err
+++ b/tests/qemuxml2argvdata/shmem-invalid-size.x86_64-latest.err
@@ -1 +1 @@
-XML error: shmem size must be a power of two
+XML error: shmem size must be a power of 2 and at least 1 MiB (1024 KiB)

diff --git a/tests/qemuxml2argvdata/shmem-small-size.x86_64-latest.err b/tests/qemuxml2argvdata/shmem-small-size.x86_64-latest.err
index b5fcd8b4cfd0a90f436b7a6a4e409797cec7139e..5409cb73c26531e13ae9796575f1b41a980eb7b5 100644 (file)
--- a/tests/qemuxml2argvdata/shmem-small-size.x86_64-latest.err
+++ b/tests/qemuxml2argvdata/shmem-small-size.x86_64-latest.err
@@ -1 +1 @@
-XML error: shmem size must be at least 1 MiB (1024 KiB)
+XML error: shmem size must be a power of 2 and at least 1 MiB (1024 KiB)

diff --git a/tests/qemuxml2xmloutdata/shmem-invalid-size.x86_64-latest.xml b/tests/qemuxml2xmloutdata/shmem-invalid-size.x86_64-latest.xml
deleted file mode 100644 (file)
index cc7b63d..0000000
--- a/tests/qemuxml2xmloutdata/shmem-invalid-size.x86_64-latest.xml
+++ /dev/null
@@ -1,34 +0,0 @@
-<domain type='qemu'>
-  <name>QEMUGuest1</name>
-  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
-  <memory unit='KiB'>219136</memory>
-  <currentMemory unit='KiB'>219136</currentMemory>
-  <vcpu placement='static'>1</vcpu>
-  <os>
-    <type arch='x86_64' machine='pc'>hvm</type>
-    <boot dev='hd'/>
-  </os>
-  <cpu mode='custom' match='exact' check='none'>
-    <model fallback='forbid'>qemu64</model>
-  </cpu>
-  <clock offset='utc'/>
-  <on_poweroff>destroy</on_poweroff>
-  <on_reboot>restart</on_reboot>
-  <on_crash>destroy</on_crash>
-  <devices>
-    <emulator>/usr/bin/qemu-system-x86_64</emulator>
-    <controller type='usb' index='0' model='piix3-uhci'>
-      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
-    </controller>
-    <controller type='pci' index='0' model='pci-root'/>
-    <input type='mouse' bus='ps2'/>
-    <input type='keyboard' bus='ps2'/>
-    <audio id='1' type='none'/>
-    <memballoon model='none'/>
-    <shmem name='shmem0'>
-      <model type='ivshmem-plain'/>
-      <size unit='M'>12</size>
-      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
-    </shmem>
-  </devices>
-</domain>

diff --git a/tests/qemuxml2xmloutdata/shmem-small-size.x86_64-latest.xml b/tests/qemuxml2xmloutdata/shmem-small-size.x86_64-latest.xml
deleted file mode 100644 (file)
index fc6f9dd..0000000
--- a/tests/qemuxml2xmloutdata/shmem-small-size.x86_64-latest.xml
+++ /dev/null
@@ -1,34 +0,0 @@
-<domain type='qemu'>
-  <name>QEMUGuest1</name>
-  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
-  <memory unit='KiB'>219136</memory>
-  <currentMemory unit='KiB'>219136</currentMemory>
-  <vcpu placement='static'>1</vcpu>
-  <os>
-    <type arch='x86_64' machine='pc'>hvm</type>
-    <boot dev='hd'/>
-  </os>
-  <cpu mode='custom' match='exact' check='none'>
-    <model fallback='forbid'>qemu64</model>
-  </cpu>
-  <clock offset='utc'/>
-  <on_poweroff>destroy</on_poweroff>
-  <on_reboot>restart</on_reboot>
-  <on_crash>destroy</on_crash>
-  <devices>
-    <emulator>/usr/bin/qemu-system-x86_64</emulator>
-    <controller type='usb' index='0' model='piix3-uhci'>
-      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
-    </controller>
-    <controller type='pci' index='0' model='pci-root'/>
-    <input type='mouse' bus='ps2'/>
-    <input type='keyboard' bus='ps2'/>
-    <audio id='1' type='none'/>
-    <memballoon model='none'/>
-    <shmem name='shmem0'>
-      <model type='ivshmem-plain'/>
-      <size unit='M'>0</size>
-      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
-    </shmem>
-  </devices>
-</domain>

diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c
index be1d9ad38eaceadb7a3c7e75412fc27d0dcecb76..23ecae63a4a544cb5daaec79ae1deb689f5c2c80 100644 (file)
--- a/tests/qemuxmlconftest.c
+++ b/tests/qemuxmlconftest.c
@@ -2456,9 +2456,9 @@ mymain(void)
     DO_TEST_CAPS_ARCH_LATEST_FULL("fips-enabled", "x86_64", ARG_FLAGS, FLAG_FIPS_HOST);
 
     DO_TEST_CAPS_LATEST("shmem-plain-doorbell");
-    DO_TEST_CAPS_LATEST_FAILURE("shmem-invalid-size");
+    DO_TEST_CAPS_LATEST_PARSE_ERROR("shmem-invalid-size");
     DO_TEST_CAPS_LATEST_FAILURE("shmem-invalid-address");
-    DO_TEST_CAPS_LATEST_FAILURE("shmem-small-size");
+    DO_TEST_CAPS_LATEST_PARSE_ERROR("shmem-small-size");
     DO_TEST_CAPS_LATEST_PARSE_ERROR("shmem-msi-only");
     DO_TEST_CAPS_LATEST("cpu-host-passthrough-features");