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

[Jiri Denemark]

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.

Please, test your series before you send a new version of it.

Jirka