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

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(a)redhat.com&gt; > --- > 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