Re: [libvirt] [PATCH v2 00/14] Add TLS support for migration

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. > 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. 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' 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 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.