[libvirt] [PATCH 13/18] qemu: Support vsock model=virtio-{non-}transitional

[Cole Robinson]

On 01/21/2019 10:48 AM, Andrea Bolognani wrote:
> On Thu, 2019-01-17 at 12:52 -0500, Cole Robinson wrote:
> [...]
>> @@ -893,7 +893,9 @@ VIR_ENUM_IMPL(virDomainIOMMUModel, VIR_DOMAIN_IOMMU_MODEL_LAST,
>>  
>>  VIR_ENUM_IMPL(virDomainVsockModel, VIR_DOMAIN_VSOCK_MODEL_LAST,
>>                "default",
>> -              "virtio")
>> +              "virtio",
>> +              "virtio-transitional",
>> +              "virtio-non-transitional")
> 
> Same comment as always for VIR_ENUM_IMPL().
> 
> [...]
>> @@ -1136,6 +1140,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
>>      {"virtio-9p-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_9P_NON_TRANSITIONAL},
>>      {"virtio-balloon-pci-transitional", QEMU_CAPS_DEVICE_VIRTIO_BALLOON_TRANSITIONAL},
>>      {"virtio-balloon-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_BALLOON_NON_TRANSITIONAL},
>> +    {"vhost-vsock-pci-transitional", QEMU_CAPS_DEVICE_VHOST_VSOCK_TRANSITIONAL},
>> +    {"vhost-vsock-pci-non-transitional", QEMU_CAPS_DEVICE_VHOST_VSOCK_NON_TRANSITIONAL},
>>  };
> 
> Same comment as always for capabilities.
> 
> [...]
>> @@ -495,6 +495,12 @@ qemuBuildVirtioTransitional(virBufferPtr buf,
>>              tmodel_cap = QEMU_CAPS_DEVICE_VIRTIO_BALLOON_TRANSITIONAL;
>>              ntmodel_cap = QEMU_CAPS_DEVICE_VIRTIO_BALLOON_NON_TRANSITIONAL;
>>              break;
>> +        case VIR_DOMAIN_DEVICE_VSOCK:
>> +            has_tmodel = model == VIR_DOMAIN_VSOCK_MODEL_VIRTIO_TRANSITIONAL;
>> +            has_ntmodel = model == VIR_DOMAIN_VSOCK_MODEL_VIRTIO_NON_TRANSITIONAL;
>> +            tmodel_cap = QEMU_CAPS_DEVICE_VHOST_VSOCK_TRANSITIONAL;
>> +            ntmodel_cap = QEMU_CAPS_DEVICE_VHOST_VSOCK_NON_TRANSITIONAL;
>> +            break;
>>  
>>          case VIR_DOMAIN_DEVICE_LEASE:
>>          case VIR_DOMAIN_DEVICE_INPUT:
> 
> Sorry I didn't point this out until now, but having an empty line
> after each 'break;' would be nice from the visual point of view...
> 

Will do

> [...]
>> @@ -10471,11 +10476,11 @@ qemuBuildVsockDevStr(virDomainDefPtr def,
>>      virBuffer buf = VIR_BUFFER_INITIALIZER;
>>      char *ret = NULL;
>>  
>> -    if (vsock->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
>> -        virBufferAddLit(&buf, "vhost-vsock-ccw");
>> -    } else {
>> -        virBufferAddLit(&buf, "vhost-vsock-pci");
>> -    }
>> +    if (qemuBuildVirtioTransitional(&buf, "vhost-vsock", qemuCaps,
>> +                                    vsock->info.type,
>> +                                    vsock->model, NULL,
>> +                                    VIR_DOMAIN_DEVICE_VSOCK) < 0)
>> +        goto cleanup;
> 
> Similar comment as vhost-scsi in 7/18: you should make sure this is
> actually safe, and either way move to qemuBuildVirtioDevStr() first
> and only then to qemuBuildVirtioTransitional().
> 

I'll split this out too.

> [...]
>> +++ b/tests/qemuxml2xmltest.c
>> @@ -1269,12 +1269,14 @@ mymain(void)
>>              QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
>>              QEMU_CAPS_DEVICE_PCIE_PCI_BRIDGE,
>>              QEMU_CAPS_DEVICE_PCIE_ROOT_PORT,
>> -            QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY);
>> +            QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY,
>> +            QEMU_CAPS_DEVICE_VHOST_VSOCK);
>>      DO_TEST("virtio-non-transitional",
>>              QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
>>              QEMU_CAPS_DEVICE_PCIE_PCI_BRIDGE,
>>              QEMU_CAPS_DEVICE_PCIE_ROOT_PORT,
>> -            QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY);
>> +            QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY,
>> +            QEMU_CAPS_DEVICE_VHOST_VSOCK);
> 
> This could have gone into patch 2/18.
> 

Seems a little strange since it only became relevant with this patch,
but I'll change it

- Cole