On Thu, Nov 21, 2013 at 03:27:46PM +0100, Christophe Fergeau wrote:
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).
Yes, I'm actually not entirely sure how to handle that.. there is a lot of duplicate code between the two but a few things that are different. I think we might have to refactor that part but I think maybe we need a builder_common.c or something. Ian [snip]