On Mon, Nov 21, 2011 at 08:27:17PM +0100, Marc-André Lureau wrote:
> Hi
>
> On Mon, Nov 21, 2011 at 6:53 PM, Zeeshan Ali (Khattak)
> <zeeshanak@xxxxxxxxx> wrote:
> > + g_return_val_if_fail(error == NULL || *error == NULL, FALSE);
>
> This is wrong, it should be error != NULL && *error == NULL.
>
> > + if (virDomainDefineXML(conn, xml) == NULL) {
> > + if (error != NULL)
> > + *error = gvir_error_new_literal(GVIR_DOMAIN_ERROR,
> > + 0,
> > + "Failed to set "
> > + "domain configuration");
> > + return FALSE;
> > + }
>
> Can you please verify that the return value is safe to ignore?
>
> I would really prefer we add a runtime check that verifiy the handle
> is the same as the one currently associated with the domain.
The actual pointer returned will *not* be the same as the same
one you currently have, but assuming the XML has matching UUID
and name, it will point to the same domain.
So I guess what you want todo is verify the UUID and name in
the XML, before defining it.
Also, you need to call virDomainFree on the return value of
virDomainDefineXML to avoid memleak.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list