[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Thu, 2019-01-17 at 12:52 -0500, Cole Robinson wrote:
[...]
> @@ -1118,6 +1120,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
>      {"virtio-blk-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_BLK_NON_TRANSITIONAL},
>      {"virtio-net-pci-transitional", QEMU_CAPS_DEVICE_VIRTIO_NET_TRANSITIONAL},
>      {"virtio-net-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_NET_NON_TRANSITIONAL},
> +    {"vhost-scsi-pci-transitional", QEMU_CAPS_DEVICE_VHOST_SCSI_TRANSITIONAL},
> +    {"vhost-scsi-pci-non-transitional", QEMU_CAPS_DEVICE_VHOST_SCSI_NON_TRANSITIONAL},
>  };

Usual comment about capabilities.

[...]
> @@ -5038,10 +5043,11 @@ qemuBuildSCSIVHostHostdevDevStr(const virDomainDef *def,
>          goto cleanup;
>      }
>  
> -    if (ARCH_IS_S390(def->os.arch))
> -        virBufferAddLit(&buf, "vhost-scsi-ccw");
> -    else
> -        virBufferAddLit(&buf, "vhost-scsi-pci");
> +    if (qemuBuildVirtioTransitional(&buf, "vhost-scsi", qemuCaps,
> +                                    dev->info->type,
> +                                    hostsrc->model, NULL,
> +                                    VIR_DOMAIN_DEVICE_HOSTDEV) < 0)
> +        goto cleanup;

This changes quite a bit: instead of basing the choice of device
upon the architecture and supporting -ccw and -pci only, now we
base it upon address type and potentially support -device as well
as-s390.

I'm assuming the latter is not going to be a problem because there
are probably checks along the way preventing us to get there in
the first place, but we should definitely make sure we address the
former correctly, most likely by setting the address type based on
the architecture in postParse().

Either way, once you've made sure the change is actually safe or
added code that makes it so, you should switch from the open-coded
version to qemuBuildVirtioDevStr(), and only later in a separate
commit switch from that to qemuBuildVirtioTransitional().

>  
>      virBufferAsprintf(&buf, ",wwpn=%s,vhostfd=%s,id=%s",
>                        hostsrc->wwpn,
> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> index 654567c500..c334ba441f 100644
> --- a/src/qemu/qemu_domain_address.c
> +++ b/src/qemu/qemu_domain_address.c
> @@ -785,11 +785,13 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
>              return pcieFlags;
>  
>          /* according to pbonzini, from the guest PoV vhost-scsi devices
> -         * are the same as virtio-scsi, so they should use virtioFlags
> -         * (same as virtio-scsi) to determine Express vs. legacy placement
> +         * are the same as virtio-scsi, so they should follor virtio logic
>           */

s/follor/follow/

Everything else looks fine.

-- 
Andrea Bolognani / Red Hat / Virtualization


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]