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

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



On 01/18/2019 09:20 AM, Andrea Bolognani wrote:
> On Thu, 2019-01-17 at 12:52 -0500, Cole Robinson wrote:
>> Add new <interface> model handling for virtio transitional devices. Ex:
>>
>> <interface>
>>   <model type='virtio-transitional'/>
>> </interface>
>>
>> * "virtio-transitional" maps to qemu "virtio-net-pci-transitional"
>> * "virtio-non-transitional" maps to qemu "virtio-net-pci-non-transitional"
>>
>> Signed-off-by: Cole Robinson <crobinso redhat com>
>> (cherry picked from commit b6698b81846e2010e0cc030bcd6c2c549cf04e97)
> 
> Guess this slipped in somehow :)
> 
> [...]
>> @@ -1112,6 +1116,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
>>      { "memory-backend-memfd", QEMU_CAPS_OBJECT_MEMORY_MEMFD },
>>      {"virtio-blk-pci-transitional", QEMU_CAPS_DEVICE_VIRTIO_BLK_TRANSITIONAL},
>>      {"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},
> 
> Same comments about spacing, naming the capabilities and introducing
> them in their separate commit as for virtio-blk apply.
> 
> [...]
>> @@ -449,6 +449,7 @@ qemuBuildVirtioTransitional(virBufferPtr buf,
>>                              virQEMUCapsPtr qemuCaps,
>>                              virDomainDeviceAddressType type,
>>                              int model,
>> +                            const char *modelstr,
> 
> Ewww.
> 
> We really, really need to make the handling of model names for
> network interface more sane.
> 
> In the meantime, can you add some sort of (ideally compile-time)
> sanity check to ensure that the function is either called with
> model == -1 && modelStr != NULL or model >= 0 && modelStr == NULL?
> Or at the very least some documentation on how you're supposed to
> be calling it.
> 

Since hopefully this will be removed by my series in the near future,
I'll just add a docstring for the function to clarify its usage

Thanks,
Cole


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