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