Re: [libvirt] [PATCH 02/13] conf: Introduce migrate_tls_x509_cert_dir
On Thu, Feb 23, 2017 at 12:19:46 -0500, John Ferlan wrote:
>>> The problem is chardevs and migration are quite
different. While you can
>>> easily have a default configuration for chardevs and use
tls="yes|no" to
>>> override it (no tls attribute will just tell libvirt to use the
>>> default), it's not really possible with migration API. API flags have
>>> not two states (in contrast to three-state boolean attributes in XML).
>>> Setting VIR_MIGRATE_TLS flag turns TLS on and using the flag turns TLS
>>> off. That is the default value is not used anywhere. If VIR_MIGRATE_TLS
>>> flag is used, the code checks migrate_tls is 1. Which translates to
>>> "yes, I configured TLS for migration" semantics of the
configuration
>>> option.
>>>
>>> In other words, it makes sense to have the configuration option for
>>> devices defined in the XML, but using it for migration doesn't make any
>>> sense. This would of course mean its parsing would need to be moved out
>>> of the macro introduced in 1/13 and used for chardevs only.
>>>
>>> Jirka
>>>
>>
>> So I've circled back around to this... It seems like you're advocating
>> the removal of the VIR_MIGRATE_TLS flag - which is fine. The only
>
> No. We definitely need VIR_MIGRATE_TLS. I'm advocating the removal of
> the migrate_tls configuration option.
>
> Jirka
>
For me it's a consistency thing - whenever TLS is used elsewhere in the
code the qemu.conf variables are referenced.
I think the result is actually inconsistent. The existing _tls options
are useful and enable TLS for the affected devices by default. But
migration is not a device, it's an action and not a device. We don't
want to set the default for migration since the usage of TLS is enabled
or disabled by the flag passed to migration APIs.
Now for migration we're going to assume the configuration is set
up
and can be used.
We are not going to assume anything. TLS will not be enabled by default.
So if a user explicitly requests TLS by adding a flag, it's their
responsibility to make sure the environment is setup correctly.
Otherwise migration will just fail. Having to set migrate_tls = 1 in
addition to using the flag is not going to give us anything. The
environment may still be improperly configured.
I can drop the migrate_tls variable, the others would seem to be
needed though. That just means the first patch is dropped and the
second one removes the migrate_tls, but keeps certDir, verify, and
secret uuid.
Right. Although you can still keep the first patch, you just need to
move *_tls out of the macro. But do whathever you think is better.
Jirka