Re: [PATCH 1/3] bridge_driver: Return the live state info of a given virtual network

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/02/2015 09:08 AM, Lin Ma wrote:
> It constructs a temporary static config of the network, Obtains all of
> attached interfaces information through netcf, Then removes the config.
>
> Signed-off-by: Lin Ma <lma@xxxxxxxx>
> ---
>  include/libvirt/libvirt-network.h    |   1 +
>  src/Makefile.am                      |   3 +
>  src/network/bridge_driver.c          | 141 ++++++++++++++++++++++++++++++++++-
>  src/network/bridge_driver_platform.h |   7 ++
>  tests/Makefile.am                    |   4 +
>  5 files changed, 155 insertions(+), 1 deletion(-)
>
> diff --git a/include/libvirt/libvirt-network.h b/include/libvirt/libvirt-network.h
> index 308f27f..9b09546 100644
> --- a/include/libvirt/libvirt-network.h
> +++ b/include/libvirt/libvirt-network.h
> @@ -30,6 +30,7 @@
>  
>  typedef enum {
>      VIR_NETWORK_XML_INACTIVE = (1 << 0), /* dump inactive network information */
> +    VIR_NETWORK_XML_IFACE_ATTACHED = (1 << 1), /* dump current live state */
>  } virNetworkXMLFlags;
>  
>  /**
> diff --git a/src/Makefile.am b/src/Makefile.am
> index b41c6d4..d22ae7e 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -1412,6 +1412,9 @@ if WITH_NETWORK
>  noinst_LTLIBRARIES += libvirt_driver_network_impl.la
>  libvirt_driver_network_la_SOURCES =
>  libvirt_driver_network_la_LIBADD = libvirt_driver_network_impl.la
> +if WITH_NETCF
> +libvirt_driver_network_la_LIBADD += $(NETCF_LIBS)
> +endif WITH_NETCF
>  if WITH_DRIVER_MODULES
>  mod_LTLIBRARIES += libvirt_driver_network.la
>  libvirt_driver_network_la_LIBADD += ../gnulib/lib/libgnu.la \
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index c56e8f2..1e49e2e 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -3333,8 +3333,17 @@ static char *networkGetXMLDesc(virNetworkPtr net,
>      virNetworkObjPtr network;
>      virNetworkDefPtr def;
>      char *ret = NULL;
> +#ifdef WITH_NETCF
> +    struct netcf_if *iface = NULL;
> +    char *bridge = NULL;
> +    char *if_xml_tmp = NULL;
> +    xmlDocPtr xml = NULL;
> +    xmlXPathContextPtr ctxt = NULL;
> +    xmlXPathObjectPtr obj = NULL;
> +#endif
>  
> -    virCheckFlags(VIR_NETWORK_XML_INACTIVE, NULL);
> +    virCheckFlags(VIR_NETWORK_XML_INACTIVE |
> +                  VIR_NETWORK_XML_IFACE_ATTACHED, NULL);
>  
>      if (!(network = networkObjFromNetwork(net)))
>          return ret;
> @@ -3342,6 +3351,135 @@ static char *networkGetXMLDesc(virNetworkPtr net,
>      if (virNetworkGetXMLDescEnsureACL(net->conn, network->def) < 0)
>          goto cleanup;
>  
> +#ifdef WITH_NETCF
> +    if ((flags & VIR_NETWORK_XML_INACTIVE) && network->newDef) {
> +        def = network->newDef;
> +        ret = virNetworkDefFormat(def, flags);
> +    }
> +    else if (flags & VIR_NETWORK_XML_IFACE_ATTACHED) {
> +        if (!(network->def->bridge)) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("network '%s' does not have a bridge name."),
> +                           network->def->name);
> +            goto cleanup;
> +        }
> +        ignore_value(VIR_STRDUP(bridge, network->def->bridge));
> +
> +        if (virAsprintf(&if_xml_tmp,
> +                        "<interface type='bridge' name='%s'>"
> +                        "<start mode='none'/><bridge/></interface>",
> +                        bridge) < 0) {
> +         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("failed to generate temp xml for network"));
> +            goto cleanup;
> +        }
> +
> +        if (ncf_init(&driver->netcf, NULL) != 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("failed to init netcf"));
> +           goto cleanup;
> +        }
> +
> +        // create a temp bridge configuration file
> +        iface = ncf_define(driver->netcf, if_xml_tmp);

NACK. This is very much against what netcf (and the virInterface API)
was intended to do, and doing this is nearly *guaranteed* to completely
mess up NetworkManager, which monitors the ifcfg files that are created
by ncf_define.

This is one of the things that we regularly have to tell people that
they *shouldn't* do - the bridge devices created by libvirt must never
be referenced in a system config file; those are by definition
persistent (even though you're deleting it after a short time), and
immediately come in the radar of whatever host system daemon is managing
the persistent network device configs.

Also, I'm not sure I like having netcf called by the bridge driver.
Anyway, if all you want to do is check whether or not a network is in
use, an even more reasonable thing would be to simply look at the
connections attribute of the net (i.e. def->connections) and check that
it is 0 - every connection from a libvirt-managed guest is counted there
(and it's re-computed every time libvirtd is restarted), and anybody who
connects to a libvirt-created bridge without going through libvirt to do
it deserves what they get.


> +        if (!iface) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("failed to define the temp bridge %s"), bridge);
> +            ncf_close(driver->netcf);
> +            goto cleanup;
> +        }
> +
> +        ret = ncf_if_xml_state(iface);
> +        if (!ret) {
> +            virReportError(VIR_ERR_XML_ERROR, "%s",
> +                           _("could not get bridge XML description"));
> +            ncf_if_free(iface);
> +            ncf_close(driver->netcf);
> +            goto cleanup;
> +        }
> +
> +        // remove the temp bridge configuration file
> +        if (ncf_if_undefine(iface) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("failed to undefine the temp bridge %s"), bridge);
> +            ncf_if_free(iface);
> +            ncf_close(driver->netcf);
> +            ret = NULL;
> +            goto cleanup;
> +        }
> +        ncf_if_free(iface);
> +        ncf_close(driver->netcf);
> +
> +        // remove the dummp tap interface section from the result
> +        if (network->def->mac_specified) {
> +            char *macTapIfName = networkBridgeDummyNicName(network->def->bridge);
> +            if (macTapIfName) {
> +                char mac[VIR_MAC_STRING_BUFLEN];
> +                xmlNodePtr cur = NULL, matchNode = NULL;
> +                xmlChar *br_xml = NULL;
> +                int br_xml_size;
> +                char buf[64];
> +	            size_t i;
> +                int diff_mac;
> +                virMacAddrFormat(&network->def->mac, mac);
> +                snprintf(buf, sizeof(buf), "./bridge/interface[@name='%s']",
> +                         macTapIfName);
> +                if (!(xml = virXMLParseStringCtxt(ret,
> +                                                  _("(bridge interface "
> +                                                    "definition)"), &ctxt))) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   ("Failed to parse network configuration"));
> +                    VIR_FREE(macTapIfName);
> +                    ret = NULL;
> +                    goto cleanup;
> +                }
> +                obj = xmlXPathEval(BAD_CAST buf, ctxt);
> +                if (obj == NULL || obj->type != XPATH_NODESET ||
> +                    obj->nodesetval == NULL || obj->nodesetval->nodeNr == 0) {
> +	                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   _("No interface found whose name is %s"),
> +                                   macTapIfName);
> +                    VIR_FREE(macTapIfName);
> +	                ret = NULL;
> +                    goto cleanup;
> +                }
> +                VIR_FREE(macTapIfName);
> +                for (i = 0; i < obj->nodesetval->nodeNr; i++) {
> +                    cur = obj->nodesetval->nodeTab[i]->children;
> +                    while (cur != NULL) {
> +                        if (cur->type == XML_ELEMENT_NODE &&
> +                            xmlStrEqual(cur->name, BAD_CAST "mac")) {
> +                            char *tmp_mac = virXMLPropString(cur, "address");
> +                            diff_mac = virMacAddrCompare(tmp_mac, mac);
> +                            VIR_FREE(tmp_mac);
> +                            if (!diff_mac) {
> +                                matchNode = obj->nodesetval->nodeTab[i];
> +                                xmlUnlinkNode(matchNode);
> +                                break;
> +                            }
> +                        }
> +                        cur = cur->next;
> +                    }
> +                }
> +                xmlDocDumpMemory(xml, &br_xml, &br_xml_size);
> +                ret = (char *)br_xml;
> +            }
> +        }
> +    }
> +    else {
> +        def = network->def;
> +        ret = virNetworkDefFormat(def, flags);
> +    }
> +
> + cleanup:
> +    xmlXPathFreeObject(obj);
> +    xmlXPathFreeContext(ctxt);
> +    xmlFreeDoc(xml);
> +    VIR_FREE(if_xml_tmp);
> +    VIR_FREE(bridge);
> +    if (network)
> +        virNetworkObjUnlock(network);
> +#else
>      if ((flags & VIR_NETWORK_XML_INACTIVE) && network->newDef)
>          def = network->newDef;
>      else
> @@ -3352,6 +3490,7 @@ static char *networkGetXMLDesc(virNetworkPtr net,
>   cleanup:
>      if (network)
>          virNetworkObjUnlock(network);
> +#endif
>      return ret;
>  }
>  
> diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h
> index 1e8264a..43ea1c3 100644
> --- a/src/network/bridge_driver_platform.h
> +++ b/src/network/bridge_driver_platform.h
> @@ -24,6 +24,9 @@
>  #ifndef __VIR_BRIDGE_DRIVER_PLATFORM_H__
>  # define __VIR_BRIDGE_DRIVER_PLATFORM_H__
>  
> +#ifdef WITH_NETCF
> +# include <netcf.h>
> +#endif
>  # include "internal.h"
>  # include "virthread.h"
>  # include "virdnsmasq.h"
> @@ -34,6 +37,10 @@
>  struct _virNetworkDriverState {
>      virMutex lock;
>  
> +#ifdef WITH_NETCF
> +    struct netcf *netcf;
> +#endif
> +
>      virNetworkObjList networks;
>  
>      char *networkConfigDir;
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 938270c..0662337 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -67,6 +67,10 @@ LDADDS = \
>  	$(GNULIB_LIBS) \
>  	../src/libvirt.la
>  
> +if WITH_NETCF
> +LDADDS += $(NETCF_LIBS)
> +endif WITH_NETCF
> +
>  EXTRA_DIST =		\
>  	bhyvexml2argvdata \
>  	bhyvexml2xmloutdata \

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]