[libvirt] [PATCH] qemu: Warn when using vhost-user without shared memory

[John Ferlan]

On 12/08/2015 10:24 AM, Martin Kletzander wrote:
> When user configures vhost-user interface and forgets to also configure
> any shared memory, the search for the root cause of non-operational
> interface might take unpleasantly long time.  Let's enhance user
> experience by emitting a warning in the logs.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1266982
> 
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
>  src/qemu/qemu_process.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 420196264685..fb471342e790 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4542,6 +4542,7 @@ qemuProcessLaunch(virConnectPtr conn,
>      unsigned int hostdev_flags = 0;
>      size_t nnicindexes = 0;
>      int *nicindexes = NULL;
> +    bool check_shmem = false;
> 
>      VIR_DEBUG("vm=%p name=%s id=%d asyncJob=%d "
>                "incoming.launchURI=%s incoming.deferredURI=%s "
> @@ -4749,6 +4750,49 @@ qemuProcessLaunch(virConnectPtr conn,
>              goto cleanup;
>      }
> 
> +    VIR_DEBUG("Checking for any possible (non-fatal) issues");
> +
> +    /*
> +     * For vhost-user to work, the domain has to have some type of
> +     * shared memory configured.  We're not the proper once to judge
> +     * whether shared hugepages or shm are enough and will be in the
> +     * future, so we'll just warn in case there is none of that
> +     * configured.  Moreover failing would give the false illusion
> +     * that libvirt is really checking that everything works before
> +     * running the domain and not only we are unable to do that, but
> +     * it's also not our aim to do so.
> +     */
> +    for (i = 0; i < vm->def->nnets; i++) {
> +        if (virDomainNetGetActualType(vm->def->nets[i]) ==
> +                                      VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
> +            check_shmem = true;
> +            break;
> +        }
> +    }
> +
> +    if (check_shmem) {
> +        bool shmem = vm->def->nshmems;
> +
> +        /*
> +         * This check is by no means complete.  We merely check
> +         * whetere there are *some* hugepages enabled and *some* NUMA
> +         * nodes with shared memory access.
> +         */
> +        if (!shmem && vm->def->mem.nhugepages) {
> +            for (i = 0; i < virDomainNumaGetNodeCount(vm->def->numa); i++) {
> +                if (virDomainNumaGetNodeMemoryAccessMode(vm->def->numa, i) ==
> +                    VIR_NUMA_MEM_ACCESS_SHARED)
> +                    shmem = true;
> +                break;

Coverity complains here that i++ is not reachable.  I think you meant to
put the break; inside the if, right?

John
> +            }
> +        }
> +
> +        if (!shmem) {
> +            VIR_WARN("Detected vhost-user interface without any shared memory. "
> +                     "The interface might not be operational");
> +        }
> +    }
> +
>      VIR_DEBUG("Building emulator command line");
>      if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig,
>                                       priv->monJSON, priv->qemuCaps,
>