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

[Daniel P. Berrange]

On Tue, Nov 22, 2011 at 02:07:23PM +0100, Christophe Fergeau wrote:
> Hey,
> 
> On Tue, Nov 22, 2011 at 12:39:31PM +0000, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange at redhat.com>
> > 
> > ---
> >  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

Cut + paste typos :-)

> 
> > + * @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.

Oooh,  gvir_connection_create_domain has the same flaw. I'll fix
both.

> > +
> > +    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.

Again this was a cut+past from gvir_connection_start_domain, so
I'll change both to the style you describe.

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 :|