[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [libvirt-designer][PATCH 4/4] domain: Introduce interface support



On 09/05/2012 11:27 AM, Michal Privoznik wrote:
> Let users add NICs to domains.
> ---
>  examples/virtxml.c                         |   96 ++++++++++++++++++++++++----
>  libvirt-designer/libvirt-designer-domain.c |   53 +++++++++++++++
>  libvirt-designer/libvirt-designer-domain.h |    3 +
>  libvirt-designer/libvirt-designer.sym      |    1 +
>  4 files changed, 141 insertions(+), 12 deletions(-)
> 
[...]
> +
> +static const gchar *
> +gvir_designer_domain_get_preferred_nic_model(GVirDesignerDomain *design,
> +                                             GError **error)
> +{
> +    const gchar *ret = NULL;
> +    OsinfoDeviceLink *link = NULL;

You are using "link" here which shadows some global declaration. I'm
saying "some" because the compiler told me so, but after many files I
wasn't so eager to find it, so I don't know which one. Anyway changing
it to dev_link (as you use everywhere else) worked.

> +
> +    link = gvir_designer_domain_get_preferred_device(design, "network", error);
> +    if (!link)
> +        goto cleanup;
> +
> +    ret = osinfo_devicelink_get_driver(link);
> +
> +cleanup:
> +    if (link)
> +        g_object_unref(link);
> +    return ret;
> +}
> +
> +/**
> + * gvir_designer_domain_add_interface_network:
> + * @design: (transfer none): the domain designer instance
> + * @network: (transfer none): network name
> + *
> + * Add new network interface card into @design. The interface is
> + * of 'network' type with @network used as the source network.
> + *
> + * Returns: (transfer none): the pointer to the new interface.
> + */
> +GVirConfigDomainInterface *
> +gvir_designer_domain_add_interface_network(GVirDesignerDomain *design,
> +                                           const char *network,
> +                                           GError **error)
> +{

Are you planning on using gvir...add_interface_full with
add_interface_{network,bridge,etc.} as "wrappers"? I liked that with the
disk in the first patch.

> +    g_return_val_if_fail(GVIR_DESIGNER_IS_DOMAIN(design), NULL);

You check this value here but not on all the previous places (and patches).

> +
> +    GVirConfigDomainInterface *ret;
> +    const gchar *model = NULL;
> +
> +    model = gvir_designer_domain_get_preferred_nic_model(design, error);
> +
> +    ret = GVIR_CONFIG_DOMAIN_INTERFACE(gvir_config_domain_interface_network_new());

I can't find the function anywhere, even though it isn't defined
automagically (IIUC), but it compiles (I wonder what the macro does in
here).

> +
> +    gvir_config_domain_interface_network_set_source(GVIR_CONFIG_DOMAIN_INTERFACE_NETWORK(ret),
> +                                                    network);
> +    if (model)
> +        gvir_config_domain_interface_set_model(ret, model);
> +
> +    gvir_config_domain_add_device(design->priv->config, GVIR_CONFIG_DOMAIN_DEVICE(ret));
> +
> +    return ret;
> +}
> diff --git a/libvirt-designer/libvirt-designer-domain.h b/libvirt-designer/libvirt-designer-domain.h
> index 06a5749..5097393 100644
> --- a/libvirt-designer/libvirt-designer-domain.h
> +++ b/libvirt-designer/libvirt-designer-domain.h
> @@ -101,6 +101,9 @@ GVirConfigDomainDisk *gvir_designer_domain_add_disk_device(GVirDesignerDomain *d
>                                                             const char *devpath,
>                                                             GError **error);
>  
> +GVirConfigDomainInterface *gvir_designer_domain_add_interface_network(GVirDesignerDomain *design,
> +                                                                      const char *network,
> +                                                                      GError **error);
>  G_END_DECLS
>  
>  #endif /* __LIBVIRT_DESIGNER_DOMAIN_H__ */
> diff --git a/libvirt-designer/libvirt-designer.sym b/libvirt-designer/libvirt-designer.sym
> index e67323a..77f76b4 100644
> --- a/libvirt-designer/libvirt-designer.sym
> +++ b/libvirt-designer/libvirt-designer.sym
> @@ -12,6 +12,7 @@ LIBVIRT_DESIGNER_0.0.1 {
>  
>  	gvir_designer_domain_add_disk_file;
>  	gvir_designer_domain_add_disk_device;
> +    gvir_designer_domain_add_interface_network;
>  

Indentation.

Martin


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]