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

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

[...] > @@ -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().

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