Re: [libvirt] [PATCH v4 14/20] qemu: add vhost-user-gpu helper unit

Hi On Sat, Sep 21, 2019 at 1:05 AM Cole Robinson <crobinso(a)redhat.com&gt; wrote: > > On 9/13/19 8:50 AM, marcandre.lureau(a)redhat.com wrote: > > From: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; > > > > Similar to the qemu_tpm.c, add a unit with a few functions to > > start/stop and setup the cgroup of the external vhost-user-gpu > > process. See function documentation. > > > > The vhost-user connection fd is set on qemuDomainVideoPrivate struct. > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; > > --- > > src/qemu/Makefile.inc.am | 2 + > > src/qemu/qemu_domain.c | 5 +- > > src/qemu/qemu_domain.h | 2 +- > > src/qemu/qemu_vhost_user_gpu.c | 276 +++++++++++++++++++++++++++++++++ > > src/qemu/qemu_vhost_user_gpu.h | 49 ++++++ > > 5 files changed, 332 insertions(+), 2 deletions(-) > > create mode 100644 src/qemu/qemu_vhost_user_gpu.c > > create mode 100644 src/qemu/qemu_vhost_user_gpu.h > >

> > + virCommandClearCaps(cmd); > > + virCommandSetPidFile(cmd, pidfile); > > + virCommandDaemonize(cmd); > > + > > + if (qemuExtDeviceLogCommand(driver, vm, cmd, "vhost-user-gpu") < 0) > > + goto error; > > + > > Most, possibly all, of these 'goto error;' usages are overwriting > detailed error messages with 'vhost-user-gpu failed to start'. So this > needs to be reworked. You can use virGetLastErrorMessage if you wanted > to prepend the vhost-user specific bit to the error. Maybe break up this > function into chunks, like one to build the Command object It doesn't "overwrite" the detailed error, it merely adds another virReportError(). Or am I missing something?

I'll leave that for now.