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

[Cole Robinson]

On 8/29/19 8:43 AM, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Aug 27, 2019 at 1:17 PM Ján Tomko <jtomko at redhat.com> wrote:
>>
>> On Fri, Aug 23, 2019 at 12:21:58PM -0400, Cole Robinson wrote:
>>> From: Marc-André Lureau <marcandre.lureau at redhat.com>
>>>
>>> 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 at redhat.com>
>>> Signed-off-by: Cole Robinson <crobinso at redhat.com>
>>> ---
>>> 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.
> 

virDomainVideoType only has VIRTIO and not virtio-gpu vs virtio-vga;
that distinction depends on a few different criteria like arch and
primary vs secondary. So video->type isn't a drop in replacement here

>>
>>> +    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.
> 

I agree that existing model='virtio' should cover it

- Cole