On 12/03/2015 12:18 PM, Joao Martins wrote:
>
> 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).
Ah, right. Dropping the lock is fine, but IMO we'll need a modify job to protect
the domain from modification by another thread. I think the existing migration
code needs improvement wrt job handling too :-/.
Regards,
Jim
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list