[libvirt PATCHv4 09/15] qemu: validate virtiofs filesystems

[Ján Tomko]

On Wed, Feb 26, 2020 at 09:15:17AM +0100, Michal Privoznik wrote:
>On 2/26/20 8:33 AM, Peter Krempa wrote:
>>On Thu, Feb 20, 2020 at 15:32:46 +0100, Ján Tomko wrote:
>>>Reject unsupported configurations.
>>>
>>>Signed-off-by: Ján Tomko <jtomko at redhat.com>
>>>---
>>>  src/qemu/qemu_domain.c | 61 +++++++++++++++++++++++++++++++++++++++---
>>>  1 file changed, 58 insertions(+), 3 deletions(-)
>>>
>>>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>>>index c3fc3fed1c..7cb283123d 100644
>>>--- a/src/qemu/qemu_domain.c
>>>+++ b/src/qemu/qemu_domain.c
>>>@@ -8266,10 +8266,44 @@ qemuDomainDeviceDefValidateIOMMU(const virDomainIOMMUDef *iommu,
>>>      return 0;
>>>  }
>>>+static int
>>>+qemuDomainDefValidateVirtioFSSharedMemory(const virDomainDef *def)
>>>+{
>>>+    size_t numa_nodes = virDomainNumaGetNodeCount(def->numa);
>>>+    size_t i;
>>>+
>>>+    for (i = 0; i < numa_nodes; i++) {
>>
>>This won't catch guests with no numa configured ...
>>

Consider this squashed in:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 090720fd76..f992e3fc6e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8276,6 +8276,12 @@ qemuDomainDefValidateVirtioFSSharedMemory(const virDomainDef *def)
      size_t numa_nodes = virDomainNumaGetNodeCount(def->numa);
      size_t i;

+    if (numa_nodes == 0) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("virtiofs requires one or more NUMA nodes"));
+        return -1;
+    }
+
      for (i = 0; i < numa_nodes; i++) {
          virDomainMemoryAccess node_access =
              virDomainNumaGetNodeMemoryAccessMode(def->numa, i);


>>>+        virDomainMemoryAccess node_access =
>>>+            virDomainNumaGetNodeMemoryAccessMode(def->numa, i);
>>>+
>>>+        switch (node_access) {
>>>+        case VIR_DOMAIN_MEMORY_ACCESS_DEFAULT:
>>>+            if (def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_SHARED) {
>>>+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>>+                               _("virtiofs requires shared memory"));
>>
>>... so this error won't be reported.
>>
>>Also must all nodes have shared memory? Isn't one enough?
>
>Maybe it is. But I wouldn't burn our resources trying to figure out 
>all the corner cases and make this work in them. I'd say that we can 
>start with this check and if somebody ever wants us to refine it, we 
>can look into our options.

Yes. Checking all nodes is the prudent choice that ensures all of the memory
will be shared.

Jano


>
>Michal
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200226/beedd240/attachment-0001.sig>