[libvirt] [PATCH 02/13] conf: Introduce migrate_tls_x509_cert_dir

[John Ferlan]

On 02/23/2017 09:19 AM, Jiri Denemark wrote:
> On Thu, Feb 23, 2017 at 08:10:18 -0500, John Ferlan wrote:
>>
>>
>> On 02/21/2017 06:43 AM, Jiri Denemark wrote:
>>> On Mon, Feb 20, 2017 at 14:28:42 -0500, John Ferlan wrote:
>>>> On 02/20/2017 11:03 AM, Jiri Denemark wrote:
>>>>> On Fri, Feb 17, 2017 at 14:39:19 -0500, John Ferlan wrote:
>>>>>> +# Enable use of TLS encryption for migration
>>>>>> +#
>>>>>> +# It is necessary to setup CA and issue a server certificate
>>>>>> +# before enabling this.
>>>>>> +#
>>>>>> +#migrate_tls = 1
>>>>>
>>>>> Actually what is this option supposed to do? It seems it doesn't do
>>>>> anything but saying "yes, I configured TLS for migration". The TLS usage
>>>>> for migration is turned on by VIR_MIGRATE_TLS flag which suggests the
>>>>> configuration option here is useless.
>>>>
>>>> It more or less follows the same logic for chardev's which got an
>>>> additional 'tls="yes"' to allow one to enable/disable an object in a
>>>> domain on a case by case basis if chardev tls was enabled for the host.
>>>>
>>>> See: http://libvirt.org/formatdomain.html#elementsConsole
>>>>
>>>> and search down for 'tls'
>>>>
>>>> So my feeling while coding was - if I don't supply some sort of way to
>>>> have an option to allow someone to choose to use the configured TLS
>>>> environment for the migration, I'd end up being asked to add one since
>>>> chardev has it.
>>>
>>> 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. Now for migration we're
going to assume the configuration is set up and can be used.  That just
seems odd, but if that's how it's felt we should proceed, then fine 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.

John