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

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