Re: [libvirt] [PATCH v2 09/16] qemu: add vhost-user helpers

Hi On Tue, Aug 27, 2019 at 12:29 PM Ján Tomko <jtomko(a)redhat.com&gt; wrote: > > On Fri, Aug 23, 2019 at 12:21:53PM -0400, Cole Robinson wrote: > >From: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; > > > >Add qemuVhostUserFetchConfigs() to discover vhost-user helpers. > > > >qemuVhostUserFillDomainGPU() will find the first matching GPU helper > >with the required capabilities and set the associated > >vhost_user_binary. > > > > Looks good, but it seems like something we might want to start caching > over time, especially if more than one helper per domain will be needed. > > >Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; > >Signed-off-by: Cole Robinson <crobinso(a)redhat.com&gt; > >--- > > src/conf/device_conf.c | 1 + > > src/conf/device_conf.h | 1 + > > src/qemu/Makefile.inc.am | 2 + > > src/qemu/qemu_vhost_user.c | 394 ++++++++++++++++++ > > src/qemu/qemu_vhost_user.h | 48 +++ > > tests/Makefile.am | 9 + > > .../etc/qemu/vhost-user/40-gpu.json | 1 + > > .../etc/qemu/vhost-user/50-gpu.json | 0 > > .../qemu/vhost-user/test-vhost-user-gpu | 11 + > > .../usr/share/qemu/vhost-user/30-gpu.json | 1 + > > .../usr/share/qemu/vhost-user/50-gpu.json | 8 + > > .../usr/share/qemu/vhost-user/60-gpu.json | 1 + > > tests/qemuvhostusertest.c | 132 ++++++ > > 13 files changed, 609 insertions(+) > > create mode 100644 src/qemu/qemu_vhost_user.c > > create mode 100644 src/qemu/qemu_vhost_user.h > > create mode 120000 tests/qemuvhostuserdata/etc/qemu/vhost-user/40-gpu.json > > create mode 100644 tests/qemuvhostuserdata/etc/qemu/vhost-user/50-gpu.json > > create mode 100755 tests/qemuvhostuserdata/usr/libexec/qemu/vhost-user/test-vhost-user-gpu > > create mode 120000 tests/qemuvhostuserdata/usr/share/qemu/vhost-user/30-gpu.json > > create mode 100644 tests/qemuvhostuserdata/usr/share/qemu/vhost-user/50-gpu.json > > create mode 120000 tests/qemuvhostuserdata/usr/share/qemu/vhost-user/60-gpu.json > > create mode 100644 tests/qemuvhostusertest.c > > > >diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c > >index 4c57f0995f..2f7077ddc4 100644 > >--- a/src/conf/device_conf.c > >+++ b/src/conf/device_conf.c > >@@ -96,6 +96,7 @@ virDomainDeviceInfoClear(virDomainDeviceInfoPtr info) > > VIR_FREE(info->loadparm); > > info->isolationGroup = 0; > > info->isolationGroupLocked = false; > >+ VIR_FREE(info->vhost_user_binary); > > } > > > > void > >diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h > >index d0854925e3..0b0c525ad8 100644 > >--- a/src/conf/device_conf.h > >+++ b/src/conf/device_conf.h > >@@ -179,6 +179,7 @@ struct _virDomainDeviceInfo { > > * cases we might want to prevent that from happening by > > * locking the isolation group */ > > bool isolationGroupLocked; > >+ char *vhost_user_binary; > > }; > > > > Not sure about the placement in DeviceInfo, looks more fit for 'driver' > to me. > That wouldn't follow firmware code, but other than that, driver doesn't really fit either since it's a per-domain/device value.