[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 01/18/2019 10:54 AM, Andrea Bolognani wrote:
> 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().

If I rework BuildVirtioTransitional like I mentioned in a previous
response, we don't need to touch this code. Then it can be cleaned up in
a separate patch after this series

Thanks,
Cole


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