Hi
Hi
Agreed with the comments from teuf +
On Fri, Nov 18, 2011 at 8:24 PM, Zeeshan Ali (Khattak)
<zeeshanak(a)gnome.org> wrote:
+void gvir_connection_redefine_domain(GVirConnection *conn,
+ GVirDomain *domain,
+ GVirConfigDomain *conf,
+ GError **err)
+{
I would check that the given arguments are correct, != NULL or GVIR_IS_FOO.
I would return TRUE on success, and error can be NULL (regular glib convention)
+ const gchar *xml;
+ virDomainPtr handle;
+ GVirDomain *dom;
+ virDomainPtr dom_handle;
+ GVirConnectionPrivate *priv = conn->priv;
+
+ xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf));
+
+ g_return_if_fail(xml != NULL);
+
+ g_mutex_lock(priv->lock);
+ dom = g_hash_table_lookup (priv->domains,
+ (gpointer)gvir_domain_get_uuid(domain));
+ g_mutex_unlock(priv->lock);
+ g_return_if_fail(dom != NULL);
+ /* FIXME: Check if config's domain ID is the same as domain passed */
I suppose this is missing Christophe gconfig patches.
+ if (!(handle = virDomainDefineXML(priv->conn, xml))) {
+ *err = gvir_error_new_literal(GVIR_CONNECTION_ERROR,
+ 0,
+ "Failed to redefine domain");
+ return NULL;
+ }
+}
And I would verify that handle is == to current domain handle. If not,
we probably need to replace it or we need to throw an error. At the
minimum it would be nice to leave a comment after verifying that it is
safe to ignore return value.
(then send an updated patch for ack)
regards
--
Marc-André Lureau