[libvirt] [PATCH] Introduce yet another migration version in API.

[Eric Blake]

On 11/02/2010 07:08 AM, Daniel P. Berrange wrote:
> +static virDomainPtr
> +virDomainMigrateVersion3 (virDomainPtr domain,
> +                          virConnectPtr dconn,
> +                          unsigned long flags,
> +                          const char *dname,
> +                          const char *uri,
> +                          unsigned long bandwidth)
> +{
> +    virDomainPtr ddomain = NULL;
> +    char *uri_out = NULL;
> +    char *cookiesrc = NULL;
> +    char *cookiedst = NULL;
> +    char *dom_xml = NULL;
> +    int cookiesrclen = 0;
> +    int cookiedstlen = 0;
> +    int ret;
> +    virDomainInfo info;
> +    virErrorPtr orig_err = NULL;
> +
> +    if (!domain->conn->driver->domainMigrateBegin3) {
> +        virLibConnError (domain->conn, VIR_ERR_INTERNAL_ERROR, __FUNCTION__);
> +        virDispatchError(domain->conn);
> +        return NULL;
> +    }

Do we ever have a step (either here, or when a driver is registered)
that ensures that if domainMigrateBegin3 exists, then all 5
domainMigrate*3 functions exist?  Otherwise, a dlopen()'d driver could
crash libvirt by maliciously supplying one but not all 5 functions.

> +
> +    if (uri == NULL && uri_out == NULL) {
> +        virLibConnError (domain->conn, VIR_ERR_INTERNAL_ERROR,
> +                         _("domainMigratePrepare2 did not set uri"));
> +        virDispatchError(domain->conn);
> +        goto done;
> +    }
> +    if (uri_out)
> +        uri = uri_out; /* Did domainMigratePrepare2 change URI? */
> +    assert (uri != NULL);

Do we really want assert() here?  If someone compiles with NDEBUG, would
that let us fall through and dereference NULL?

> @@ -3716,6 +3875,11 @@ virDomainMigrate (virDomainPtr domain,
>                   VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn,
>                                            VIR_DRV_FEATURE_MIGRATION_V2))
>              ddomain = virDomainMigrateVersion2(domain, dconn, flags, dname, uri, bandwidth);
> +        else if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
> +                                          VIR_DRV_FEATURE_MIGRATION_V3) &&
> +                 VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn,
> +                                          VIR_DRV_FEATURE_MIGRATION_V3))
> +            ddomain = virDomainMigrateVersion3(domain, dconn, flags, dname, uri, bandwidth);

Should migrateVersion3 be attempted with a higher priority than
version2?  As written, this attempts version2 first, so you'll never get
the benefits of version3.

-- 
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/20101103/86e0d6d4/attachment-0001.sig>