[libvirt] [PATCH 17/18] qemu: Support virtio-serial controller model=virtio-{non-}transitional

[Cole Robinson]

On 01/21/2019 11:59 AM, Andrea Bolognani wrote:
> On Thu, 2019-01-17 at 12:52 -0500, Cole Robinson wrote:
> [...]
>> +typedef enum {
>> +    VIR_DOMAIN_CONTROLLER_MODEL_VIRTIO_SERIAL_DEFAULT = -1,
>> +    VIR_DOMAIN_CONTROLLER_MODEL_VIRTIO_SERIAL_VIRTIO,
>> +    VIR_DOMAIN_CONTROLLER_MODEL_VIRTIO_SERIAL_VIRTIO_TRANSITIONAL,
>> +    VIR_DOMAIN_CONTROLLER_MODEL_VIRTIO_SERIAL_VIRTIO_NON_TRANSITIONAL,
>> +
>> +    VIR_DOMAIN_CONTROLLER_MODEL_VIRTIO_SERIAL_LAST
>> +} virDomainControllerModelVirtioSerial;
> 
> Do we even need MODEL_VIRTIO_SERIAL_DEFAULT here? Honest question :)
> 

The DEFAULT = -1 is the pattern used by other controller models, and I
believe it's kinda enforced by ControllerDefNew which sets cont->model =
-1. It might not serve any purpose in this particular case besides
consistency.

Working in this area made me explore enum DEFAULT/NONE naming a bit and
there's definitely a lot of room for making things consistent.
controller models are the oddball here with the -1 value but I think
that's to do with some old XML migration compat that can be unwound in a
different way. Another task for another day

> [...]
>> @@ -670,6 +670,8 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
>>              break;
>>  
>>          case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
>> +            if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_VIRTIO_SERIAL_VIRTIO_TRANSITIONAL)
>> +                return pciFlags;
>>              return virtioFlags;
> 
> I'd prefer to see the kind of switch statement you added for
> VIR_DOMAIN_DEVICE_VSOCK here as well.
> 

Okay I'll try to expand that pattern out to other patches too

- Cole