Re: [libvirt] [PATCH v2 14/16] qemu: build vhost-user GPU devices

Hi On Tue, Aug 27, 2019 at 1:17 PM Ján Tomko <jtomko(a)redhat.com&gt; wrote: > > On Fri, Aug 23, 2019 at 12:21:58PM -0400, Cole Robinson wrote: >> From: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; >> >> For each vhost-user GPUs, >> - build a socket chardev, and pass the vhost-user socket to it >> - build a vhost-user video device and associate it with the chardev >> >> Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; >> Signed-off-by: Cole Robinson <crobinso(a)redhat.com&gt; >> --- >> src/qemu/qemu_command.c | 46 +++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 44 insertions(+), 2 deletions(-) >> >> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c >> index 8bef103f68..0e1d9510e5 100644 >> --- a/src/qemu/qemu_command.c >> +++ b/src/qemu/qemu_command.c >> @@ -4672,8 +4672,15 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, >> goto error; >> } >> >> - if (STREQ(model, "virtio-gpu")) { >> - if (qemuBuildVirtioDevStr(&buf, "virtio-gpu", qemuCaps, > > Eww, why do we do a string comparison here when we store the model as an > enum? Yup, pre-existing. I suppose that can be fixed.

> >> + if (video->vhostuser) { >> + if (STREQ(model, "virtio-vga")) >> + model = "vhost-user-vga"; >> + if (STREQ(model, "virtio-gpu")) >> + model = "vhost-user-gpu"; >> + } > > Not sure why we need to reassign model here instead of getting it right > the first time. > >> + > > How different is the vhost-user-gpu device from virtio-gpu, > don't we new a new VIDEO_TYPE_VHOST_USER instead? From guest PoV, it's actually the same device. So we shouldn't introduce a new virDomainVideoType, right? But we need a mapping to -device, it has to be in libvirt qemu driver.