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

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