Re: [libvirt] KVM migration patch

[Daniel Veillard <veillard redhat com>]

On Tue, Jun 03, 2008 at 09:17:01AM +0100, Richard W.M. Jones wrote:
> 
> I've just updated this patch so it can be applied to CVS.
[...]
> Index: src/libvirt.c
> ===================================================================
> RCS file: /data/cvs/libvirt/src/libvirt.c,v
> retrieving revision 1.143
[...]
> +        if (uri_out) uri = uri_out; /* Did domainMigratePrepare change URI? */
> +
> +        assert (uri != NULL);

  Hum, assert forces the process to exit. I don't think that's
acceptable for one of the library entry points.
     And error and returning NULL looks far mor proper there.

[...]
>  
> -    assert (uri != NULL);
> +        assert (uri != NULL);
> +    }
[...]
> +qemudDomainMigratePerform (virDomainPtr dom,
> +                           const char *cookie ATTRIBUTE_UNUSED,
> +                           int cookielen ATTRIBUTE_UNUSED,
> +                           const char *uri,
> +                           unsigned long flags ATTRIBUTE_UNUSED,
> +                           const char *dname ATTRIBUTE_UNUSED,
> +                           unsigned long resource)
> +{
[...]
> +        free (info);
[...]
> +    free (safe_uri);
[...]
> +    free (info);

  VIR_FREE should be used now i think.

  Except for those points the patch looks fine to me,

Daniel


-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/