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

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