Hey,
Here's an attempt at reviewing the rest of this patch. I'm not familiar at
all with libvirt-sandbox, so I'm not in a really good position to review
how this fits in the overall design of the library.
Christophe
On Tue, Nov 05, 2013 at 05:25:54PM -0800, Ian Main wrote:
> This patch adds two new classes, filterref and filterref-parameter.
> Network interfaces can now have an associated filter reference with any
> number of filterref parameters. Also added filter= option to
> virt-sandbox tool.
>
> V2:
>
> - Changed set_filter to set_name and get_filter to get_name.
> ---
> libvirt-sandbox/Makefile.am | 4 +
> .../libvirt-sandbox-builder-container.c | 37 +++-
> libvirt-sandbox/libvirt-sandbox-builder-machine.c | 36 ++++
> ...rt-sandbox-config-network-filterref-parameter.c | 205 ++++++++++++++++++++
> ...rt-sandbox-config-network-filterref-parameter.h | 75 ++++++++
> .../libvirt-sandbox-config-network-filterref.c | 209 +++++++++++++++++++++
> .../libvirt-sandbox-config-network-filterref.h | 75 ++++++++
> libvirt-sandbox/libvirt-sandbox-config-network.c | 33 ++++
> libvirt-sandbox/libvirt-sandbox-config-network.h | 4 +
> libvirt-sandbox/libvirt-sandbox-config.c | 39 ++++
> libvirt-sandbox/libvirt-sandbox.h | 3 +
> libvirt-sandbox/libvirt-sandbox.sym | 14 ++
> 12 files changed, 733 insertions(+), 1 deletion(-)
> create mode 100644 libvirt-sandbox/libvirt-sandbox-config-network-filterref-parameter.c
> create mode 100644 libvirt-sandbox/libvirt-sandbox-config-network-filterref-parameter.h
> create mode 100644 libvirt-sandbox/libvirt-sandbox-config-network-filterref.c
> create mode 100644 libvirt-sandbox/libvirt-sandbox-config-network-filterref.h
>
> diff --git a/libvirt-sandbox/libvirt-sandbox-builder-container.c b/libvirt-sandbox/libvirt-sandbox-builder-container.c
> index 43ee5ef..db70403 100644
> --- a/libvirt-sandbox/libvirt-sandbox-builder-container.c
> +++ b/libvirt-sandbox/libvirt-sandbox-builder-container.c
> @@ -319,11 +319,12 @@ static gboolean gvir_sandbox_builder_container_construct_devices(GVirSandboxBuil
> g_list_foreach(mounts, (GFunc)g_object_unref, NULL);
> g_list_free(mounts);
>
> -
> tmp = networks = gvir_sandbox_config_get_networks(config);
> while (tmp) {
> const gchar *source, *mac;
> GVirSandboxConfigNetwork *network = GVIR_SANDBOX_CONFIG_NETWORK(tmp->data);
> + GVirSandboxConfigNetworkFilterref *filterref;
> + GVirConfigDomainInterfaceFilterref *glib_fref;
>
> iface = gvir_config_domain_interface_network_new();
> source = gvir_sandbox_config_network_get_source(network);
> @@ -339,6 +340,40 @@ static gboolean gvir_sandbox_builder_container_construct_devices(GVirSandboxBuil
>
> gvir_config_domain_add_device(domain,
> GVIR_CONFIG_DOMAIN_DEVICE(iface));
> +
> + filterref = gvir_sandbox_config_network_get_filterref(network);
> + if (filterref) {
> + GList *param_iter, *parameters;
> + const gchar *fref_name = gvir_sandbox_config_network_filterref_get_name(filterref);
> + glib_fref = gvir_config_domain_interface_filterref_new();
> + gvir_config_domain_interface_filterref_set_name(glib_fref, fref_name);
> + param_iter = parameters = gvir_sandbox_config_network_filterref_get_parameters(filterref);
> + while (param_iter) {
> + const gchar *name;
> + const gchar *value;
> + GVirSandboxConfigNetworkFilterrefParameter *param = GVIR_SANDBOX_CONFIG_NETWORK_FILTERREF_PARAMETER(param_iter->data);
> + GVirConfigDomainInterfaceFilterrefParameter *glib_param;
> +
> + name = gvir_sandbox_config_network_filterref_parameter_get_name(param);
> + value = gvir_sandbox_config_network_filterref_parameter_get_value(param);
> +
> + glib_param = gvir_config_domain_interface_filterref_parameter_new();
> + gvir_config_domain_interface_filterref_parameter_set_name(glib_param, name);
> + gvir_config_domain_interface_filterref_parameter_set_value(glib_param, value);
> +
> + gvir_config_domain_interface_filterref_add_parameter(glib_fref, glib_param);
> + g_object_unref(glib_param);
> +
> + param_iter = param_iter->next;
> + }
> +
> + g_list_foreach(parameters, (GFunc)g_object_unref, NULL);
> + g_list_free(parameters);
> +
> + gvir_config_domain_interface_set_filterref(GVIR_CONFIG_DOMAIN_INTERFACE(iface), glib_fref);
> + g_object_unref(glib_fref);
> + }
> +
> g_object_unref(iface);
>
> tmp = tmp->next;
> diff --git a/libvirt-sandbox/libvirt-sandbox-builder-machine.c b/libvirt-sandbox/libvirt-sandbox-builder-machine.c
> index a8c5d8c..0cfedc7 100644
> --- a/libvirt-sandbox/libvirt-sandbox-builder-machine.c
> +++ b/libvirt-sandbox/libvirt-sandbox-builder-machine.c
> @@ -577,6 +577,8 @@ static gboolean gvir_sandbox_builder_machine_construct_devices(GVirSandboxBuilde
> while (tmp) {
> const gchar *source, *mac;
> GVirSandboxConfigNetwork *network = GVIR_SANDBOX_CONFIG_NETWORK(tmp->data);
> + GVirSandboxConfigNetworkFilterref *filterref;
> + GVirConfigDomainInterfaceFilterref *glib_fref;
>
> source = gvir_sandbox_config_network_get_source(network);
> if (source) {
> @@ -596,6 +598,40 @@ static gboolean gvir_sandbox_builder_machine_construct_devices(GVirSandboxBuilde
>
> gvir_config_domain_add_device(domain,
> GVIR_CONFIG_DOMAIN_DEVICE(iface));
> +
> + filterref = gvir_sandbox_config_network_get_filterref(network);
> + if (filterref) {
> + GList *param_iter, *parameters;
> + const gchar *fref_name = gvir_sandbox_config_network_filterref_get_name(filterref);
> + glib_fref = gvir_config_domain_interface_filterref_new();
> + gvir_config_domain_interface_filterref_set_name(glib_fref, fref_name);
> + param_iter = parameters = gvir_sandbox_config_network_filterref_get_parameters(filterref);
> + while (param_iter) {
> + const gchar *name;
> + const gchar *value;
> + GVirSandboxConfigNetworkFilterrefParameter *param = GVIR_SANDBOX_CONFIG_NETWORK_FILTERREF_PARAMETER(param_iter->data);
> + GVirConfigDomainInterfaceFilterrefParameter *glib_param;
> +
> + name = gvir_sandbox_config_network_filterref_parameter_get_name(param);
> + value = gvir_sandbox_config_network_filterref_parameter_get_value(param);
> +
> + glib_param = gvir_config_domain_interface_filterref_parameter_new();
> + gvir_config_domain_interface_filterref_parameter_set_name(glib_param, name);
> + gvir_config_domain_interface_filterref_parameter_set_value(glib_param, value);
> +
> + gvir_config_domain_interface_filterref_add_parameter(glib_fref, glib_param);
> + g_object_unref(glib_param);
> +
> + param_iter = param_iter->next;
> + }
> +
> + g_list_foreach(parameters, (GFunc)g_object_unref, NULL);
> + g_list_free(parameters);
> +
> + gvir_config_domain_interface_set_filterref(iface, glib_fref);
> + g_object_unref(glib_fref);
> + }
> +
This seems to be exactly the same code as what you added in
libvirt-sandbox-builder-container.c, this should go into a helper
(gvir_sandbox_config_network_filterref_get_config(), or
gvir_sandbox_builder_get_network_filterref_config() I think).
> g_object_unref(iface);
>
> tmp = tmp->next;
> diff --git a/libvirt-sandbox/libvirt-sandbox-config-network.c b/libvirt-sandbox/libvirt-sandbox-config-network.c
> index 7e7c015..555a360 100644
> --- a/libvirt-sandbox/libvirt-sandbox-config-network.c
> +++ b/libvirt-sandbox/libvirt-sandbox-config-network.c
> @@ -47,6 +47,7 @@ struct _GVirSandboxConfigNetworkPrivate
> gchar *mac;
> GList *routes;
> GList *addrs;
> + GVirSandboxConfigNetworkFilterref *filterref;
> };
>
> G_DEFINE_TYPE(GVirSandboxConfigNetwork, gvir_sandbox_config_network, G_TYPE_OBJECT);
> @@ -285,6 +286,38 @@ GList *gvir_sandbox_config_network_get_addresses(GVirSandboxConfigNetwork *confi
> }
>
> /**
> + * gvir_sandbox_config_network_set_filterref:
> + * @config: (transfer none): the sandbox network configuration
> + * @ref: (transfer none): the network filterref
> + *
> + * Set a network filterref for the given network.
> + */
> +void gvir_sandbox_config_network_set_filterref(GVirSandboxConfigNetwork *config,
> + GVirSandboxConfigNetworkFilterref *filterref)
> +{
> + GVirSandboxConfigNetworkPrivate *priv = config->priv;
> + if (priv->filterref)
> + g_object_unref(priv->filterref);
> + priv->filterref = g_object_ref(filterref);
This needs to be unref'ed in _finalize.
Rest of the patch looks good.
Christophe
Attachment:
pgpZII6Y1JkaE.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list