[libvirt] [PATCH] qemu: Warn when using vhost-user without shared memory
Martin Kletzander
mkletzan at redhat.com
Tue Dec 15 14:07:46 UTC 2015
On Tue, Dec 15, 2015 at 06:44:25AM -0500, John Ferlan wrote:
>
>
>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
Yes, exactly, thanks for noticing, this should be the diff:
diff --git i/src/qemu/qemu_process.c w/src/qemu/qemu_process.c
index ba8dfebd1357..f2740687f655 100644
--- i/src/qemu/qemu_process.c
+++ w/src/qemu/qemu_process.c
@@ -4781,9 +4781,10 @@ qemuProcessLaunch(virConnectPtr conn,
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)
+ VIR_NUMA_MEM_ACCESS_SHARED) {
shmem = true;
- break;
+ break;
+ }
}
}
--
I will push it later on if you agree.
Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151215/a38c0561/attachment-0001.sig>
More information about the libvir-list
mailing list