On 02/28/2017 08:48 AM, Jiri Denemark wrote:
On Thu, Feb 23, 2017 at 13:42:02 -0500, John Ferlan wrote:
AFAIU - removal of the objects would remove the migration tls-creds, tls-hostname settings.
This would be a very strange behavior.
I left the cfg->migrateTLS in for now - it's very simple to remove, but there would still need to be a key on something to ensure the migrateTLS environment has been properly configured since that would mean the default environment would need to be used/configured. Setting up the default environment is keyed off having the migrateTLS defined. That's all part of the qemu_conf reading logic.
I still don't see the value in keeping migrate_tls option in qemu.conf. If we want to check the environment is setup correctly we should check the certificates are in place rather than relying on the option. The error reported when migrate_tls is set, but the environment is not prepared is:
internal error: unable to execute QEMU command 'object-add': Unable to access credentials /etc/pki/qemu/ca-cert.pem: No such file or directory
Not ideal but at least it names the file which is missing.
In my latest branch I've removed migrate_tls.
John Ferlan (14): qemu: Introduce qemuDomainSecretInfoNew qemu: Introduce qemuDomainSecretMigratePrepare qemu: Move exit monitor calls in failure paths qemu: Refactor hotplug to introduce qemuDomain{Add|Del}TLSObjects qemu: Refactor qemuDomainGetChardevTLSObjects to converge code qemu: Move qemuDomainSecretChardevPrepare call qemu: Move qemuDomainPrepareChardevSourceTLS call qemu: Introduce qemuDomainGetTLSObjects
The eight patches above are mostly OK except for some small issues.
OK - I will separate them out with the latest adjustments from review and repost.
But the rest of the series needs more work...
qemu: Add TLS params to _qemuMonitorMigrationParams Add new migration flag VIR_MIGRATE_TLS qemu: Create #define for TLS configuration setup. conf: Introduce migrate_tls_x509_cert_dir qemu: Set up the migrate TLS objects for target qemu: Set up the migration TLS objects for source
The code should check whether QEMU actually supports TLS migration, otherwise you will get
internal error: unable to execute QEMU command 'migrate-set-parameters': Invalid parameter 'tls-creds'
Hmm... I see tls-creds added to migrate-set-parameters in 2.7 while they were added to ChardevSocket in 2.6... Ugh. Have to refresh my recollection of how to get the answer I need from capabilities.
As I mentioned in my v1 review, we should always set the parameters if QEMU supports them to make sure they don't contain any leftovers from a previous migration.
I see from a quick scan of the qemu code though that it appears as if the code checks for a non null value being passed: params->has_tls_creds = !!s->parameters.tls_creds; params->has_tls_hostname = !!s->parameters.tls_hostname; So in order to "allow" clearing the tls_creds and tls_hostname, what would one do? The clearing would be necessary since a target of a migration will become a source for a migration and the tls-creds object would be different (using endpoint={server|client}). This would be the I used TLS on one migration, but not on the next type operation for the same domain. Doesn't seem we can assume that TLS will be used for every migration since the desire is to have usage controlled via API option and not host config option
I also said we should properly clean the TLS stuff somewhere inside qemuProcessRecoverMigration*. And if we do it in addition to properly cleaning up everything in the Finish and Confirm phases, we should not need to store migrateTLS in the status XML. We can just always do the cleanup if QEMU supports TLS migration.
Anyway, TLS migration doesn't work even if it is properly configured:
operation failed: migration job: unexpectedly failed
And the destination QEMU log contains:
qemu-system-x86_64: No TLS credentials with id 'objmigrate_tls0'
The reason is an interesting flow of QMP commands issued by libvirt during the Prepare phase:
{ "execute": "object-add", "arguments": { "qom-type": "tls-creds-x509", "id": "objmigrate_tls0", "props": { "dir": "/etc/pki/libvirt", "endpoint": "server", "verify-peer": false } }, "id": "libvirt-21" }
{ "execute": "migrate-set-parameters", "arguments": { "tls-creds": "objmigrate_tls0" }, "id": "libvirt-26" }
{ "execute": "migrate-incoming", "arguments": { "uri": "tcp:[::]:49152" }, "id": "libvirt-27" }
{ "execute": "object-del", "arguments": { "id": "objmigrate_tls0" }, "id": "libvirt-28" }
The error reported after you deleted the objmigrate_tls0 object doesn't seem to confirm your idea about migration parameters begin unset when the object is removed.
OK - well obviously there's still quite a bit for me to understand about the migration model.
Please, test your series before you send a new version of it.
Yep - something I noted in my cover letter - the need for a test environment... Hopefully I'll find a kind soul that will allow me to have access to an already configured environment to test with... John