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

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