[RFC] qemu: virtiofs can be used without NUMA nodes

Marc Hartmayer posted 1 patch 3 years, 6 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20201006162058.44596-1-mhartmay@linux.ibm.com
src/qemu/qemu_validate.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
[RFC] qemu: virtiofs can be used without NUMA nodes
Posted by Marc Hartmayer 3 years, 6 months ago
...if a machine memory-backend using shared memory is configured for
the guest. This is especially important for QEMU machine types that
don't have NUMA but virtiofs support.

An example snippet:

  <domain type='kvm'>
    <name>test</name>
    <memory unit='KiB'>2097152</memory>
    <memoryBacking>
      <access mode='shared'/>
    </memoryBacking>
    <devices>
      <filesystem type='mount' accessmode='passthrough'>
	<driver type='virtiofs'/>
	<source dir='/tmp/test'/>
	<target dir='coffee'/>
      </filesystem>
      ...
    </devices>
    ...
  </domain>

and the corresponding QEMU command line:

  /usr/bin/qemu-system-s390x \
  -machine s390-ccw-virtio-5.2,memory-backend=s390.ram \
  -m 2048 \
  -object
  memory-backend-file,id=s390.ram,mem-path=/var/lib/libvirt/qemu/ram/46-test/s390.ram,share=yes,size=2147483648 \
  ...

Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
---
Note: There are still some TODOs left... e.g. adapt the virtiofs
documentation of libvirt.
---
 src/qemu/qemu_validate.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index a212605579d2..077a85b30802 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -3470,14 +3470,21 @@ qemuValidateDomainDeviceDefGraphics(const virDomainGraphicsDef *graphics,
 
 
 static int
-qemuValidateDomainDefVirtioFSSharedMemory(const virDomainDef *def)
+qemuValidateDomainDefVirtioFSSharedMemory(const virDomainDef *def,
+                                          virQEMUCapsPtr qemuCaps)
 {
+    const char *defaultRAMId = virQEMUCapsGetMachineDefaultRAMid(qemuCaps,
+                                                                 def->virtType,
+                                                                 def->os.machine);
     size_t numa_nodes = virDomainNumaGetNodeCount(def->numa);
     size_t i;
 
-    if (numa_nodes == 0) {
+    if (numa_nodes == 0 &&
+        !(defaultRAMId && def->mem.access == VIR_DOMAIN_MEMORY_ACCESS_SHARED)) {
+        /* TODO do we need further checks here (e.g. check whether
+         * memory backend is supported by the QEMU binary)? */
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("virtiofs requires one or more NUMA nodes"));
+                       _("virtiofs requires shared memory"));
         return -1;
     }
 
@@ -3591,7 +3598,7 @@ qemuValidateDomainDeviceDefFS(virDomainFSDefPtr fs,
                            _("virtiofs does not support multidevs"));
             return -1;
         }
-        if (qemuValidateDomainDefVirtioFSSharedMemory(def) < 0)
+        if (qemuValidateDomainDefVirtioFSSharedMemory(def, qemuCaps) < 0)
             return -1;
         break;
 
-- 
2.25.4

Re: [RFC] qemu: virtiofs can be used without NUMA nodes
Posted by Marc Hartmayer 3 years, 6 months ago
On Tue, Oct 06, 2020 at 06:20 PM +0200, Marc Hartmayer <mhartmay@linux.ibm.com> wrote:
> ...if a machine memory-backend using shared memory is configured for
> the guest. This is especially important for QEMU machine types that
> don't have NUMA but virtiofs support.
>
> An example snippet:
>
>   <domain type='kvm'>
>     <name>test</name>
>     <memory unit='KiB'>2097152</memory>
>     <memoryBacking>
>       <access mode='shared'/>
>     </memoryBacking>
>     <devices>
>       <filesystem type='mount' accessmode='passthrough'>
> 	<driver type='virtiofs'/>
> 	<source dir='/tmp/test'/>
> 	<target dir='coffee'/>
>       </filesystem>
>       ...
>     </devices>
>     ...
>   </domain>
>
> and the corresponding QEMU command line:
>
>   /usr/bin/qemu-system-s390x \
>   -machine s390-ccw-virtio-5.2,memory-backend=s390.ram \
>   -m 2048 \
>   -object
>   memory-backend-file,id=s390.ram,mem-path=/var/lib/libvirt/qemu/ram/46-test/s390.ram,share=yes,size=2147483648 \
>   ...
>
> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
> ---
> Note: There are still some TODOs left... e.g. adapt the virtiofs
> documentation of libvirt.
> ---
>  src/qemu/qemu_validate.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index a212605579d2..077a85b30802 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -3470,14 +3470,21 @@ qemuValidateDomainDeviceDefGraphics(const virDomainGraphicsDef *graphics,
>  
>  
>  static int
> -qemuValidateDomainDefVirtioFSSharedMemory(const virDomainDef *def)
> +qemuValidateDomainDefVirtioFSSharedMemory(const virDomainDef *def,
> +                                          virQEMUCapsPtr qemuCaps)
>  {
> +    const char *defaultRAMId = virQEMUCapsGetMachineDefaultRAMid(qemuCaps,
> +                                                                 def->virtType,
> +                                                                 def->os.machine);
>      size_t numa_nodes = virDomainNumaGetNodeCount(def->numa);
>      size_t i;
>  
> -    if (numa_nodes == 0) {
> +    if (numa_nodes == 0 &&
> +        !(defaultRAMId && def->mem.access == VIR_DOMAIN_MEMORY_ACCESS_SHARED)) {
> +        /* TODO do we need further checks here (e.g. check whether
> +         * memory backend is supported by the QEMU binary)? */
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("virtiofs requires one or more NUMA nodes"));
> +                       _("virtiofs requires shared memory"));
>          return -1;
>      }
>  
> @@ -3591,7 +3598,7 @@ qemuValidateDomainDeviceDefFS(virDomainFSDefPtr fs,
>                             _("virtiofs does not support multidevs"));
>              return -1;
>          }
> -        if (qemuValidateDomainDefVirtioFSSharedMemory(def) < 0)
> +        if (qemuValidateDomainDefVirtioFSSharedMemory(def, qemuCaps) < 0)
>              return -1;
>          break;
>  
> -- 
> 2.25.4
>

Gentle ping :)

-- 
Kind regards / Beste Grüße
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen 
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


Re: [RFC] qemu: virtiofs can be used without NUMA nodes
Posted by Michal Privoznik 3 years, 6 months ago
On 10/6/20 6:20 PM, Marc Hartmayer wrote:
> ...if a machine memory-backend using shared memory is configured for
> the guest. This is especially important for QEMU machine types that
> don't have NUMA but virtiofs support.
> 
> An example snippet:
> 
>    <domain type='kvm'>
>      <name>test</name>
>      <memory unit='KiB'>2097152</memory>
>      <memoryBacking>
>        <access mode='shared'/>
>      </memoryBacking>
>      <devices>
>        <filesystem type='mount' accessmode='passthrough'>
> 	<driver type='virtiofs'/>
> 	<source dir='/tmp/test'/>
> 	<target dir='coffee'/>
>        </filesystem>
>        ...
>      </devices>
>      ...
>    </domain>
> 
> and the corresponding QEMU command line:
> 
>    /usr/bin/qemu-system-s390x \
>    -machine s390-ccw-virtio-5.2,memory-backend=s390.ram \
>    -m 2048 \
>    -object
>    memory-backend-file,id=s390.ram,mem-path=/var/lib/libvirt/qemu/ram/46-test/s390.ram,share=yes,size=2147483648 \
>    ...
> 
> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
> ---
> Note: There are still some TODOs left... e.g. adapt the virtiofs
> documentation of libvirt.

Yep, but looks good.

> ---
>   src/qemu/qemu_validate.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index a212605579d2..077a85b30802 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -3470,14 +3470,21 @@ qemuValidateDomainDeviceDefGraphics(const virDomainGraphicsDef *graphics,
>   
>   
>   static int
> -qemuValidateDomainDefVirtioFSSharedMemory(const virDomainDef *def)
> +qemuValidateDomainDefVirtioFSSharedMemory(const virDomainDef *def,
> +                                          virQEMUCapsPtr qemuCaps)
>   {
> +    const char *defaultRAMId = virQEMUCapsGetMachineDefaultRAMid(qemuCaps,
> +                                                                 def->virtType,
> +                                                                 def->os.machine);
>       size_t numa_nodes = virDomainNumaGetNodeCount(def->numa);
>       size_t i;
>   
> -    if (numa_nodes == 0) {
> +    if (numa_nodes == 0 &&
> +        !(defaultRAMId && def->mem.access == VIR_DOMAIN_MEMORY_ACCESS_SHARED)) {
> +        /* TODO do we need further checks here (e.g. check whether
> +         * memory backend is supported by the QEMU binary)? */

I don't think we need that. memory backends can't be compiled out (well, 
unless a distro has a patch on the top of qemu which would do exactly 
that), can defaultRAMId exposed is strictly newer than memory backends.

I think this comment can be removed and the rest can be kept as is (plus 
  the docs).

Michal