[libvirt PATCH 1/2] qemu: relax shared memory check for vhostuser daemons

Ján Tomko posted 2 patches 1 year, 4 months ago
[libvirt PATCH 1/2] qemu: relax shared memory check for vhostuser daemons
Posted by Ján Tomko 1 year, 4 months ago
For some vhostuser daemons, we validate that the guest memory is shared
with the host.

With earlier versions of QEMU, it was only possible to mark memory
as shared by defining an explicit NUMA topology.  Later, QEMU exposed
the name of the default memory backend (defaultRAMid) so we can mark
that memory as shared.

Since libvirt commit:
  commit bff2ad5d6b1f25da02802273934d2a519159fec7
    qemu: Relax validation for mem->access if guest has no NUMA
we already check for the case when user requests shared memory,
but QEMU did not expose defaultRAMid.

Drop the duplicit check from vhostuser device validation, to make
it pass on hotplug even after libvirtd restart.

This avoids the need to store the defaultRAMid, since we don't really
need it for anything after the VM has been already started.

https://bugzilla.redhat.com/show_bug.cgi?id=2078693
https://bugzilla.redhat.com/show_bug.cgi?id=2177701

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 src/qemu/qemu_validate.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 0146220d8f..f750684414 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1782,7 +1782,7 @@ qemuValidateNetSupportsCoalesce(virDomainNetType type)
 static int
 qemuValidateDomainDefVhostUserRequireSharedMemory(const virDomainDef *def,
                                                   const char *name,
-                                                  virQEMUCaps *qemuCaps)
+                                                  virQEMUCaps *qemuCaps G_GNUC_UNUSED)
 {
     const char *defaultRAMId = virQEMUCapsGetMachineDefaultRAMid(qemuCaps,
                                                                  def->virtType,
@@ -1790,8 +1790,7 @@ qemuValidateDomainDefVhostUserRequireSharedMemory(const virDomainDef *def,
     size_t numa_nodes = virDomainNumaGetNodeCount(def->numa);
     size_t i;
 
-    if (numa_nodes == 0 &&
-        !(defaultRAMId && def->mem.access == VIR_DOMAIN_MEMORY_ACCESS_SHARED)) {
+    if (numa_nodes == 0 && def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_SHARED) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("'%s' requires shared memory"), name);
         return -1;
-- 
2.39.2

Re: [libvirt PATCH 1/2] qemu: relax shared memory check for vhostuser daemons
Posted by Michal Prívozník 1 year, 4 months ago
On 3/13/23 15:09, Ján Tomko wrote:
> For some vhostuser daemons, we validate that the guest memory is shared
> with the host.
> 
> With earlier versions of QEMU, it was only possible to mark memory
> as shared by defining an explicit NUMA topology.  Later, QEMU exposed
> the name of the default memory backend (defaultRAMid) so we can mark
> that memory as shared.
> 
> Since libvirt commit:
>   commit bff2ad5d6b1f25da02802273934d2a519159fec7
>     qemu: Relax validation for mem->access if guest has no NUMA
> we already check for the case when user requests shared memory,
> but QEMU did not expose defaultRAMid.
> 
> Drop the duplicit check from vhostuser device validation, to make
> it pass on hotplug even after libvirtd restart.
> 
> This avoids the need to store the defaultRAMid, since we don't really
> need it for anything after the VM has been already started.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=2078693
> https://bugzilla.redhat.com/show_bug.cgi?id=2177701
> 
> Signed-off-by: Ján Tomko <jtomko@redhat.com>
> ---
>  src/qemu/qemu_validate.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 0146220d8f..f750684414 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -1782,7 +1782,7 @@ qemuValidateNetSupportsCoalesce(virDomainNetType type)
>  static int
>  qemuValidateDomainDefVhostUserRequireSharedMemory(const virDomainDef *def,
>                                                    const char *name,
> -                                                  virQEMUCaps *qemuCaps)
> +                                                  virQEMUCaps *qemuCaps G_GNUC_UNUSED)
>  {
>      const char *defaultRAMId = virQEMUCapsGetMachineDefaultRAMid(qemuCaps,
>                                                                   def->virtType,

My GCC spots that this ^^^ is an unused variable now. In fact, either
of: 1) marking the @qemuCaps argument G_GNUC_UNUSED, or 2) not removing
this variable is wrong;

> @@ -1790,8 +1790,7 @@ qemuValidateDomainDefVhostUserRequireSharedMemory(const virDomainDef *def,
>      size_t numa_nodes = virDomainNumaGetNodeCount(def->numa);
>      size_t i;
>  
> -    if (numa_nodes == 0 &&
> -        !(defaultRAMId && def->mem.access == VIR_DOMAIN_MEMORY_ACCESS_SHARED)) {
> +    if (numa_nodes == 0 && def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_SHARED) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                         _("'%s' requires shared memory"), name);
>          return -1;

Michal