Re: [libvirt] [PATCH libvirt-glib 4/5] Add API for creating transient domains

Hey, On Tue, Nov 22, 2011 at 12:39:31PM +0000, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; > > --- > libvirt-gobject/libvirt-gobject-connection.c | 49 ++++++++++++++++++++++++++ > libvirt-gobject/libvirt-gobject-connection.h | 4 ++ No addition of the new function in libvirt-gobject.sym? > 2 files changed, 53 insertions(+), 0 deletions(-) > > diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c > index f52b825..b6e7f3b 100644 > --- a/libvirt-gobject/libvirt-gobject-connection.c > +++ b/libvirt-gobject/libvirt-gobject-connection.c > @@ -1164,6 +1164,10 @@ GVirStream *gvir_connection_get_stream(GVirConnection *self, > * gvir_connection_create_domain: > * @conn: the connection on which to create the dmain domain > * @conf: the configuration for the new domain > + * > + * Create the configuration file for a new persistent domain. > + * The returned domain will initially be in the shutoff state. > + * > * Returns: (transfer full): the newly created domain > */ > GVirDomain *gvir_connection_create_domain(GVirConnection *conn, > @@ -1201,6 +1205,51 @@ GVirDomain *gvir_connection_create_domain(GVirConnection *conn, > } > > /** > + * gvir_connection_start_domain: > + * @conn: the connection on which to create the dmain Same here, domain

> + * @conf: the configuration for the new domain > + * > + * Start a new transient domain without persistent configuration. > + * The returned domain will initially be running. > + * > + * Returns: (transfer full): the newly created domain > + */ > +GVirDomain *gvir_connection_start_domain(GVirConnection *conn, > + GVirConfigDomain *conf, > + guint flags, > + GError **err) > +{ > + const gchar *xml; > + virDomainPtr handle; > + GVirConnectionPrivate *priv = conn->priv; > + > + xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf)); xml shouldn't be const and need to be g_freed when it's no longer needed.

> + > + g_return_val_if_fail(xml != NULL, NULL); > + > + if (!(handle = virDomainCreateXML(priv->conn, xml, flags))) { > + *err = gvir_error_new_literal(GVIR_CONNECTION_ERROR, > + 0, > + "Failed to create domain"); > + return NULL; > + } > + > + GVirDomain *domain; > + > + domain = GVIR_DOMAIN(g_object_new(GVIR_TYPE_DOMAIN, > + "handle", handle, > + NULL)); > + > + g_mutex_lock(priv->lock); > + g_hash_table_insert(priv->domains, > + (gpointer)gvir_domain_get_uuid(domain), > + domain); > + g_mutex_unlock(priv->lock); > + > + return g_object_ref(domain); Here I'd do g_mutex_lock(priv->lock); g_hash_table_insert(priv->domains, (gpointer)gvir_domain_get_uuid(domain), g_object_ref(domain)); g_mutex_unlock(priv->lock); return domain; to make it clearer that the hash table needs a ref on the object. The way it's written, I had to check how priv->domains is declared to make sure that wasn't an extra _ref.