[libvirt] [PATCH 03/16] Introduce migration cookies to QEMU driver

[Eric Blake]

On 05/12/2011 10:04 AM, Daniel P. Berrange wrote:
> The migration protocol has support for a 'cookie' parameter which
> is an opaque array of bytes as far as libvirt is concerned. Drivers
> may use this for passing around arbitrary extra data they might
> need during migration. The QEMU driver needs to do a few things:
> 
> +static int
> +qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
> +                            xmlXPathContextPtr ctxt,
> +                            int flags ATTRIBUTE_UNUSED)
> +{
> +    char uuidstr[VIR_UUID_STRING_BUFLEN];
> +    char *tmp;
> +
> +    /* We don't store the uuid, name, hostname, or hostuuid
> +     * values. We just compare them to local data to do some
> +     * sanity checking on migration operation
> +     */

Should we do sanity checking that no unknown XML elements appear in the
cookie?  And/or validate that flags contains no unexpected bits?  That
is, should you insert virCheckFlags(0, -1) here, or in
qemuMigrationEatCookie?  And rather than just using virXPathString to
probe whether a particular XML element is present, shouldn't you instead
do an iteration over all XML elements in order to detect unrecognized
elements?

Otherwise, what I'm afraid of is that the cookie eater (whether the
destination eating the cookie from Begin, or the source eating the
cookie from Prepare) will be running an earlier libvirt version than the
baker; if the baker added a mandatory flag to the cookie, but the eater
is unaware to look for that element and silently ignores it, then we
risk silent botching of migration.

Do we have enough infrastructure in place for source and destination to
agree on what features are mandatory vs. optional in the cookie, to
allow for omission of optional elements that would make migration nicer
but aren't fatal if left out?  That is, a baker can always try a flag,
then retry without the flag, but retries can get expensive if there were
an alternative to first do a capability query to determine the common
subset of flags to use in the first place.

> @@ -342,6 +612,15 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver,
>      event = virDomainEventNewFromObj(vm,
>                                       VIR_DOMAIN_EVENT_STARTED,
>                                       VIR_DOMAIN_EVENT_STARTED_MIGRATED);
> +
> +    if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, 0) < 0) {
> +        /* We could tear down the whole guest here, but
> +         * cookie data is (so far) non-critical, so that
> +         * seems a little harsh. We'll just warn for now.
> +         */
> +        VIR_WARN("Unable to encode migration cookie");
> +    }
> +
>      ret = 0;
>  
>  endjob:
> @@ -369,7 +648,7 @@ cleanup:
>          virDomainObjUnlock(vm);
>      if (event)
>          qemuDomainEventQueue(driver, event);
> -    qemuDriverUnlock(driver);
> +    qemuMigrationCookieFree(mig);

Umm, did you really intend to drop the qemuDriverUnlock line?

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110512/5d7634a5/attachment-0001.sig>