On 12/02/2015 11:27 PM, Jim Fehlig wrote:
> On 11/10/2015 08:32 AM, Joao Martins wrote:
>> Introduce support for VIR_MIGRATE_PEER2PEER in libxl driver
>> for supporting migration in Openstack. Most of the changes
>> occur at the source and no modifications at the receiver.
>>
>> In P2P mode there is only the Perform phase so we must handle
>> the connection with the destination and actually perform the
>> migration. libxlDomainPerformP2P implements the connection to
>> the destination and let libxlDoMigrateP2P implements the actual
>> migration logic with virConnectPtr. In this function we do
>> the migration steps in the destination similar to
>> virDomainMigrateVersion3Full. We appropriately save the last
>> error reported in each of the phases to provide proper
>> reporting. We don't yet support VIR_MIGRATE_TUNNELED and
>> we always use V3 with extensible params, making the
>> implementation simpler.
>>
>> It is worth noting that the receiver didn't have any changes,
>> and because it's still the v3 sequence thus it is possible to
>> migrate from a P2P to non-P2P host.
>>
>> Signed-off-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
>> ---
>> Changes since v1:
>> - Move Begin step to libxlDoMigrateP2P to have all 4 steps
>> together.
>> - Remove if before VIR_FREE(dom_xml)
>> ---
>> src/libxl/libxl_driver.c | 13 ++-
>> src/libxl/libxl_migration.c | 220 ++++++++++++++++++++++++++++++++++++++++++++
>> src/libxl/libxl_migration.h | 11 +++
>> 3 files changed, 241 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index fcdcbdb..da98265 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -4713,6 +4713,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int feature)
>> switch (feature) {
>> case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
>> case VIR_DRV_FEATURE_MIGRATION_PARAMS:
>> + case VIR_DRV_FEATURE_MIGRATION_P2P:
>> return 1;
>> default:
>> return 0;
>> @@ -5039,9 +5040,15 @@ libxlDomainMigratePerform3Params(virDomainPtr dom,
>> if (virDomainMigratePerform3ParamsEnsureACL(dom->conn, vm->def) < 0)
>> goto cleanup;
>>
>> - if (libxlDomainMigrationPerform(driver, vm, dom_xml, dconnuri,
>> - uri, dname, flags) < 0)
>> - goto cleanup;
>> + if (flags & VIR_MIGRATE_PEER2PEER) {
>> + if (libxlDomainMigrationPerformP2P(driver, vm, dom->conn, dom_xml,
>> + dconnuri, uri, dname, flags) < 0)
>> + goto cleanup;
>> + } else {
>> + if (libxlDomainMigrationPerform(driver, vm, dom_xml, dconnuri,
>> + uri, dname, flags) < 0)
>> + goto cleanup;
>> + }
>>
>> ret = 0;
>>
>> diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
>> index 0d23e5f..a1c7b55 100644
>> --- a/src/libxl/libxl_migration.c
>> +++ b/src/libxl/libxl_migration.c
>> @@ -42,6 +42,7 @@
>> #include "libxl_conf.h"
>> #include "libxl_migration.h"
>> #include "locking/domain_lock.h"
>> +#include "virtypedparam.h"
>>
>> #define VIR_FROM_THIS VIR_FROM_LIBXL
>>
>> @@ -456,6 +457,225 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
>> return ret;
>> }
>>
>> +/* This function is a simplification of virDomainMigrateVersion3Full
>> + * excluding tunnel support and restricting it to migration v3
>> + * with params since it was the first to be introduced in libxl.
>> + */
>> +static int
>> +libxlDoMigrateP2P(libxlDriverPrivatePtr driver,
>> + virDomainObjPtr vm,
>> + virConnectPtr sconn,
>> + const char *xmlin,
>> + virConnectPtr dconn,
>> + const char *dconnuri ATTRIBUTE_UNUSED,
>> + const char *dname,
>> + const char *uri,
>> + unsigned int flags)
>> +{
>> + virDomainPtr ddomain = NULL;
>> + virTypedParameterPtr params = NULL;
>> + int nparams = 0;
>> + int maxparams = 0;
>> + char *uri_out = NULL;
>> + char *dom_xml = NULL;
>> + unsigned long destflags;
>> + bool cancelled = true;
>> + virErrorPtr orig_err = NULL;
>> + int ret = -1;
>> +
>> + dom_xml = libxlDomainMigrationBegin(sconn, vm, xmlin);
>> + if (!dom_xml)
>> + goto cleanup;
>> +
>> + if (virTypedParamsAddString(¶ms, &nparams, &maxparams,
>> + VIR_MIGRATE_PARAM_DEST_XML, dom_xml) < 0)
>> + goto cleanup;
>> +
>> + if (dname &&
>> + virTypedParamsAddString(¶ms, &nparams, &maxparams,
>> + VIR_MIGRATE_PARAM_DEST_NAME, dname) < 0)
>> + goto cleanup;
>> +
>> + if (uri &&
>> + virTypedParamsAddString(¶ms, &nparams, &maxparams,
>> + VIR_MIGRATE_PARAM_URI, uri) < 0)
>> + goto cleanup;
>> +
>> + /* We don't require the destination to have P2P support
>> + * as it looks to be normal migration from the receiver perpective.
>> + */
>> + destflags = flags & ~(VIR_MIGRATE_PEER2PEER);
>> +
>> + VIR_DEBUG("Prepare3");
>> + virObjectUnlock(vm);
>> + ret = dconn->driver->domainMigratePrepare3Params
>> + (dconn, params, nparams, NULL, 0, NULL, NULL, &uri_out, destflags);
>> + virObjectLock(vm);
>
> Is it necessary to unlock and lock the vm while calling prepare and finish on
> the destination libvirtd?
>
They can be removed, but then we leave the virDomainObj locked for a "long" time
when no operations are being done on his context. Plus no API calls could be
done there in the meantime (and migration can take a rather long time). Prepare,
Finish, ConnectOpenAuth and ConnectSupportsFeature are done on the remote
driver (RPC calls to the target libvirtd) so they have a rather high "cost".
What do you think?
>> +
>> + if (ret == -1)
>> + goto cleanup;
>> +
>> + if (uri_out) {
>> + if (virTypedParamsReplaceString(¶ms, &nparams,
>> + VIR_MIGRATE_PARAM_URI, uri_out) < 0) {
>> + orig_err = virSaveLastError();
>> + goto finish;
>> + }
>> + } else {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("domainMigratePrepare3 did not set uri"));
>> + goto finish;
>> + }
>> +
>> + VIR_DEBUG("Perform3 uri=%s", NULLSTR(uri_out));
>> + ret = libxlDomainMigrationPerform(driver, vm, NULL, NULL,
>> + uri_out, NULL, flags);
>> +
>> + if (ret < 0)
>> + orig_err = virSaveLastError();
>> +
>> + cancelled = (ret < 0);
>> +
>> + finish:
>> + VIR_DEBUG("Finish3 ret=%d", ret);
>> + if (virTypedParamsGetString(params, nparams,
>> + VIR_MIGRATE_PARAM_DEST_NAME, NULL) <= 0 &&
>> + virTypedParamsReplaceString(¶ms, &nparams,
>> + VIR_MIGRATE_PARAM_DEST_NAME,
>> + vm->def->name) < 0) {
>> + ddomain = NULL;
>> + } else {
>> + virObjectUnlock(vm);
>> + ddomain = dconn->driver->domainMigrateFinish3Params
>> + (dconn, params, nparams, NULL, 0, NULL, NULL,
>> + destflags, cancelled);
>> + virObjectLock(vm);
>> + }
>> +
>> + cancelled = (ddomain == NULL);
>> +
>> + /* If Finish3Params set an error, and we don't have an earlier
>> + * one we need to preserve it in case confirm3 overwrites
>> + */
>> + if (!orig_err)
>> + orig_err = virSaveLastError();
>> +
>> + VIR_DEBUG("Confirm3 cancelled=%d vm=%p", cancelled, vm);
>> + ret = libxlDomainMigrationConfirm(driver, vm, flags, cancelled);
>> +
>> + if (ret < 0)
>> + VIR_WARN("Guest %s probably left in 'paused' state on source",
>> + vm->def->name);
>> +
>> + cleanup:
>> + if (ddomain) {
>> + virObjectUnref(ddomain);
>> + ret = 0;
>> + } else {
>> + ret = -1;
>> + }
>> +
>> + if (orig_err) {
>> + virSetError(orig_err);
>> + virFreeError(orig_err);
>> + }
>> +
>> + VIR_FREE(dom_xml);
>> + VIR_FREE(uri_out);
>> + virTypedParamsFree(params, nparams);
>> + return ret;
>> +}
>> +
>> +static int virConnectCredType[] = {
>> + VIR_CRED_AUTHNAME,
>> + VIR_CRED_PASSPHRASE,
>> +};
>> +
>> +static virConnectAuth virConnectAuthConfig = {
>> + .credtype = virConnectCredType,
>> + .ncredtype = ARRAY_CARDINALITY(virConnectCredType),
>> +};
>> +
>> +static void
>> +libxlMigrationConnectionClosed(virConnectPtr conn,
>> + int reason,
>> + void *opaque)
>> +{
>> + virDomainObjPtr vm = opaque;
>> +
>> + VIR_DEBUG("conn=%p, reason=%d, vm=%s", conn, reason, vm->def->name);
>> + virDomainObjBroadcast(vm);
>
> I would expect to see a corresponding virDomainObjWait for this call to
> virDomainObjBroadcast. But with the current migration code, I don't think the
> connect close callback is needed. Is there anything to do when the connection to
> destination libvirtd closes?
>
You're right, it's my mistake. There is no one waiting for migration to complete
so this part isn't needed. I will remove the close callback on v3.
Regards,
Joao
>> +}
>> +
>> +/* On P2P mode there is only the Perform3 phase and we need to handle
>> + * the connection with the destination libvirtd and perform the migration.
>> + * Here we first tackle the first part of it, and libxlDoMigrationP2P handles
>> + * the migration process with an established virConnectPtr to the destination.
>> + */
>> +int
>> +libxlDomainMigrationPerformP2P(libxlDriverPrivatePtr driver,
>> + virDomainObjPtr vm,
>> + virConnectPtr sconn,
>> + const char *xmlin,
>> + const char *dconnuri,
>> + const char *uri_str ATTRIBUTE_UNUSED,
>> + const char *dname,
>> + unsigned int flags)
>> +{
>> + int ret = -1;
>> + int keepAliveInterval = 5;
>> + int keepAliveCount = 5;
>> + bool useParams;
>> + virConnectPtr dconn = NULL;
>> + virErrorPtr orig_err = NULL;
>> +
>> + virObjectUnlock(vm);
>> + dconn = virConnectOpenAuth(dconnuri, &virConnectAuthConfig, 0);
>> + virObjectLock(vm);
>> +
>> + if (dconn == NULL) {
>> + virReportError(VIR_ERR_OPERATION_FAILED,
>> + _("Failed to connect to remote libvirt URI %s: %s"),
>> + dconnuri, virGetLastErrorMessage());
>> + return ret;
>> + }
>> +
>> + if (virConnectSetKeepAlive(dconn, keepAliveInterval,
>> + keepAliveCount) < 0)
>> + goto cleanup;
>> +
>> + if (virConnectRegisterCloseCallback(dconn, libxlMigrationConnectionClosed,
>> + vm, NULL) < 0) {
>> + goto cleanup;
>> + }
>> +
>> + virObjectUnlock(vm);
>> + useParams = VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn,
>> + VIR_DRV_FEATURE_MIGRATION_PARAMS);
>> + virObjectLock(vm);
>> +
>> + if (!useParams) {
>> + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
>> + _("Destination libvirt does not support migration with extensible parameters"));
>> + goto cleanup;
>> + }
>> +
>> + ret = libxlDoMigrateP2P(driver, vm, sconn, xmlin, dconn, dconnuri,
>> + dname, uri_str, flags);
>> +
>> + cleanup:
>> + orig_err = virSaveLastError();
>> + virObjectUnlock(vm);
>> + virConnectUnregisterCloseCallback(dconn, libxlMigrationConnectionClosed);
>> + virObjectUnref(dconn);
>> + virObjectLock(vm);
>> + if (orig_err) {
>> + virSetError(orig_err);
>> + virFreeError(orig_err);
>> + }
>> + return ret;
>> +}
>> +
>> int
>> libxlDomainMigrationPerform(libxlDriverPrivatePtr driver,
>> virDomainObjPtr vm,
>> diff --git a/src/libxl/libxl_migration.h b/src/libxl/libxl_migration.h
>> index 20b45d8..0f83bb4 100644
>> --- a/src/libxl/libxl_migration.h
>> +++ b/src/libxl/libxl_migration.h
>> @@ -28,6 +28,7 @@
>>
>> # define LIBXL_MIGRATION_FLAGS \
>> (VIR_MIGRATE_LIVE | \
>> + VIR_MIGRATE_PEER2PEER | \
>> VIR_MIGRATE_UNDEFINE_SOURCE | \
>> VIR_MIGRATE_PAUSED)
>>
>> @@ -56,6 +57,16 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
>> unsigned int flags);
>>
>> int
>> +libxlDomainMigrationPerformP2P(libxlDriverPrivatePtr driver,
>> + virDomainObjPtr vm,
>> + virConnectPtr sconn,
>> + const char *dom_xml,
>> + const char *dconnuri,
>> + const char *uri_str,
>> + const char *dname,
>> + unsigned int flags);
>> +
>> +int
>> libxlDomainMigrationPerform(libxlDriverPrivatePtr driver,
>> virDomainObjPtr vm,
>> const char *dom_xml,
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list