[libvirt] [PATCH v8 1/5] domain: Add optional 'tls' attribute for TCP chardev

[John Ferlan]

[...]
>>
>> UGH... mea culpa - in my mind the TLS hadn't been added to the chardev
>> yet and that was the rest of this series...  Of course the rest of this
>> series is adding the passphrase for the environment (<sigh>).
>>
>> If I could have got that review earlier, then this situation wouldn't be
>> a problem (see in my mind it wasn't).
>>
>> If a domain is already running, then encryption is occurring and this
>> setting has no effect except for hotplug.
>>
>> If we were using encryption in 2.3.0... stopped our domain... installed
>> 2.4.0... started our domain, then yes, I see/agree....  Frustrating
>> since there's really no "simple" way to determine this.
>>
>>>
>>> The correct solution is:
>>>
>>>     - if chardev_tls is set to 1 and tls attribute is not configured in the XML
>>>     the default after starting the domain should be tls=yes
>>
>> So IOW:
>>
>> +        if (cfg->chardevTLS &&
>> +            dev->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES) {
>>
>> changes to
>>
>> +        if (cfg->chardevTLS &&
>> +            dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_NO) {
>>
>> or (haveTLS == YES || haveTLS == ABSENT)
>>
>> This of course is essentially the logic from v6 which said disableTLS on
>> a case by case basis.... All we have now is the positive form...
>>
>> So for both qemuBuildChrChardevStr and qemuDomainAttachChrDevice I'd
>> have a similar change.  Does that suffice and do you need to see the change?
> 
> I think that we should reflect the state in live XML if the attribute exists.
> 
> Add a code to qemuProcessPrepareDomain() that will set dev->data.tcp.haveTLS
> to config value if dev->data.tcp.haveTLS == VIR_TRISTATE_BOOL_ABSENT and in
> qemuBuildChrChardevStr() you can just check if
> dev->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES.  This will also ensure that
> the live XML contains the 'tls' attribute.  But make sure, that migration works
> properly for all combinations.
> 

I don't see the advantage... Altering the running domain would involve
messing in qemuProcessAttach and setting the attribute for the domain if
chardevTLS is set *BUT* how do we know without a bit of introspection if
that domain was started with TLS and is using TLS? (IOW: looking at the
running domain for tls-creds-x509). Additionally, someone could have
changed the chardevTLS setting in qemu.conf after the domain was
initially started and thus it wouldn't have it, so setting it blindly
would probably have disastrous results.

Altering a domain definition during qemuProcessPrepareDomain (e.g.
domain startup time) still has no way of determining if that domain had
ever been started using TLS if it's not in the XML.

Finally, yeah migration is the final nail in the coffin for this. How
does one migrate from 2.4 to 2.3 if 2.3 doesn't have this attribute we
just quietly set to YES for them?

I think the != NO is the safest solution (plus some adjustment to the
formatdomain.html.in in order to describe this situation).

John

> I guess a another version would be better if you agree with exposing current
> state into live XML.
> 
> Pavel
> 
>>>     - if chardev_tls is not set and tls attribute is not configured in the XML
>>>     the default after starting the domain should be tls=no
>>>
>>
>> We're not getting anywhere if chardev_tls is not set.
>>
>> John
>>
>> [...]
>>
>> --
>> libvir-list mailing list
>> libvir-list at redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list