[libvirt] [libvirt-sandbox PATCH V2] Add filter support.

Christophe Fergeau cfergeau at redhat.com
Thu Nov 21 14:27:46 UTC 2013


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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131121/cfdddb24/attachment-0001.sig>


More information about the libvir-list mailing list