[libvirt] [PATCH v4 2/6] vz: add migration backbone code

[Daniel P. Berrange]

On Wed, Sep 02, 2015 at 03:09:23PM +0300, Nikolay Shirokovskiy wrote:
> From: nshirokovskiy at virtuozzo.com <nshirokovskiy at virtuozzo.com>
> 
> This patch makes basic vz migration possible. For example by virsh:
> 
> virsh -c vz:///system migrate $NAME vz+ssh://$DST/system --p2p
> 
> Vz migration is implemented as p2p migration. The reason
> is that vz sdk do all the job. The question may arise then
> why don't implement it as a direct migration. The reason
> is that we want to leverage rich libvirt authentication abilities
> we lack in vz sdk. We can do it because vz sdk can use tokens to
> factor out authentication from migration command.
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
>  src/vz/vz_driver.c |  192 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/vz/vz_sdk.c    |   33 +++++++++
>  src/vz/vz_sdk.h    |    2 +
>  3 files changed, 227 insertions(+), 0 deletions(-)
> 

> +static int
> +vzDomainMigratePrepare3Params(virConnectPtr conn,
> +                                virTypedParameterPtr params ATTRIBUTE_UNUSED,
> +                                int nparams ATTRIBUTE_UNUSED,
> +                                const char *cookiein ATTRIBUTE_UNUSED,
> +                                int cookieinlen ATTRIBUTE_UNUSED,
> +                                char **cookieout,
> +                                int *cookieoutlen,
> +                                char **uri_out ATTRIBUTE_UNUSED,
> +                                unsigned int flags)
> +{
> +    vzConnPtr privconn = conn->privateData;
> +    int ret = -1;
> +
> +    virCheckFlags(0, -1);
> +
> +    if (!(*cookieout = vzFormatCookie(privconn->session_uuid)))
> +        goto cleanup;
> +    *cookieoutlen = strlen(*cookieout) + 1;

As mentioned before, you should fill in uri_out here with the
hypervisor URI to use for migration, by filling in the URI of
the current host (ie dest host). eg

     char *thishost = virGetHostname();
     virAsprintf(uri_out, "tcp://%s", thishost);
     VIR_FREE(thishost);

> +    ret = 0;
> +
> + cleanup:
> +    if (ret != 0) {
> +        VIR_FREE(*cookieout);
> +        *cookieoutlen = 0;
> +    }
> +
> +    return ret;
> +}
> +

> +static int
> +vzDomainMigratePerform3Params(virDomainPtr domain,
> +                              const char *dconnuri,
> +                              virTypedParameterPtr params,
> +                              int nparams,
> +                              const char *cookiein ATTRIBUTE_UNUSED,
> +                              int cookieinlen ATTRIBUTE_UNUSED,
> +                              char **cookieout ATTRIBUTE_UNUSED,
> +                              int *cookieoutlen ATTRIBUTE_UNUSED,
> +                              unsigned int flags)
> +{
> +    int ret = -1;
> +    virDomainObjPtr dom = NULL;
> +    virConnectPtr dconn = NULL;
> +    virURIPtr vzuri = NULL;
> +    unsigned char session_uuid[VIR_UUID_BUFLEN];
> +    vzConnPtr privconn = domain->conn->privateData;
> +    char *cookie = NULL;
> +    int cookielen = 0;
> +
> +    virCheckFlags(VZ_MIGRATION_FLAGS, -1);
> +
> +    if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0)
> +        goto cleanup;
> +
> +    if (!(vzuri = vzMakeVzUri(dconnuri)))
> +        goto cleanup;

This is not right - you can't use the libvirt URI to form the
hypervisor migration URI, since the libvirt URI may not in
fact refer to the hypervisor host.

eg people may be accessing libvirt(d) via a SSH tunnel in
which case the dconnuri would include 'localhost' and not
the actual target host. This is why you must fill in the
uri_out parameter in the Prepare() method and use that,
if the "migrateuri" parameter is not provided in the
virTypedParams array.

> +
> +    if (!(dom = vzDomObjFromDomain(domain)))
> +        goto cleanup;
> +
> +    dconn = virConnectOpen(dconnuri);
> +    if (dconn == NULL) {
> +        virReportError(VIR_ERR_OPERATION_FAILED,
> +                       _("Failed to connect to remote libvirt URI %s: %s"),
> +                       dconnuri, virGetLastErrorMessage());
> +        goto cleanup;
> +    }
> +
> +    /* NULL and zero elements are unused */
> +    if (virDomainMigratePrepare3Params(dconn, NULL, 0, NULL, 0,
> +                                       &cookie, &cookielen, NULL, 0) < 0)
> +        goto cleanup;

Since this is implementing v3, I'd prefer to see you provide the
full set of 5 callbacks, even though they will currently be no-ops.
This provides better future proofing for the migration impl in case
those become neccessary later.

You can also then trivially implement the non-p2p plain migration
in this method, by checking whether or not the PEER2PEER flag
is set or not. If it is not set, you can just skip the connect
open & prepare calls on the basis that libvirt will have done
them for you.

> +
> +    if (vzParseCookie(cookie, session_uuid) < 0)
> +        goto cleanup;
> +
> +    if (prlsdkMigrate(dom, vzuri, session_uuid) < 0)
> +        goto cleanup;
> +
> +    virDomainObjListRemove(privconn->domains, dom);
> +    dom = NULL;
> +
> +    ret = 0;
> +
> + cleanup:
> +    if (dom)
> +        virObjectUnlock(dom);
> +    virObjectUnref(dconn);
> +    virURIFree(vzuri);
> +    VIR_FREE(cookie);
> +
> +    return ret;
> +}
> +
>  static virHypervisorDriver vzDriver = {
>      .name = "vz",
>      .connectOpen = vzConnectOpen,            /* 0.10.0 */
> @@ -1396,6 +1585,9 @@ static virHypervisorDriver vzDriver = {
>      .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.2.17 */
>      .domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */
>      .domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */
> +    .connectSupportsFeature = vzConnectSupportsFeature, /* 1.2.20 */
> +    .domainMigratePrepare3Params = vzDomainMigratePrepare3Params, /* 1.2.20 */
> +    .domainMigratePerform3Params = vzDomainMigratePerform3Params, /* 1.2.20 */

Somewhat annoyingly you also need to implement the callbacks for
.domainMigratePrepare3 and .domainMigratePerform3, as we don't
automatically convert non-params usage to the params based
method AFAICT.

Your impl of .domainMigratePerform3 could pack the values into a
virTypedParams array and then call .domainMigratePerform3Params,
or do the reverse.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|