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

[Jiri Denemark]

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