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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list