It only accepts a virObjecLockable, not a virObjecRWLockable Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/util/virobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virobject.c b/src/util/virobject.c index 9fb0328d58..a597ff4f54 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -408,7 +408,7 @@ virObjectGetRWLockableObj(void *anyobj) /** * virObjectLock: - * @anyobj: any instance of virObjectLockable or virObjectRWLockable + * @anyobj: any instance of virObjectLockable * * Acquire a lock on @anyobj. The lock must be released by * virObjectUnlock. -- 2.17.0

On 05/15/2018 01:43 PM, Daniel P. Berrangé wrote: I haven't had time to look at this in detail yet, or to really think about the comment I'm going to make, but I wanted to be sure I said it right away in case there are any public API implications By now we all know the horror by which OpenStack's networking creates a separate bridge device, and connects the guest's tap device to that bridge so that (in addition to other reasons) there is a place for libvirt's nwfilter rules to attach (what they *really* want to connect to is an Open vSwitch, but ipfilter rules aren't in the data path when a tap device is connected to OVS). This atrocity could be avoided if nwfilter supported OVN (OVS's ipfilter analog) directly. In order to support it though, nwfilter would need to know more details about the network device that it's adding rules for. (because some guests may be connected to OVS and others may be connected to a standard host bridge (or nothing at all) we can't just have a system-wide config to decide). I can't recall if there is a reasonable API to figure out what a tap device is connected to, but I think such a thing may not exist and, if so, we'll likely need to send some sort of indicator in the NWFilterBinding XML. It *might* be simpler / more futureproof if we included the <virtualport> element that is already in the domain XML <interface> information - that's currently what we use to decide how to connect the tap device; hopefully in the future it will continue to conain everything that's needed (if we think that's inadequate, we could just go for broke and include the entire <interface>, but that's probably overkill. (although..... - thinking about the potential case where some SRIOV network card supported some sort of filtering, if we sent the entire <interface>, we would know that it was a hostdev...) I started typing this wondering if the C API might need any change, but now that I've typed this, I realize it would only require additions to the XML, which can always be done later, so    nevermind (kind of) </EmilyLittela>

Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- docs/schemas/domaincommon.rng | 27 +----------------- docs/schemas/nwfilter.rng | 29 +------------------ docs/schemas/nwfilter_params.rng | 32 +++++++++++++++++++++ docs/schemas/nwfilterbinding.rng | 49 ++++++++++++++++++++++++++++++++ tests/virschematest.c | 1 + 5 files changed, 84 insertions(+), 54 deletions(-) create mode 100644 docs/schemas/nwfilter_params.rng create mode 100644 docs/schemas/nwfilterbinding.rng diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index ff539607cc..f1661d29b6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5,6 +5,7 @@ <include href='storagecommon.rng'/> <include href='networkcommon.rng'/> <include href='cputypes.rng'/> + <include href='nwfilter_params.rng'/> <!-- description and title element, may be placed anywhere under the root @@ -5282,22 +5283,6 @@ </element> </define> - <define name="filterref-node-attributes"> - <attribute name="filter"> - <data type="NCName"/> - </attribute> - <zeroOrMore> - <element name="parameter"> - <attribute name="name"> - <ref name="filter-param-name"/> - </attribute> - <attribute name="value"> - <ref name="filter-param-value"/> - </attribute> - </element> - </zeroOrMore> - </define> - <define name="deviceBoot"> <element name="boot"> <attribute name="order"> @@ -6024,16 +6009,6 @@ <param name="pattern">[a-zA-Z0-9_\.\+\-/]+</param> </data> </define> - <define name="filter-param-name"> - <data type="string"> - <param name="pattern">[a-zA-Z0-9_]+</param> - </data> - </define> - <define name="filter-param-value"> - <data type="string"> - <param name="pattern">[a-zA-Z0-9_\.:]+</param> - </data> - </define> <define name="spaprvioReg"> <data type="string"> <param name="pattern">(0x)?[0-9a-fA-F]{1,16}</param> diff --git a/docs/schemas/nwfilter.rng b/docs/schemas/nwfilter.rng index cca6ff2954..05a79da112 100644 --- a/docs/schemas/nwfilter.rng +++ b/docs/schemas/nwfilter.rng @@ -1,6 +1,7 @@ <?xml version="1.0" encoding="UTF-8"?> <grammar ns="" xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> <include href='basictypes.rng'/> + <include href='nwfilter_params.rng'/> <start> <ref name="filter"/> </start> @@ -245,22 +246,6 @@ </optional> </define> - <define name="filterref-node-attributes"> - <attribute name="filter"> - <data type="NCName"/> - </attribute> - <zeroOrMore> - <element name="parameter"> - <attribute name="name"> - <ref name="filter-param-name"/> - </attribute> - <attribute name="value"> - <ref name="filter-param-value"/> - </attribute> - </element> - </zeroOrMore> - </define> - <define name="rule-node-attributes"> <attribute name="action"> <ref name='action-type'/> @@ -937,18 +922,6 @@ </choice> </define> - <define name="filter-param-name"> - <data type="string"> - <param name="pattern">[a-zA-Z0-9_]+</param> - </data> - </define> - - <define name="filter-param-value"> - <data type="string"> - <param name="pattern">[a-zA-Z0-9_\.:]+</param> - </data> - </define> - <define name='action-type'> <choice> <value>drop</value> diff --git a/docs/schemas/nwfilter_params.rng b/docs/schemas/nwfilter_params.rng new file mode 100644 index 0000000000..a3e7b35b6f --- /dev/null +++ b/docs/schemas/nwfilter_params.rng @@ -0,0 +1,32 @@ +<?xml version="1.0"?> +<!-- network-related definitions used in multiple grammars --> +<grammar xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> + + <define name="filterref-node-attributes"> + <attribute name="filter"> + <data type="NCName"/> + </attribute> + <zeroOrMore> + <element name="parameter"> + <attribute name="name"> + <ref name="filter-param-name"/> + </attribute> + <attribute name="value"> + <ref name="filter-param-value"/> + </attribute> + </element> + </zeroOrMore> + </define> + + <define name="filter-param-name"> + <data type="string"> + <param name="pattern">[a-zA-Z0-9_]+</param> + </data> + </define> + <define name="filter-param-value"> + <data type="string"> + <param name="pattern">[a-zA-Z0-9_\.:]+</param> + </data> + </define> + +</grammar> diff --git a/docs/schemas/nwfilterbinding.rng b/docs/schemas/nwfilterbinding.rng new file mode 100644 index 0000000000..dcfc67fdd6 --- /dev/null +++ b/docs/schemas/nwfilterbinding.rng @@ -0,0 +1,49 @@ +<?xml version="1.0"?> +<grammar xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> + <!-- domain-related definitions used in multiple grammars --> + <include href='basictypes.rng'/> + <include href='nwfilter_params.rng'/> + + <start> + <ref name="filterbinding"/> + </start> + + <define name="filterbinding"> + <element name="filterbinding"> + <interleave> + <element name="owner"> + <element name="name"> + <text/> + </element> + <element name="uuid"> + <ref name="UUID"/> + </element> + </element> + + <element name="portdev"> + <attribute name="name"/> + <empty/> + </element> + + <optional> + <element name="linkdev"> + <attribute name="name"/> + <empty/> + </element> + </optional> + + <element name="mac"> + <attribute name="address"> + <ref name="uniMacAddr"/> + </attribute> + <empty/> + </element> + + <element name="filterref"> + <ref name="filterref-node-attributes"/> + </element> + </interleave> + </element> + </define> + +</grammar> diff --git a/tests/virschematest.c b/tests/virschematest.c index 2d35833919..5bae022111 100644 --- a/tests/virschematest.c +++ b/tests/virschematest.c @@ -230,6 +230,7 @@ mymain(void) "networkxml2xmlout", "networkxml2confdata"); DO_TEST_DIR("nodedev.rng", "nodedevschemadata"); DO_TEST_DIR("nwfilter.rng", "nwfilterxml2xmlout"); + DO_TEST_DIR("nwfilterbinding.rng", "virnwfilterbindingxml2xmldata"); DO_TEST_DIR("secret.rng", "secretxml2xmlin"); DO_TEST_DIR("storagepool.rng", "storagepoolxml2xmlin", "storagepoolxml2xmlout", "storagepoolschemadata"); -- 2.17.0

When the daemons are split there will need to be a way for the virt drivers and/or network driver to create and delete bindings between network ports and network filters. This defines a set of public APIs that are suitable for managing this facility. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- include/libvirt/libvirt-nwfilter.h | 39 ++++ include/libvirt/virterror.h | 2 + src/datatypes.c | 67 +++++++ src/datatypes.h | 31 +++ src/driver-nwfilter.h | 30 +++ src/libvirt-nwfilter.c | 305 +++++++++++++++++++++++++++++ src/libvirt_private.syms | 1 + src/libvirt_public.syms | 13 ++ src/util/virerror.c | 12 ++ 9 files changed, 500 insertions(+) diff --git a/include/libvirt/libvirt-nwfilter.h b/include/libvirt/libvirt-nwfilter.h index 9f01c175a9..20e6d1ff9a 100644 --- a/include/libvirt/libvirt-nwfilter.h +++ b/include/libvirt/libvirt-nwfilter.h @@ -43,6 +43,23 @@ typedef struct _virNWFilter virNWFilter; */ typedef virNWFilter *virNWFilterPtr; +/** + * virNWFilterBinding: + * + * a virNWFilterBinding is a private structure representing a network + * filter binding to a port + */ +typedef struct _virNWFilterBinding virNWFilterBinding; + +/** + * virNWFilterBindingPtr: + * + * a virNWFilterBindingPtr is pointer to a virNWFilterBinding private + * structure, this is the type used to reference a network filter + * port binding in the API. + */ +typedef virNWFilterBinding *virNWFilterBindingPtr; + /* * List NWFilters @@ -92,4 +109,26 @@ int virNWFilterGetUUIDString (virNWFilterPtr nwfilter, char * virNWFilterGetXMLDesc (virNWFilterPtr nwfilter, unsigned int flags); + +virNWFilterBindingPtr virNWFilterBindingLookupByPortDev(virConnectPtr conn, + const char *portdev); + +const char * virNWFilterBindingGetPortDev(virNWFilterBindingPtr binding); +const char * virNWFilterBindingGetFilterName(virNWFilterBindingPtr binding); + +int virConnectListAllNWFilterBindings(virConnectPtr conn, + virNWFilterBindingPtr **bindings, + unsigned int flags); + +virNWFilterBindingPtr virNWFilterBindingCreateXML(virConnectPtr conn, + const char *xml, + unsigned int flags); + +char * virNWFilterBindingGetXMLDesc(virNWFilterBindingPtr binding, + unsigned int flags); + +int virNWFilterBindingDelete(virNWFilterBindingPtr binding); +int virNWFilterBindingRef(virNWFilterBindingPtr binding); +int virNWFilterBindingFree(virNWFilterBindingPtr binding); + #endif /* __VIR_LIBVIRT_NWFILTER_H__ */ diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 3e7c7a02c7..02bc141f1c 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -321,6 +321,8 @@ typedef enum { to guest-sync command (DEPRECATED)*/ VIR_ERR_LIBSSH = 98, /* error in libssh transport driver */ VIR_ERR_DEVICE_MISSING = 99, /* fail to find the desired device */ + VIR_ERR_INVALID_NWFILTER_BINDING = 100, /* invalid nwfilter binding */ + VIR_ERR_NO_NWFILTER_BINDING = 101, /* no nwfilter binding */ } virErrorNumber; /** diff --git a/src/datatypes.c b/src/datatypes.c index 09b8eea5a2..878a1c5b5f 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -41,6 +41,7 @@ virClassPtr virInterfaceClass; virClassPtr virNetworkClass; virClassPtr virNodeDeviceClass; virClassPtr virNWFilterClass; +virClassPtr virNWFilterBindingClass; virClassPtr virSecretClass; virClassPtr virStreamClass; virClassPtr virStorageVolClass; @@ -54,6 +55,7 @@ static void virInterfaceDispose(void *obj); static void virNetworkDispose(void *obj); static void virNodeDeviceDispose(void *obj); static void virNWFilterDispose(void *obj); +static void virNWFilterBindingDispose(void *obj); static void virSecretDispose(void *obj); static void virStreamDispose(void *obj); static void virStorageVolDispose(void *obj); @@ -89,6 +91,7 @@ virDataTypesOnceInit(void) DECLARE_CLASS(virNetwork); DECLARE_CLASS(virNodeDevice); DECLARE_CLASS(virNWFilter); + DECLARE_CLASS(virNWFilterBinding); DECLARE_CLASS(virSecret); DECLARE_CLASS(virStream); DECLARE_CLASS(virStorageVol); @@ -830,6 +833,70 @@ virNWFilterDispose(void *obj) } +/** + * virGetNWFilterBinding: + * @conn: the hypervisor connection + * @portdev: pointer to the network filter port device name + * @filtername: name of the network filter + * + * Allocates a new network filter binding object. When the object is no longer + * needed, virObjectUnref() must be called in order to not leak data. + * + * Returns a pointer to the network filter binding object, or NULL on error. + */ +virNWFilterBindingPtr +virGetNWFilterBinding(virConnectPtr conn, const char *portdev, + const char *filtername) +{ + virNWFilterBindingPtr ret = NULL; + + if (virDataTypesInitialize() < 0) + return NULL; + + virCheckConnectGoto(conn, error); + virCheckNonNullArgGoto(portdev, error); + + if (!(ret = virObjectNew(virNWFilterBindingClass))) + goto error; + + if (VIR_STRDUP(ret->portdev, portdev) < 0) + goto error; + + if (VIR_STRDUP(ret->filtername, filtername) < 0) + goto error; + + ret->conn = virObjectRef(conn); + + return ret; + + error: + virObjectUnref(ret); + return NULL; +} + + +/** + * virNWFilterBindingDispose: + * @obj: the network filter binding to release + * + * Unconditionally release all memory associated with a nwfilter binding. + * The nwfilter binding object must not be used once this method returns. + * + * It will also unreference the associated connection object, + * which may also be released if its ref count hits zero. + */ +static void +virNWFilterBindingDispose(void *obj) +{ + virNWFilterBindingPtr binding = obj; + + VIR_DEBUG("release binding %p %s", binding, binding->portdev); + + VIR_FREE(binding->portdev); + virObjectUnref(binding->conn); +} + + /** * virGetDomainSnapshot: * @domain: the domain to snapshot diff --git a/src/datatypes.h b/src/datatypes.h index 192c86be80..e1b38706dc 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -36,6 +36,7 @@ extern virClassPtr virInterfaceClass; extern virClassPtr virNetworkClass; extern virClassPtr virNodeDeviceClass; extern virClassPtr virNWFilterClass; +extern virClassPtr virNWFilterBindingClass; extern virClassPtr virSecretClass; extern virClassPtr virStreamClass; extern virClassPtr virStorageVolClass; @@ -277,6 +278,20 @@ extern virClassPtr virAdmClientClass; } \ } while (0) +# define virCheckNWFilterBindingReturn(obj, retval) \ + do { \ + virNWFilterBindingPtr _nw = (obj); \ + if (!virObjectIsClass(_nw, virNWFilterBindingClass) || \ + !virObjectIsClass(_nw->conn, virConnectClass)) { \ + virReportErrorHelper(VIR_FROM_NWFILTER, \ + VIR_ERR_INVALID_NWFILTER_BINDING, \ + __FILE__, __FUNCTION__, __LINE__, \ + __FUNCTION__); \ + virDispatchError(NULL); \ + return retval; \ + } \ + } while (0) + # define virCheckDomainSnapshotReturn(obj, retval) \ do { \ virDomainSnapshotPtr _snap = (obj); \ @@ -676,6 +691,19 @@ struct _virNWFilter { }; +/** +* _virNWFilterBinding: +* +* Internal structure associated to a network filter port binding +*/ +struct _virNWFilterBinding { + virObject parent; + virConnectPtr conn; /* pointer back to the connection */ + char *portdev; /* the network filter port device name */ + char *filtername; /* the network filter name */ +}; + + /* * Helper APIs for allocating new object instances */ @@ -712,6 +740,9 @@ virStreamPtr virGetStream(virConnectPtr conn); virNWFilterPtr virGetNWFilter(virConnectPtr conn, const char *name, const unsigned char *uuid); +virNWFilterBindingPtr virGetNWFilterBinding(virConnectPtr conn, + const char *portdev, + const char *filtername); virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain, const char *name); diff --git a/src/driver-nwfilter.h b/src/driver-nwfilter.h index cb49542f92..2c3e480a32 100644 --- a/src/driver-nwfilter.h +++ b/src/driver-nwfilter.h @@ -57,6 +57,31 @@ typedef char * (*virDrvNWFilterGetXMLDesc)(virNWFilterPtr nwfilter, unsigned int flags); +typedef virNWFilterBindingPtr +(*virDrvNWFilterBindingLookupByPortDev)(virConnectPtr conn, + const char *portdev); + +typedef int +(*virDrvConnectListAllNWFilterBindings)(virConnectPtr conn, + virNWFilterBindingPtr **bindings, + unsigned int flags); + +typedef virNWFilterBindingPtr +(*virDrvNWFilterBindingCreateXML)(virConnectPtr conn, + const char *xml, + unsigned int flags); + +typedef char * +(*virDrvNWFilterBindingGetXMLDesc)(virNWFilterBindingPtr binding, + unsigned int flags); + +typedef int +(*virDrvNWFilterBindingDelete)(virNWFilterBindingPtr binding); +typedef int +(*virDrvNWFilterBindingRef)(virNWFilterBindingPtr binding); +typedef int +(*virDrvNWFilterBindingFree)(virNWFilterBindingPtr binding); + typedef struct _virNWFilterDriver virNWFilterDriver; typedef virNWFilterDriver *virNWFilterDriverPtr; @@ -77,6 +102,11 @@ struct _virNWFilterDriver { virDrvNWFilterDefineXML nwfilterDefineXML; virDrvNWFilterUndefine nwfilterUndefine; virDrvNWFilterGetXMLDesc nwfilterGetXMLDesc; + virDrvConnectListAllNWFilterBindings connectListAllNWFilterBindings; + virDrvNWFilterBindingLookupByPortDev nwfilterBindingLookupByPortDev; + virDrvNWFilterBindingCreateXML nwfilterBindingCreateXML; + virDrvNWFilterBindingDelete nwfilterBindingDelete; + virDrvNWFilterBindingGetXMLDesc nwfilterBindingGetXMLDesc; }; diff --git a/src/libvirt-nwfilter.c b/src/libvirt-nwfilter.c index 948c30deef..e572d46c18 100644 --- a/src/libvirt-nwfilter.c +++ b/src/libvirt-nwfilter.c @@ -513,3 +513,308 @@ virNWFilterRef(virNWFilterPtr nwfilter) virObjectRef(nwfilter); return 0; } + + +/** + * virConnectListAllNWFilterBindings: + * @conn: Pointer to the hypervisor connection. + * @bindings: Pointer to a variable to store the array containing the network + * filter objects or NULL if the list is not required (just returns + * number of network filters). + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Collect the list of network filters, and allocate an array to store those + * objects. + * + * Returns the number of network filters found or -1 and sets @filters to NULL + * in case of error. On success, the array stored into @filters is guaranteed to + * have an extra allocated element set to NULL but not included in the return count, + * to make iteration easier. The caller is responsible for calling + * virNWFilterFree() on each array element, then calling free() on @filters. + */ +int +virConnectListAllNWFilterBindings(virConnectPtr conn, + virNWFilterBindingPtr **bindings, + unsigned int flags) +{ + VIR_DEBUG("conn=%p, bindings=%p, flags=0x%x", conn, bindings, flags); + + virResetLastError(); + + if (bindings) + *bindings = NULL; + + virCheckConnectReturn(conn, -1); + + if (conn->nwfilterDriver && + conn->nwfilterDriver->connectListAllNWFilterBindings) { + int ret; + ret = conn->nwfilterDriver->connectListAllNWFilterBindings(conn, bindings, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(conn); + return -1; +} + + +/** + * virNWFilterBindingLookupByPortDev: + * @conn: pointer to the hypervisor connection + * @portdev: name for the network port device + * + * Try to lookup a network filter binding on the given hypervisor based + * on network port device name. + * + * virNWFilterBindingFree should be used to free the resources after the + * binding object is no longer needed. + * + * Returns a new binding object or NULL in case of failure. If the + * network filter cannot be found, then VIR_ERR_NO_NWFILTER_BINDING + * error is raised. + */ +virNWFilterBindingPtr +virNWFilterBindingLookupByPortDev(virConnectPtr conn, const char *portdev) +{ + VIR_DEBUG("conn=%p, name=%s", conn, NULLSTR(portdev)); + + virResetLastError(); + + virCheckConnectReturn(conn, NULL); + virCheckNonNullArgGoto(portdev, error); + + if (conn->nwfilterDriver && conn->nwfilterDriver->nwfilterBindingLookupByPortDev) { + virNWFilterBindingPtr ret; + ret = conn->nwfilterDriver->nwfilterBindingLookupByPortDev(conn, portdev); + if (!ret) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(conn); + return NULL; +} + + +/** + * virNWFilterBindingFree: + * @binding: a binding object + * + * Free the binding object. The running instance is kept alive. + * The data structure is freed and should not be used thereafter. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virNWFilterBindingFree(virNWFilterBindingPtr binding) +{ + VIR_DEBUG("binding=%p", binding); + + virResetLastError(); + + virCheckNWFilterBindingReturn(binding, -1); + + virObjectUnref(binding); + return 0; +} + + +/** + * virNWFilterBindingGetPortDev: + * @binding: a binding object + * + * Get the port dev name for the network filter binding + * + * Returns a pointer to the name or NULL, the string need not be deallocated + * its lifetime will be the same as the binding object. + */ +const char * +virNWFilterBindingGetPortDev(virNWFilterBindingPtr binding) +{ + VIR_DEBUG("binding=%p", binding); + + virResetLastError(); + + virCheckNWFilterBindingReturn(binding, NULL); + + return binding->portdev; +} + + +/** + * virNWFilterBindingGetFilterName: + * @binding: a binding object + * + * Get the filter name for the network filter binding + * + * Returns a pointer to the name or NULL, the string need not be deallocated + * its lifetime will be the same as the binding object. + */ +const char * +virNWFilterBindingGetFilterName(virNWFilterBindingPtr binding) +{ + VIR_DEBUG("binding=%p", binding); + + virResetLastError(); + + virCheckNWFilterBindingReturn(binding, NULL); + + return binding->filtername; +} + + +/** + * virNWFilterBindingCreateXML: + * @conn: pointer to the hypervisor connection + * @xml: an XML description of the binding + * @flags: currently unused, pass 0 + * + * Define a new network filter, based on an XML description + * similar to the one returned by virNWFilterGetXMLDesc() + * + * virNWFilterFree should be used to free the resources after the + * binding object is no longer needed. + * + * Returns a new binding object or NULL in case of failure + */ +virNWFilterBindingPtr +virNWFilterBindingCreateXML(virConnectPtr conn, const char *xml, unsigned int flags) +{ + VIR_DEBUG("conn=%p, xml=%s", conn, NULLSTR(xml)); + + virResetLastError(); + + virCheckConnectReturn(conn, NULL); + virCheckNonNullArgGoto(xml, error); + virCheckReadOnlyGoto(conn->flags, error); + + if (conn->nwfilterDriver && conn->nwfilterDriver->nwfilterBindingCreateXML) { + virNWFilterBindingPtr ret; + ret = conn->nwfilterDriver->nwfilterBindingCreateXML(conn, xml, flags); + if (!ret) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(conn); + return NULL; +} + + +/** + * virNWFilterBindingDelete: + * @binding: a binding object + * + * Delete the binding object. This does not free the + * associated virNWFilterBindingPtr object. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virNWFilterBindingDelete(virNWFilterBindingPtr binding) +{ + virConnectPtr conn; + VIR_DEBUG("binding=%p", binding); + + virResetLastError(); + + virCheckNWFilterBindingReturn(binding, -1); + conn = binding->conn; + + virCheckReadOnlyGoto(conn->flags, error); + + if (conn->nwfilterDriver && conn->nwfilterDriver->nwfilterBindingDelete) { + int ret; + ret = conn->nwfilterDriver->nwfilterBindingDelete(binding); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(binding->conn); + return -1; +} + + +/** + * virNWFilterBindingGetXMLDesc: + * @binding: a binding object + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Provide an XML description of the network filter. The description may be + * reused later to redefine the network filter with virNWFilterCreateXML(). + * + * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error. + * the caller must free() the returned value. + */ +char * +virNWFilterBindingGetXMLDesc(virNWFilterBindingPtr binding, unsigned int flags) +{ + virConnectPtr conn; + VIR_DEBUG("binding=%p, flags=0x%x", binding, flags); + + virResetLastError(); + + virCheckNWFilterBindingReturn(binding, NULL); + conn = binding->conn; + + if (conn->nwfilterDriver && conn->nwfilterDriver->nwfilterBindingGetXMLDesc) { + char *ret; + ret = conn->nwfilterDriver->nwfilterBindingGetXMLDesc(binding, flags); + if (!ret) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(binding->conn); + return NULL; +} + + +/** + * virNWFilterBindingRef: + * @binding: the binding to hold a reference on + * + * Increment the reference count on the binding. For each + * additional call to this method, there shall be a corresponding + * call to virNWFilterFree to release the reference count, once + * the caller no longer needs the reference to this object. + * + * This method is typically useful for applications where multiple + * threads are using a connection, and it is required that the + * connection remain open until all threads have finished using + * it. ie, each new thread using an binding would increment + * the reference count. + * + * Returns 0 in case of success, -1 in case of failure. + */ +int +virNWFilterBindingRef(virNWFilterBindingPtr binding) +{ + VIR_DEBUG("binding=%p refs=%d", binding, + binding ? binding->parent.u.s.refs : 0); + + virResetLastError(); + + virCheckNWFilterBindingReturn(binding, -1); + + virObjectRef(binding); + return 0; +} diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 03145c70d5..0ce685b6f2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1193,6 +1193,7 @@ virGetInterface; virGetNetwork; virGetNodeDevice; virGetNWFilter; +virGetNWFilterBinding; virGetSecret; virGetStoragePool; virGetStorageVol; diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 95df3a0dbc..cc73fd8528 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -785,4 +785,17 @@ LIBVIRT_4.1.0 { virStoragePoolLookupByTargetPath; } LIBVIRT_3.9.0; +LIBVIRT_4.4.0 { + global: + virNWFilterBindingLookupByPortDev; + virConnectListAllNWFilterBindings; + virNWFilterBindingCreateXML; + virNWFilterBindingGetXMLDesc; + virNWFilterBindingDelete; + virNWFilterBindingRef; + virNWFilterBindingFree; + virNWFilterBindingGetPortDev; + virNWFilterBindingGetFilterName; +} LIBVIRT_4.1.0; + # .... define new API here using predicted next version number .... diff --git a/src/util/virerror.c b/src/util/virerror.c index c000b00436..074e217849 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1459,6 +1459,18 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("device not found: %s"); break; + case VIR_ERR_INVALID_NWFILTER_BINDING: + if (info == NULL) + errmsg = _("Invalid network filter binding"); + else + errmsg = _("Invalid network filter binding: %s"); + break; + case VIR_ERR_NO_NWFILTER_BINDING: + if (info == NULL) + errmsg = _("Network filter binding not found"); + else + errmsg = _("Network filter binding not found: %s"); + break; } return errmsg; } -- 2.17.0

Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/access/viraccessdriver.h | 5 ++++ src/access/viraccessdrivernop.c | 10 ++++++++ src/access/viraccessdriverpolkit.c | 21 ++++++++++++++++ src/access/viraccessdriverstack.c | 24 ++++++++++++++++++ src/access/viraccessmanager.c | 15 ++++++++++++ src/access/viraccessmanager.h | 5 ++++ src/access/viraccessperm.c | 7 +++++- src/access/viraccessperm.h | 39 ++++++++++++++++++++++++++++++ src/rpc/gendispatch.pl | 3 ++- 9 files changed, 127 insertions(+), 2 deletions(-) diff --git a/src/access/viraccessdriver.h b/src/access/viraccessdriver.h index e3050b6439..3b25f36cab 100644 --- a/src/access/viraccessdriver.h +++ b/src/access/viraccessdriver.h @@ -47,6 +47,10 @@ typedef int (*virAccessDriverCheckNWFilterDrv)(virAccessManagerPtr manager, const char *driverName, virNWFilterDefPtr nwfilter, virAccessPermNWFilter av); +typedef int (*virAccessDriverCheckNWFilterBindingDrv)(virAccessManagerPtr manager, + const char *driverName, + virNWFilterBindingDefPtr binding, + virAccessPermNWFilterBinding av); typedef int (*virAccessDriverCheckSecretDrv)(virAccessManagerPtr manager, const char *driverName, virSecretDefPtr secret, @@ -80,6 +84,7 @@ struct _virAccessDriver { virAccessDriverCheckNetworkDrv checkNetwork; virAccessDriverCheckNodeDeviceDrv checkNodeDevice; virAccessDriverCheckNWFilterDrv checkNWFilter; + virAccessDriverCheckNWFilterBindingDrv checkNWFilterBinding; virAccessDriverCheckSecretDrv checkSecret; virAccessDriverCheckStoragePoolDrv checkStoragePool; virAccessDriverCheckStorageVolDrv checkStorageVol; diff --git a/src/access/viraccessdrivernop.c b/src/access/viraccessdrivernop.c index 86ceef37c2..98ef9206c5 100644 --- a/src/access/viraccessdrivernop.c +++ b/src/access/viraccessdrivernop.c @@ -75,6 +75,15 @@ virAccessDriverNopCheckNWFilter(virAccessManagerPtr manager ATTRIBUTE_UNUSED, return 1; /* Allow */ } +static int +virAccessDriverNopCheckNWFilterBinding(virAccessManagerPtr manager ATTRIBUTE_UNUSED, + const char *driverName ATTRIBUTE_UNUSED, + virNWFilterBindingDefPtr binding ATTRIBUTE_UNUSED, + virAccessPermNWFilterBinding perm ATTRIBUTE_UNUSED) +{ + return 1; /* Allow */ +} + static int virAccessDriverNopCheckSecret(virAccessManagerPtr manager ATTRIBUTE_UNUSED, const char *driverName ATTRIBUTE_UNUSED, @@ -112,6 +121,7 @@ virAccessDriver accessDriverNop = { .checkNetwork = virAccessDriverNopCheckNetwork, .checkNodeDevice = virAccessDriverNopCheckNodeDevice, .checkNWFilter = virAccessDriverNopCheckNWFilter, + .checkNWFilterBinding = virAccessDriverNopCheckNWFilterBinding, .checkSecret = virAccessDriverNopCheckSecret, .checkStoragePool = virAccessDriverNopCheckStoragePool, .checkStorageVol = virAccessDriverNopCheckStorageVol, diff --git a/src/access/viraccessdriverpolkit.c b/src/access/viraccessdriverpolkit.c index 48a83f66d7..6954d74a15 100644 --- a/src/access/viraccessdriverpolkit.c +++ b/src/access/viraccessdriverpolkit.c @@ -276,6 +276,26 @@ virAccessDriverPolkitCheckNWFilter(virAccessManagerPtr manager, attrs); } +static int +virAccessDriverPolkitCheckNWFilterBinding(virAccessManagerPtr manager, + const char *driverName, + virNWFilterBindingDefPtr binding, + virAccessPermNWFilterBinding perm) +{ + const char *attrs[] = { + "connect_driver", driverName, + "nwfilter_binding_portdev", binding->portdevname, + "nwfilter_binding_linkdev", binding->linkdevname, + "nwfilter_binding_filter", binding->filter, + NULL, + }; + + return virAccessDriverPolkitCheck(manager, + "nwfilter_binding", + virAccessPermNWFilterBindingTypeToString(perm), + attrs); +} + static int virAccessDriverPolkitCheckSecret(virAccessManagerPtr manager, const char *driverName, @@ -409,6 +429,7 @@ virAccessDriver accessDriverPolkit = { .checkNetwork = virAccessDriverPolkitCheckNetwork, .checkNodeDevice = virAccessDriverPolkitCheckNodeDevice, .checkNWFilter = virAccessDriverPolkitCheckNWFilter, + .checkNWFilterBinding = virAccessDriverPolkitCheckNWFilterBinding, .checkSecret = virAccessDriverPolkitCheckSecret, .checkStoragePool = virAccessDriverPolkitCheckStoragePool, .checkStorageVol = virAccessDriverPolkitCheckStorageVol, diff --git a/src/access/viraccessdriverstack.c b/src/access/viraccessdriverstack.c index b43a743027..0ffc6abaf3 100644 --- a/src/access/viraccessdriverstack.c +++ b/src/access/viraccessdriverstack.c @@ -197,6 +197,29 @@ virAccessDriverStackCheckNWFilter(virAccessManagerPtr manager, return ret; } +static int +virAccessDriverStackCheckNWFilterBinding(virAccessManagerPtr manager, + const char *driverName, + virNWFilterBindingDefPtr binding, + virAccessPermNWFilterBinding perm) +{ + virAccessDriverStackPrivatePtr priv = virAccessManagerGetPrivateData(manager); + int ret = 1; + size_t i; + + for (i = 0; i < priv->managersLen; i++) { + int rv; + /* We do not short-circuit on first denial - always check all drivers */ + rv = virAccessManagerCheckNWFilterBinding(priv->managers[i], driverName, binding, perm); + if (rv == 0 && ret != -1) + ret = 0; + else if (rv < 0) + ret = -1; + } + + return ret; +} + static int virAccessDriverStackCheckSecret(virAccessManagerPtr manager, const char *driverName, @@ -277,6 +300,7 @@ virAccessDriver accessDriverStack = { .checkNetwork = virAccessDriverStackCheckNetwork, .checkNodeDevice = virAccessDriverStackCheckNodeDevice, .checkNWFilter = virAccessDriverStackCheckNWFilter, + .checkNWFilterBinding = virAccessDriverStackCheckNWFilterBinding, .checkSecret = virAccessDriverStackCheckSecret, .checkStoragePool = virAccessDriverStackCheckStoragePool, .checkStorageVol = virAccessDriverStackCheckStorageVol, diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c index b048a367e3..e7b5bf38da 100644 --- a/src/access/viraccessmanager.c +++ b/src/access/viraccessmanager.c @@ -296,6 +296,21 @@ int virAccessManagerCheckNWFilter(virAccessManagerPtr manager, return virAccessManagerSanitizeError(ret); } +int virAccessManagerCheckNWFilterBinding(virAccessManagerPtr manager, + const char *driverName, + virNWFilterBindingDefPtr binding, + virAccessPermNWFilterBinding perm) +{ + int ret = 0; + VIR_DEBUG("manager=%p(name=%s) driver=%s binding=%p perm=%d", + manager, manager->drv->name, driverName, binding, perm); + + if (manager->drv->checkNWFilterBinding) + ret = manager->drv->checkNWFilterBinding(manager, driverName, binding, perm); + + return virAccessManagerSanitizeError(ret); +} + int virAccessManagerCheckSecret(virAccessManagerPtr manager, const char *driverName, virSecretDefPtr secret, diff --git a/src/access/viraccessmanager.h b/src/access/viraccessmanager.h index e7eb15d30c..4fc86a1ff2 100644 --- a/src/access/viraccessmanager.h +++ b/src/access/viraccessmanager.h @@ -29,6 +29,7 @@ # include "conf/storage_conf.h" # include "conf/secret_conf.h" # include "conf/interface_conf.h" +# include "conf/virnwfilterbindingdef.h" # include "access/viraccessperm.h" typedef struct _virAccessManager virAccessManager; @@ -73,6 +74,10 @@ int virAccessManagerCheckNWFilter(virAccessManagerPtr manager, const char *driverName, virNWFilterDefPtr nwfilter, virAccessPermNWFilter perm); +int virAccessManagerCheckNWFilterBinding(virAccessManagerPtr manager, + const char *driverName, + virNWFilterBindingDefPtr binding, + virAccessPermNWFilterBinding perm); int virAccessManagerCheckSecret(virAccessManagerPtr manager, const char *driverName, virSecretDefPtr secret, diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c index 0f58290173..d7cbb70b7b 100644 --- a/src/access/viraccessperm.c +++ b/src/access/viraccessperm.c @@ -29,7 +29,7 @@ VIR_ENUM_IMPL(virAccessPermConnect, "search_domains", "search_networks", "search_storage_pools", "search_node_devices", "search_interfaces", "search_secrets", - "search_nwfilters", + "search_nwfilters", "search_nwfilter_bindings", "detect_storage_pools", "pm_control", "interface_transaction"); @@ -66,6 +66,11 @@ VIR_ENUM_IMPL(virAccessPermNWFilter, "getattr", "read", "write", "save", "delete"); +VIR_ENUM_IMPL(virAccessPermNWFilterBinding, + VIR_ACCESS_PERM_NWFILTER_BINDING_LAST, + "getattr", "read", + "create", "delete"); + VIR_ENUM_IMPL(virAccessPermSecret, VIR_ACCESS_PERM_SECRET_LAST, "getattr", "read", "write", diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h index 1817da73bc..0ea1f7a489 100644 --- a/src/access/viraccessperm.h +++ b/src/access/viraccessperm.h @@ -94,6 +94,13 @@ typedef enum { */ VIR_ACCESS_PERM_CONNECT_SEARCH_NWFILTERS, + /** + * @desc: List network filter bindings + * @message: Listing network filter bindings requires authorization + * @anonymous: 1 + */ + VIR_ACCESS_PERM_CONNECT_SEARCH_NWFILTER_BINDINGS, + /** * @desc: Detect storage pools @@ -486,6 +493,37 @@ typedef enum { VIR_ACCESS_PERM_NWFILTER_LAST } virAccessPermNWFilter; +typedef enum { + + /** + * @desc: Access network filter + * @message: Accessing network filter requires authorization + * @anonymous: 1 + */ + VIR_ACCESS_PERM_NWFILTER_BINDING_GETATTR, + + /** + * @desc: Read network filter binding + * @message: Reading network filter configuration requires authorization + * @anonymous: 1 + */ + VIR_ACCESS_PERM_NWFILTER_BINDING_READ, + + /** + * @desc: Create network filter binding + * @message: Creating network filter binding requires authorization + */ + VIR_ACCESS_PERM_NWFILTER_BINDING_CREATE, + + /** + * @desc: Delete network filter binding + * @message: Deleting network filter binding requires authorization + */ + VIR_ACCESS_PERM_NWFILTER_BINDING_DELETE, + + VIR_ACCESS_PERM_NWFILTER_BINDING_LAST +} virAccessPermNWFilterBinding; + typedef enum { /** @@ -657,6 +695,7 @@ VIR_ENUM_DECL(virAccessPermInterface); VIR_ENUM_DECL(virAccessPermNetwork); VIR_ENUM_DECL(virAccessPermNodeDevice); VIR_ENUM_DECL(virAccessPermNWFilter); +VIR_ENUM_DECL(virAccessPermNWFilterBinding); VIR_ENUM_DECL(virAccessPermSecret); VIR_ENUM_DECL(virAccessPermStoragePool); VIR_ENUM_DECL(virAccessPermStorageVol); diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index b8b83b6b40..480ebe7b00 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -2033,7 +2033,8 @@ elsif ($mode eq "client") { "storage_conf.h", "nwfilter_conf.h", "node_device_conf.h", - "interface_conf.h" + "interface_conf.h", + "virnwfilterbindingdef.h", ); foreach my $hdr (@headers) { print "#include \"$hdr\"\n"; -- 2.17.0

Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/remote/remote_daemon_dispatch.c | 15 +++++ src/remote/remote_driver.c | 20 +++++++ src/remote/remote_protocol.x | 90 ++++++++++++++++++++++++++++- src/remote_protocol-structs | 43 ++++++++++++++ src/rpc/gendispatch.pl | 12 ++-- 5 files changed, 173 insertions(+), 7 deletions(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index a8a5932d71..26ccc8d4b6 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -90,6 +90,7 @@ static virStoragePoolPtr get_nonnull_storage_pool(virConnectPtr conn, remote_non static virStorageVolPtr get_nonnull_storage_vol(virConnectPtr conn, remote_nonnull_storage_vol vol); static virSecretPtr get_nonnull_secret(virConnectPtr conn, remote_nonnull_secret secret); static virNWFilterPtr get_nonnull_nwfilter(virConnectPtr conn, remote_nonnull_nwfilter nwfilter); +static virNWFilterBindingPtr get_nonnull_nwfilter_binding(virConnectPtr conn, remote_nonnull_nwfilter_binding binding); static virDomainSnapshotPtr get_nonnull_domain_snapshot(virDomainPtr dom, remote_nonnull_domain_snapshot snapshot); static virNodeDevicePtr get_nonnull_node_device(virConnectPtr conn, remote_nonnull_node_device dev); static void make_nonnull_domain(remote_nonnull_domain *dom_dst, virDomainPtr dom_src); @@ -100,6 +101,7 @@ static void make_nonnull_storage_vol(remote_nonnull_storage_vol *vol_dst, virSto static void make_nonnull_node_device(remote_nonnull_node_device *dev_dst, virNodeDevicePtr dev_src); static void make_nonnull_secret(remote_nonnull_secret *secret_dst, virSecretPtr secret_src); static void make_nonnull_nwfilter(remote_nonnull_nwfilter *net_dst, virNWFilterPtr nwfilter_src); +static void make_nonnull_nwfilter_binding(remote_nonnull_nwfilter_binding *binding_dst, virNWFilterBindingPtr binding_src); static void make_nonnull_domain_snapshot(remote_nonnull_domain_snapshot *snapshot_dst, virDomainSnapshotPtr snapshot_src); static int @@ -6998,6 +7000,12 @@ get_nonnull_nwfilter(virConnectPtr conn, remote_nonnull_nwfilter nwfilter) return virGetNWFilter(conn, nwfilter.name, BAD_CAST nwfilter.uuid); } +static virNWFilterBindingPtr +get_nonnull_nwfilter_binding(virConnectPtr conn, remote_nonnull_nwfilter_binding binding) +{ + return virGetNWFilterBinding(conn, binding.portdev, binding.filtername); +} + static virDomainSnapshotPtr get_nonnull_domain_snapshot(virDomainPtr dom, remote_nonnull_domain_snapshot snapshot) { @@ -7070,6 +7078,13 @@ make_nonnull_nwfilter(remote_nonnull_nwfilter *nwfilter_dst, virNWFilterPtr nwfi memcpy(nwfilter_dst->uuid, nwfilter_src->uuid, VIR_UUID_BUFLEN); } +static void +make_nonnull_nwfilter_binding(remote_nonnull_nwfilter_binding *binding_dst, virNWFilterBindingPtr binding_src) +{ + ignore_value(VIR_STRDUP_QUIET(binding_dst->portdev, binding_src->portdev)); + ignore_value(VIR_STRDUP_QUIET(binding_dst->filtername, binding_src->filtername)); +} + static void make_nonnull_domain_snapshot(remote_nonnull_domain_snapshot *snapshot_dst, virDomainSnapshotPtr snapshot_src) { diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 95437b4365..13eafa67d1 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -141,6 +141,7 @@ static int remoteAuthPolkit(virConnectPtr conn, struct private_data *priv, static virDomainPtr get_nonnull_domain(virConnectPtr conn, remote_nonnull_domain domain); static virNetworkPtr get_nonnull_network(virConnectPtr conn, remote_nonnull_network network); static virNWFilterPtr get_nonnull_nwfilter(virConnectPtr conn, remote_nonnull_nwfilter nwfilter); +static virNWFilterBindingPtr get_nonnull_nwfilter_binding(virConnectPtr conn, remote_nonnull_nwfilter_binding binding); static virInterfacePtr get_nonnull_interface(virConnectPtr conn, remote_nonnull_interface iface); static virStoragePoolPtr get_nonnull_storage_pool(virConnectPtr conn, remote_nonnull_storage_pool pool); static virStorageVolPtr get_nonnull_storage_vol(virConnectPtr conn, remote_nonnull_storage_vol vol); @@ -156,6 +157,7 @@ static void make_nonnull_node_device(remote_nonnull_node_device *dev_dst, virNodeDevicePtr dev_src); static void make_nonnull_secret(remote_nonnull_secret *secret_dst, virSecretPtr secret_src); static void make_nonnull_nwfilter(remote_nonnull_nwfilter *nwfilter_dst, virNWFilterPtr nwfilter_src); +static void make_nonnull_nwfilter_binding(remote_nonnull_nwfilter_binding *binding_dst, virNWFilterBindingPtr binding_src); static void make_nonnull_domain_snapshot(remote_nonnull_domain_snapshot *snapshot_dst, virDomainSnapshotPtr snapshot_src); /*----------------------------------------------------------------------*/ @@ -8138,6 +8140,12 @@ get_nonnull_nwfilter(virConnectPtr conn, remote_nonnull_nwfilter nwfilter) return virGetNWFilter(conn, nwfilter.name, BAD_CAST nwfilter.uuid); } +static virNWFilterBindingPtr +get_nonnull_nwfilter_binding(virConnectPtr conn, remote_nonnull_nwfilter_binding binding) +{ + return virGetNWFilterBinding(conn, binding.portdev, binding.filtername); +} + static virDomainSnapshotPtr get_nonnull_domain_snapshot(virDomainPtr domain, remote_nonnull_domain_snapshot snapshot) { @@ -8205,6 +8213,13 @@ make_nonnull_nwfilter(remote_nonnull_nwfilter *nwfilter_dst, virNWFilterPtr nwfi memcpy(nwfilter_dst->uuid, nwfilter_src->uuid, VIR_UUID_BUFLEN); } +static void +make_nonnull_nwfilter_binding(remote_nonnull_nwfilter_binding *binding_dst, virNWFilterBindingPtr binding_src) +{ + binding_dst->portdev = binding_src->portdev; + binding_dst->filtername = binding_src->filtername; +} + static void make_nonnull_domain_snapshot(remote_nonnull_domain_snapshot *snapshot_dst, virDomainSnapshotPtr snapshot_src) { @@ -8583,6 +8598,11 @@ static virNWFilterDriver nwfilter_driver = { .connectNumOfNWFilters = remoteConnectNumOfNWFilters, /* 0.8.0 */ .connectListNWFilters = remoteConnectListNWFilters, /* 0.8.0 */ .connectListAllNWFilters = remoteConnectListAllNWFilters, /* 0.10.2 */ + .connectListAllNWFilterBindings = remoteConnectListAllNWFilterBindings, /* 4.4.0 */ + .nwfilterBindingLookupByPortDev = remoteNWFilterBindingLookupByPortDev, /* 4.4.0 */ + .nwfilterBindingCreateXML = remoteNWFilterBindingCreateXML, /* 4.4.0 */ + .nwfilterBindingDelete = remoteNWFilterBindingDelete, /* 4.4.0 */ + .nwfilterBindingGetXMLDesc = remoteNWFilterBindingGetXMLDesc, /* 4.4.0 */ }; static virConnectDriver connect_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 296a087181..347ab7fa32 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -94,6 +94,9 @@ const REMOTE_NODE_DEVICE_CAPS_LIST_MAX = 65536; /* Upper limit on lists of network filters. */ const REMOTE_NWFILTER_LIST_MAX = 16384; +/* Upper limit on lists of network filter bindings. */ +const REMOTE_NWFILTER_BINDING_LIST_MAX = 16384; + /* Upper limit on list of scheduler parameters. */ const REMOTE_DOMAIN_SCHEDULER_PARAMETERS_MAX = 16; @@ -275,6 +278,12 @@ struct remote_nonnull_nwfilter { remote_uuid uuid; }; +/* A network filter binding which may not be NULL. */ +struct remote_nonnull_nwfilter_binding { + remote_nonnull_string portdev; + remote_nonnull_string filtername; +}; + /* An interface which may not be NULL. */ struct remote_nonnull_interface { remote_nonnull_string name; @@ -316,6 +325,7 @@ struct remote_nonnull_domain_snapshot { typedef remote_nonnull_domain *remote_domain; typedef remote_nonnull_network *remote_network; typedef remote_nonnull_nwfilter *remote_nwfilter; +typedef remote_nonnull_nwfilter_binding *remote_nwfilter_binding; typedef remote_nonnull_storage_pool *remote_storage_pool; typedef remote_nonnull_storage_vol *remote_storage_vol; typedef remote_nonnull_node_device *remote_node_device; @@ -3448,6 +3458,48 @@ struct remote_domain_set_lifecycle_action_args { unsigned int flags; }; +/* nwfilter binding */ + +struct remote_nwfilter_binding_lookup_by_port_dev_args { + remote_nonnull_string name; +}; + +struct remote_nwfilter_binding_lookup_by_port_dev_ret { + remote_nonnull_nwfilter_binding nwfilter; +}; + +struct remote_nwfilter_binding_create_xml_args { + remote_nonnull_string xml; + unsigned int flags; +}; + +struct remote_nwfilter_binding_create_xml_ret { + remote_nonnull_nwfilter_binding nwfilter; +}; + +struct remote_nwfilter_binding_delete_args { + remote_nonnull_nwfilter_binding nwfilter; +}; + +struct remote_nwfilter_binding_get_xml_desc_args { + remote_nonnull_nwfilter_binding nwfilter; + unsigned int flags; +}; + +struct remote_nwfilter_binding_get_xml_desc_ret { + remote_nonnull_string xml; +}; + +struct remote_connect_list_all_nwfilter_bindings_args { + int need_results; + unsigned int flags; +}; + +struct remote_connect_list_all_nwfilter_bindings_ret { /* insert@1 */ + remote_nonnull_nwfilter_binding bindings<REMOTE_NWFILTER_BINDING_LIST_MAX>; + unsigned int ret; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -6135,5 +6187,41 @@ enum remote_procedure { * @priority: high * @acl: storage_pool:getattr */ - REMOTE_PROC_STORAGE_POOL_LOOKUP_BY_TARGET_PATH = 391 + REMOTE_PROC_STORAGE_POOL_LOOKUP_BY_TARGET_PATH = 391, + + /** + * @generate: both + * @priority: high + * @acl: nwfilter_binding:getattr + */ + REMOTE_PROC_NWFILTER_BINDING_LOOKUP_BY_PORT_DEV = 392, + + /** + * @generate: both + * @priority: high + * @acl: nwfilter_binding:read + */ + REMOTE_PROC_NWFILTER_BINDING_GET_XML_DESC = 393, + + /** + * @generate: both + * @priority: high + * @acl: nwfilter_binding:create + */ + REMOTE_PROC_NWFILTER_BINDING_CREATE_XML = 394, + + /** + * @generate: both + * @priority: high + * @acl: nwfilter_binding:delete + */ + REMOTE_PROC_NWFILTER_BINDING_DELETE = 395, + + /** + * @generate: both + * @priority: high + * @acl: connect:search_nwfilter_bindings + * @aclfilter: nwfilter_binding:getattr + */ + REMOTE_PROC_CONNECT_LIST_ALL_NWFILTER_BINDINGS = 396 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index fe163db73f..6deb79dff0 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -21,6 +21,10 @@ struct remote_nonnull_nwfilter { remote_nonnull_string name; remote_uuid uuid; }; +struct remote_nonnull_nwfilter_binding { + remote_nonnull_string portdev; + remote_nonnull_string filtername; +}; struct remote_nonnull_interface { remote_nonnull_string name; remote_nonnull_string mac; @@ -2877,6 +2881,40 @@ struct remote_domain_set_lifecycle_action_args { u_int action; u_int flags; }; +struct remote_nwfilter_binding_lookup_by_port_dev_args { + remote_nonnull_string name; +}; +struct remote_nwfilter_binding_lookup_by_port_dev_ret { + remote_nonnull_nwfilter_binding nwfilter; +}; +struct remote_nwfilter_binding_create_xml_args { + remote_nonnull_string xml; + u_int flags; +}; +struct remote_nwfilter_binding_create_xml_ret { + remote_nonnull_nwfilter_binding nwfilter; +}; +struct remote_nwfilter_binding_delete_args { + remote_nonnull_nwfilter_binding nwfilter; +}; +struct remote_nwfilter_binding_get_xml_desc_args { + remote_nonnull_nwfilter_binding nwfilter; + u_int flags; +}; +struct remote_nwfilter_binding_get_xml_desc_ret { + remote_nonnull_string xml; +}; +struct remote_connect_list_all_nwfilter_bindings_args { + int need_results; + u_int flags; +}; +struct remote_connect_list_all_nwfilter_bindings_ret { + struct { + u_int bindings_len; + remote_nonnull_nwfilter_binding * bindings_val; + } bindings; + u_int ret; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -3269,4 +3307,9 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_MANAGED_SAVE_DEFINE_XML = 389, REMOTE_PROC_DOMAIN_SET_LIFECYCLE_ACTION = 390, REMOTE_PROC_STORAGE_POOL_LOOKUP_BY_TARGET_PATH = 391, + REMOTE_PROC_NWFILTER_BINDING_LOOKUP_BY_PORT_DEV = 392, + REMOTE_PROC_NWFILTER_BINDING_GET_XML_DESC = 393, + REMOTE_PROC_NWFILTER_BINDING_CREATE_XML = 394, + REMOTE_PROC_NWFILTER_BINDING_DELETE = 395, + REMOTE_PROC_CONNECT_LIST_ALL_NWFILTER_BINDINGS = 396, }; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 480ebe7b00..0c4648c0fb 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -557,7 +557,7 @@ elsif ($mode eq "server") { if ($args_member =~ m/^remote_nonnull_string name;/ and $has_node_device) { # ignore the name arg for node devices next - } elsif ($args_member =~ m/^remote_nonnull_(domain|network|storage_pool|storage_vol|interface|secret|nwfilter) (\S+);/) { + } elsif ($args_member =~ m/^remote_nonnull_(domain|network|storage_pool|storage_vol|interface|secret|nwfilter|nwfilter_binding) (\S+);/) { my $type_name = name_to_TypeName($1); push(@vars_list, "vir${type_name}Ptr $2 = NULL"); @@ -722,7 +722,7 @@ elsif ($mode eq "server") { if (!$modern_ret_as_list) { push(@ret_list, "ret->$3 = tmp.$3;"); } - } elsif ($ret_member =~ m/(?:admin|remote)_nonnull_(secret|nwfilter|node_device|interface|network|storage_vol|storage_pool|domain_snapshot|domain|server|client) (\S+)<(\S+)>;/) { + } elsif ($ret_member =~ m/(?:admin|remote)_nonnull_(secret|nwfilter|nwfilter_binding|node_device|interface|network|storage_vol|storage_pool|domain_snapshot|domain|server|client) (\S+)<(\S+)>;/) { $modern_ret_struct_name = $1; $single_ret_list_error_msg_type = $1; $single_ret_list_name = $2; @@ -780,7 +780,7 @@ elsif ($mode eq "server") { $single_ret_var = $1; $single_ret_by_ref = 0; $single_ret_check = " == NULL"; - } elsif ($ret_member =~ m/^remote_nonnull_(domain|network|storage_pool|storage_vol|interface|node_device|secret|nwfilter|domain_snapshot) (\S+);/) { + } elsif ($ret_member =~ m/^remote_nonnull_(domain|network|storage_pool|storage_vol|interface|node_device|secret|nwfilter|nwfilter_binding|domain_snapshot) (\S+);/) { my $type_name = name_to_TypeName($1); if ($call->{ProcName} eq "DomainCreateWithFlags") { @@ -1325,7 +1325,7 @@ elsif ($mode eq "client") { $priv_src = "dev->conn"; push(@args_list, "virNodeDevicePtr dev"); push(@setters_list, "args.name = dev->name;"); - } elsif ($args_member =~ m/^remote_nonnull_(domain|network|storage_pool|storage_vol|interface|secret|nwfilter|domain_snapshot) (\S+);/) { + } elsif ($args_member =~ m/^remote_nonnull_(domain|network|storage_pool|storage_vol|interface|secret|nwfilter|nwfilter_binding|domain_snapshot) (\S+);/) { my $name = $1; my $arg_name = $2; my $type_name = name_to_TypeName($name); @@ -1518,7 +1518,7 @@ elsif ($mode eq "client") { } push(@ret_list, "memcpy(result->$3, ret.$3, sizeof(result->$3));"); - } elsif ($ret_member =~ m/(?:admin|remote)_nonnull_(secret|nwfilter|node_device|interface|network|storage_vol|storage_pool|domain_snapshot|domain|server|client) (\S+)<(\S+)>;/) { + } elsif ($ret_member =~ m/(?:admin|remote)_nonnull_(secret|nwfilter|nwfilter_binding|node_device|interface|network|storage_vol|storage_pool|domain_snapshot|domain|server|client) (\S+)<(\S+)>;/) { my $proc_name = name_to_TypeName($1); if ($structprefix eq "admin") { @@ -1571,7 +1571,7 @@ elsif ($mode eq "client") { push(@ret_list, "VIR_FREE(ret.$1);"); $single_ret_var = "char *rv = NULL"; $single_ret_type = "char *"; - } elsif ($ret_member =~ m/^remote_nonnull_(domain|network|storage_pool|storage_vol|node_device|interface|secret|nwfilter|domain_snapshot) (\S+);/) { + } elsif ($ret_member =~ m/^remote_nonnull_(domain|network|storage_pool|storage_vol|node_device|interface|secret|nwfilter|nwfilter_binding|domain_snapshot) (\S+);/) { my $name = $1; my $arg_name = $2; my $type_name = name_to_TypeName($name); -- 2.17.0

$ virsh nwfilter-binding-list Port Dev Filter ------------------------------------------------------------------ vnet0 clean-traffic vnet1 clean-traffic $ virsh nwfilter-binding-dumpxml vnet1 <filterbinding> <owner> <name>f25arm7</name> <uuid>12ac8b8c-4f23-4248-ae42-fdcd50c400fd</uuid> </owner> <portdev name='vnet1'/> <mac address='52:54:00:9d:81:b1'/> <filterref filter='clean-traffic'> <parameter name='MAC' value='52:54:00:9d:81:b1'/> </filterref> </filterbinding> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- tools/virsh-completer.c | 45 ++++++ tools/virsh-completer.h | 4 + tools/virsh-nwfilter.c | 318 ++++++++++++++++++++++++++++++++++++++++ tools/virsh-nwfilter.h | 8 + 4 files changed, 375 insertions(+) diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index e3b8234b41..b1737130b4 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -427,6 +427,51 @@ virshNWFilterNameCompleter(vshControl *ctl, } +char ** +virshNWFilterBindingNameCompleter(vshControl *ctl, + const vshCmd *cmd ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virshControlPtr priv = ctl->privData; + virNWFilterBindingPtr *bindings = NULL; + int nbindings = 0; + size_t i = 0; + char **ret = NULL; + + virCheckFlags(0, NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if ((nbindings = virConnectListAllNWFilterBindings(priv->conn, &bindings, flags)) < 0) + return NULL; + + if (VIR_ALLOC_N(ret, nbindings + 1) < 0) + goto error; + + for (i = 0; i < nbindings; i++) { + const char *name = virNWFilterBindingGetPortDev(bindings[i]); + + if (VIR_STRDUP(ret[i], name) < 0) + goto error; + + virNWFilterBindingFree(bindings[i]); + } + VIR_FREE(bindings); + + return ret; + + error: + for (; i < nbindings; i++) + virNWFilterBindingFree(bindings[i]); + VIR_FREE(bindings); + for (i = 0; i < nbindings; i++) + VIR_FREE(ret[i]); + VIR_FREE(ret); + return NULL; +} + + char ** virshSecretUUIDCompleter(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED, diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index fa443d3ad7..3abced765c 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -62,6 +62,10 @@ char ** virshNWFilterNameCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); +char ** virshNWFilterBindingNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + char ** virshSecretUUIDCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index 06a002dffd..881afc5dda 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -443,6 +443,300 @@ cmdNWFilterEdit(vshControl *ctl, const vshCmd *cmd) return ret; } +virNWFilterBindingPtr +virshCommandOptNWFilterBindingBy(vshControl *ctl, const vshCmd *cmd, + const char **name, unsigned int flags) +{ + virNWFilterBindingPtr binding = NULL; + const char *n = NULL; + const char *optname = "binding"; + virshControlPtr priv = ctl->privData; + + virCheckFlags(0, NULL); + + if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) + return NULL; + + vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", + cmd->def->name, optname, n); + + if (name) + *name = n; + + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as nwfilter binding port dev\n", + cmd->def->name, optname); + binding = virNWFilterBindingLookupByPortDev(priv->conn, n); + + if (!binding) + vshError(ctl, _("failed to get nwfilter binding '%s'"), n); + + return binding; +} + +/* + * "nwfilter-binding-create" command + */ +static const vshCmdInfo info_nwfilter_binding_create[] = { + {.name = "help", + .data = N_("create a network filter binding from an XML file") + }, + {.name = "desc", + .data = N_("Create a new network filter binding.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_nwfilter_binding_create[] = { + VIRSH_COMMON_OPT_FILE(N_("file containing an XML network " + "filter binding description")), + {.name = NULL} +}; + +static bool +cmdNWFilterBindingCreate(vshControl *ctl, const vshCmd *cmd) +{ + virNWFilterBindingPtr binding; + const char *from = NULL; + bool ret = true; + char *buffer; + virshControlPtr priv = ctl->privData; + + if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + return false; + + if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) + return false; + + binding = virNWFilterBindingCreateXML(priv->conn, buffer, 0); + VIR_FREE(buffer); + + if (binding != NULL) { + vshPrintExtra(ctl, _("Network filter binding on %s created from %s\n"), + virNWFilterBindingGetPortDev(binding), from); + virNWFilterBindingFree(binding); + } else { + vshError(ctl, _("Failed to create network filter from %s"), from); + ret = false; + } + return ret; +} + +/* + * "nwfilter-binding-delete" command + */ +static const vshCmdInfo info_nwfilter_binding_delete[] = { + {.name = "help", + .data = N_("delete a network filter binding") + }, + {.name = "desc", + .data = N_("Delete a given network filter binding.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_nwfilter_binding_delete[] = { + {.name = "binding", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("network filter binding port dev"), + .completer = virshNWFilterBindingNameCompleter, + }, + {.name = NULL} +}; + +static bool +cmdNWFilterBindingDelete(vshControl *ctl, const vshCmd *cmd) +{ + virNWFilterBindingPtr binding; + bool ret = true; + const char *portdev; + + if (!(binding = virshCommandOptNWFilterBinding(ctl, cmd, &portdev))) + return false; + + if (virNWFilterBindingDelete(binding) == 0) { + vshPrintExtra(ctl, _("Network filter binding on %s deleted\n"), portdev); + } else { + vshError(ctl, _("Failed to delete network filter binding on %s"), portdev); + ret = false; + } + + virNWFilterBindingFree(binding); + return ret; +} + +/* + * "nwfilter-binding-dumpxml" command + */ +static const vshCmdInfo info_nwfilter_binding_dumpxml[] = { + {.name = "help", + .data = N_("network filter information in XML") + }, + {.name = "desc", + .data = N_("Output the network filter information as an XML dump to stdout.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_nwfilter_binding_dumpxml[] = { + {.name = "binding", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("network filter binding portdev"), + .completer = virshNWFilterBindingNameCompleter, + }, + {.name = NULL} +}; + +static bool +cmdNWFilterBindingDumpXML(vshControl *ctl, const vshCmd *cmd) +{ + virNWFilterBindingPtr binding; + bool ret = true; + char *dump; + + if (!(binding = virshCommandOptNWFilterBinding(ctl, cmd, NULL))) + return false; + + dump = virNWFilterBindingGetXMLDesc(binding, 0); + if (dump != NULL) { + vshPrint(ctl, "%s", dump); + VIR_FREE(dump); + } else { + ret = false; + } + + virNWFilterBindingFree(binding); + return ret; +} + +static int +virshNWFilterBindingSorter(const void *a, const void *b) +{ + virNWFilterBindingPtr *fa = (virNWFilterBindingPtr *) a; + virNWFilterBindingPtr *fb = (virNWFilterBindingPtr *) b; + + if (*fa && !*fb) + return -1; + + if (!*fa) + return *fb != NULL; + + return vshStrcasecmp(virNWFilterBindingGetPortDev(*fa), + virNWFilterBindingGetPortDev(*fb)); +} + +struct virshNWFilterBindingList { + virNWFilterBindingPtr *bindings; + size_t nbindings; +}; +typedef struct virshNWFilterBindingList *virshNWFilterBindingListPtr; + +static void +virshNWFilterBindingListFree(virshNWFilterBindingListPtr list) +{ + size_t i; + + if (list && list->bindings) { + for (i = 0; i < list->nbindings; i++) { + if (list->bindings[i]) + virNWFilterBindingFree(list->bindings[i]); + } + VIR_FREE(list->bindings); + } + VIR_FREE(list); +} + +static virshNWFilterBindingListPtr +virshNWFilterBindingListCollect(vshControl *ctl, + unsigned int flags) +{ + virshNWFilterBindingListPtr list = vshMalloc(ctl, sizeof(*list)); + size_t i; + int ret; + bool success = false; + size_t deleted = 0; + int nbindings = 0; + char **names = NULL; + virshControlPtr priv = ctl->privData; + + /* try the list with flags support (0.10.2 and later) */ + if ((ret = virConnectListAllNWFilterBindings(priv->conn, + &list->bindings, + flags)) < 0) { + /* there was an error during the call */ + vshError(ctl, "%s", _("Failed to list network filter bindings")); + goto cleanup; + } + + list->nbindings = ret; + + /* sort the list */ + if (list->bindings && list->nbindings) + qsort(list->bindings, list->nbindings, + sizeof(*list->bindings), virshNWFilterBindingSorter); + + /* truncate the list for not found filter objects */ + if (deleted) + VIR_SHRINK_N(list->bindings, list->nbindings, deleted); + + success = true; + + cleanup: + for (i = 0; nbindings != -1 && i < nbindings; i++) + VIR_FREE(names[i]); + VIR_FREE(names); + + if (!success) { + virshNWFilterBindingListFree(list); + list = NULL; + } + + return list; +} + +/* + * "nwfilter-binding-list" command + */ +static const vshCmdInfo info_nwfilter_binding_list[] = { + {.name = "help", + .data = N_("list network filter bindings") + }, + {.name = "desc", + .data = N_("Returns list of network filter bindings.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_nwfilter_binding_list[] = { + {.name = NULL} +}; + +static bool +cmdNWFilterBindingList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + size_t i; + virshNWFilterBindingListPtr list = NULL; + + if (!(list = virshNWFilterBindingListCollect(ctl, 0))) + return false; + + vshPrintExtra(ctl, " %-36s %-20s \n", _("Port Dev"), _("Filter")); + vshPrintExtra(ctl, "---------------------------------" + "---------------------------------\n"); + + for (i = 0; i < list->nbindings; i++) { + virNWFilterBindingPtr binding = list->bindings[i]; + + vshPrint(ctl, " %-20s %-20s\n", + virNWFilterBindingGetPortDev(binding), + virNWFilterBindingGetFilterName(binding)); + } + + virshNWFilterBindingListFree(list); + return true; +} + const vshCmdDef nwfilterCmds[] = { {.name = "nwfilter-define", .handler = cmdNWFilterDefine, @@ -474,5 +768,29 @@ const vshCmdDef nwfilterCmds[] = { .info = info_nwfilter_undefine, .flags = 0 }, + {.name = "nwfilter-binding-create", + .handler = cmdNWFilterBindingCreate, + .opts = opts_nwfilter_binding_create, + .info = info_nwfilter_binding_create, + .flags = 0 + }, + {.name = "nwfilter-binding-delete", + .handler = cmdNWFilterBindingDelete, + .opts = opts_nwfilter_binding_delete, + .info = info_nwfilter_binding_delete, + .flags = 0 + }, + {.name = "nwfilter-binding-dumpxml", + .handler = cmdNWFilterBindingDumpXML, + .opts = opts_nwfilter_binding_dumpxml, + .info = info_nwfilter_binding_dumpxml, + .flags = 0 + }, + {.name = "nwfilter-binding-list", + .handler = cmdNWFilterBindingList, + .opts = opts_nwfilter_binding_list, + .info = info_nwfilter_binding_list, + .flags = 0 + }, {.name = NULL} }; diff --git a/tools/virsh-nwfilter.h b/tools/virsh-nwfilter.h index 2b76a7c849..d8ca0e3960 100644 --- a/tools/virsh-nwfilter.h +++ b/tools/virsh-nwfilter.h @@ -32,11 +32,19 @@ virNWFilterPtr virshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd, const char **name, unsigned int flags); +virNWFilterBindingPtr +virshCommandOptNWFilterBindingBy(vshControl *ctl, const vshCmd *cmd, + const char **name, unsigned int flags); + /* default is lookup by Name and UUID */ # define virshCommandOptNWFilter(_ctl, _cmd, _name) \ virshCommandOptNWFilterBy(_ctl, _cmd, _name, \ VIRSH_BYUUID | VIRSH_BYNAME) +/* default is lookup by port dev */ +# define virshCommandOptNWFilterBinding(_ctl, _cmd, _name) \ + virshCommandOptNWFilterBindingBy(_ctl, _cmd, _name, 0) + extern const vshCmdDef nwfilterCmds[]; #endif /* VIRSH_NWFILTER_H */ -- 2.17.0

Use the virNWFilterBindingDefPtr struct in the gentech driver code directly. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/nwfilter/nwfilter_dhcpsnoop.c | 35 +++-- src/nwfilter/nwfilter_driver.c | 22 ++- src/nwfilter/nwfilter_gentech_driver.c | 209 +++++++++++++------------ src/nwfilter/nwfilter_gentech_driver.h | 22 ++- src/nwfilter/nwfilter_learnipaddr.c | 16 +- 5 files changed, 167 insertions(+), 137 deletions(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index aff062ca7c..f24fec1638 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -497,15 +497,18 @@ virNWFilterSnoopIPLeaseInstallRule(virNWFilterSnoopIPLeasePtr ipl, /* instantiate the filters */ - if (req->ifname) + if (req->ifname) { + virNWFilterBindingDef binding = { + .portdevname = req->ifname, + .linkdevname = req->linkdev, + .mac = req->macaddr, + .filter = req->filtername, + .filterparams = req->vars, + }; rc = virNWFilterInstantiateFilterLate(req->driver, - NULL, - req->ifname, - req->ifindex, - req->linkdev, - &req->macaddr, - req->filtername, - req->vars); + &binding, + req->ifindex); + } exit_snooprequnlock: virNWFilterSnoopReqUnlock(req); @@ -884,14 +887,16 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req, goto skip_instantiate; if (ipAddrLeft) { + virNWFilterBindingDef binding = { + .portdevname = req->ifname, + .linkdevname = req->linkdev, + .mac = req->macaddr, + .filter = req->filtername, + .filterparams = req->vars, + }; ret = virNWFilterInstantiateFilterLate(req->driver, - NULL, - req->ifname, - req->ifindex, - req->linkdev, - &req->macaddr, - req->filtername, - req->vars); + &binding, + req->ifindex); } else { virNWFilterVarValuePtr dhcpsrvrs = virHashLookup(req->vars, NWFILTER_VARNAME_DHCPSERVER); diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index d17a8ec00b..b57e5dd00d 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -38,6 +38,7 @@ #include "domain_conf.h" #include "domain_nwfilter.h" #include "nwfilter_driver.h" +#include "virnwfilterbindingdef.h" #include "nwfilter_gentech_driver.h" #include "configmake.h" #include "virfile.h" @@ -642,19 +643,34 @@ nwfilterGetXMLDesc(virNWFilterPtr nwfilter, static int -nwfilterInstantiateFilter(const char *vmname ATTRIBUTE_UNUSED, +nwfilterInstantiateFilter(const char *vmname, const unsigned char *vmuuid, virDomainNetDefPtr net) { - return virNWFilterInstantiateFilter(driver, vmuuid, net); + virNWFilterBindingDefPtr binding; + int ret; + + if (!(binding = virNWFilterBindingDefForNet(vmname, vmuuid, net))) + return -1; + ret = virNWFilterInstantiateFilter(driver, binding); + virNWFilterBindingDefFree(binding); + return ret; } static void nwfilterTeardownFilter(virDomainNetDefPtr net) { + virNWFilterBindingDef binding = { + .portdevname = net->ifname, + .linkdevname = (net->type == VIR_DOMAIN_NET_TYPE_DIRECT ? + net->data.direct.linkdev : NULL), + .mac = net->mac, + .filter = net->filter, + .filterparams = net->filterparams, + }; if ((net->ifname) && (net->filter)) - virNWFilterTeardownFilter(net); + virNWFilterTeardownFilter(&binding); } diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index af4411d4db..dc925dee16 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -577,12 +577,9 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter, /** * virNWFilterDoInstantiate: - * @vmuuid: The UUID of the VM * @techdriver: The driver to use for instantiation + * @binding: description of port to bind the filter to * @filter: The filter to instantiate - * @ifname: The name of the interface to apply the rules to - * @vars: A map holding variable names and values used for instantiating - * the filter and its subfilters. * @forceWithPendingReq: Ignore the check whether a pending learn request * is active; 'true' only when the rules are applied late * @@ -596,17 +593,13 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter, * Call this function while holding the NWFilter filter update lock */ static int -virNWFilterDoInstantiate(const unsigned char *vmuuid, - virNWFilterTechDriverPtr techdriver, +virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver, + virNWFilterBindingDefPtr binding, virNWFilterDefPtr filter, - const char *ifname, int ifindex, - const char *linkdev, - virHashTablePtr vars, enum instCase useNewFilter, bool *foundNewFilter, bool teardownOld, - const virMacAddr *macaddr, virNWFilterDriverStatePtr driver, bool forceWithPendingReq) { @@ -628,14 +621,14 @@ virNWFilterDoInstantiate(const unsigned char *vmuuid, } rc = virNWFilterDetermineMissingVarsRec(filter, - vars, + binding->filterparams, missing_vars, useNewFilter, driver); if (rc < 0) goto err_exit; - lv = virHashLookup(vars, NWFILTER_VARNAME_CTRL_IP_LEARNING); + lv = virHashLookup(binding->filterparams, NWFILTER_VARNAME_CTRL_IP_LEARNING); if (lv) learning = virNWFilterVarValueGetNthValue(lv, 0); else @@ -652,19 +645,20 @@ virNWFilterDoInstantiate(const unsigned char *vmuuid, goto err_unresolvable_vars; } if (STRCASEEQ(learning, "dhcp")) { - rc = virNWFilterDHCPSnoopReq(techdriver, ifname, linkdev, - vmuuid, macaddr, - filter->name, vars, driver); + rc = virNWFilterDHCPSnoopReq(techdriver, binding->portdevname, + binding->linkdevname, + binding->owneruuid, &binding->mac, + filter->name, binding->filterparams, driver); goto err_exit; } else if (STRCASEEQ(learning, "any")) { if (!virNWFilterHasLearnReq(ifindex)) { rc = virNWFilterLearnIPAddress(techdriver, - ifname, + binding->portdevname, ifindex, - linkdev, - macaddr, + binding->linkdevname, + &binding->mac, filter->name, - vars, driver, + binding->filterparams, driver, DETECT_DHCP|DETECT_STATIC); } goto err_exit; @@ -688,7 +682,7 @@ virNWFilterDoInstantiate(const unsigned char *vmuuid, rc = virNWFilterDefToInst(driver, filter, - vars, + binding->filterparams, useNewFilter, foundNewFilter, &inst); @@ -705,22 +699,22 @@ virNWFilterDoInstantiate(const unsigned char *vmuuid, } if (instantiate) { - if (virNWFilterLockIface(ifname) < 0) + if (virNWFilterLockIface(binding->portdevname) < 0) goto err_exit; - rc = techdriver->applyNewRules(ifname, inst.rules, inst.nrules); + rc = techdriver->applyNewRules(binding->portdevname, inst.rules, inst.nrules); if (teardownOld && rc == 0) - techdriver->tearOldRules(ifname); + techdriver->tearOldRules(binding->portdevname); - if (rc == 0 && (virNetDevValidateConfig(ifname, NULL, ifindex) <= 0)) { + if (rc == 0 && (virNetDevValidateConfig(binding->portdevname, NULL, ifindex) <= 0)) { virResetLastError(); /* interface changed/disppeared */ - techdriver->allTeardown(ifname); + techdriver->allTeardown(binding->portdevname); rc = -1; } - virNWFilterUnlockIface(ifname); + virNWFilterUnlockIface(binding->portdevname); } err_exit: @@ -749,14 +743,9 @@ virNWFilterDoInstantiate(const unsigned char *vmuuid, */ static int virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver, - const unsigned char *vmuuid, bool teardownOld, - const char *ifname, + virNWFilterBindingDefPtr binding, int ifindex, - const char *linkdev, - const virMacAddr *macaddr, - const char *filtername, - virHashTablePtr filterparams, enum instCase useNewFilter, bool forceWithPendingReq, bool *foundNewFilter) @@ -765,7 +754,6 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver, const char *drvname = EBIPTABLES_DRIVER_ID; virNWFilterTechDriverPtr techdriver; virNWFilterObjPtr obj; - virHashTablePtr vars, vars1; virNWFilterDefPtr filter; virNWFilterDefPtr newFilter; char vmmacaddr[VIR_MAC_STRING_BUFLEN] = {0}; @@ -781,29 +769,22 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver, return -1; } - VIR_DEBUG("filter name: %s", filtername); + VIR_DEBUG("filter name: %s", binding->filter); if (!(obj = virNWFilterObjListFindInstantiateFilter(driver->nwfilters, - filtername))) + binding->filter))) return -1; - virMacAddrFormat(macaddr, vmmacaddr); + virMacAddrFormat(&binding->mac, vmmacaddr); - ipaddr = virNWFilterIPAddrMapGetIPAddr(ifname); + ipaddr = virNWFilterIPAddrMapGetIPAddr(binding->portdevname); - vars1 = virNWFilterCreateVarHashmap(vmmacaddr, ipaddr); - if (!vars1) { + if (virNWFilterVarHashmapAddStdValues(binding->filterparams, + vmmacaddr, ipaddr) < 0) { rc = -1; goto err_exit; } - vars = virNWFilterCreateVarsFrom(vars1, - filterparams); - if (!vars) { - rc = -1; - goto err_exit_vars1; - } - filter = virNWFilterObjGetDef(obj); switch (useNewFilter) { @@ -819,17 +800,11 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver, break; } - rc = virNWFilterDoInstantiate(vmuuid, techdriver, filter, - ifname, ifindex, linkdev, - vars, useNewFilter, foundNewFilter, - teardownOld, macaddr, driver, + rc = virNWFilterDoInstantiate(techdriver, binding, filter, + ifindex, useNewFilter, foundNewFilter, + teardownOld, driver, forceWithPendingReq); - virHashFree(vars); - - err_exit_vars1: - virHashFree(vars1); - err_exit: virNWFilterObjUnlock(obj); @@ -839,15 +814,11 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver, static int virNWFilterInstantiateFilterInternal(virNWFilterDriverStatePtr driver, - const unsigned char *vmuuid, - const virDomainNetDef *net, + virNWFilterBindingDefPtr binding, bool teardownOld, enum instCase useNewFilter, bool *foundNewFilter) { - const char *linkdev = (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) - ? net->data.direct.linkdev - : NULL; int ifindex; int rc; @@ -856,8 +827,8 @@ virNWFilterInstantiateFilterInternal(virNWFilterDriverStatePtr driver, /* after grabbing the filter update lock check for the interface; if it's not there anymore its filters will be or are being removed (while holding the lock) and we don't want to build new ones */ - if (virNetDevExists(net->ifname) != 1 || - virNetDevGetIndex(net->ifname, &ifindex) < 0) { + if (virNetDevExists(binding->portdevname) != 1 || + virNetDevGetIndex(binding->portdevname, &ifindex) < 0) { /* interfaces / VMs can disappear during filter instantiation; don't mark it as an error */ virResetLastError(); @@ -865,10 +836,10 @@ virNWFilterInstantiateFilterInternal(virNWFilterDriverStatePtr driver, goto cleanup; } - rc = virNWFilterInstantiateFilterUpdate(driver, vmuuid, teardownOld, - net->ifname, ifindex, linkdev, - &net->mac, net->filter, - net->filterparams, useNewFilter, + rc = virNWFilterInstantiateFilterUpdate(driver, teardownOld, + binding, + ifindex, + useNewFilter, false, foundNewFilter); cleanup: @@ -880,13 +851,8 @@ virNWFilterInstantiateFilterInternal(virNWFilterDriverStatePtr driver, int virNWFilterInstantiateFilterLate(virNWFilterDriverStatePtr driver, - const unsigned char *vmuuid, - const char *ifname, - int ifindex, - const char *linkdev, - const virMacAddr *macaddr, - const char *filtername, - virHashTablePtr filterparams) + virNWFilterBindingDefPtr binding, + int ifindex) { int rc; bool foundNewFilter = false; @@ -894,18 +860,17 @@ virNWFilterInstantiateFilterLate(virNWFilterDriverStatePtr driver, virNWFilterReadLockFilterUpdates(); virMutexLock(&updateMutex); - rc = virNWFilterInstantiateFilterUpdate(driver, vmuuid, true, - ifname, ifindex, linkdev, - macaddr, filtername, filterparams, + rc = virNWFilterInstantiateFilterUpdate(driver, true, + binding, ifindex, INSTANTIATE_ALWAYS, true, &foundNewFilter); if (rc < 0) { /* something went wrong... 'DOWN' the interface */ - if ((virNetDevValidateConfig(ifname, NULL, ifindex) <= 0) || - (virNetDevSetOnline(ifname, false) < 0)) { + if ((virNetDevValidateConfig(binding->portdevname, NULL, ifindex) <= 0) || + (virNetDevSetOnline(binding->portdevname, false) < 0)) { virResetLastError(); /* assuming interface disappeared... */ - _virNWFilterTeardownFilter(ifname); + _virNWFilterTeardownFilter(binding->portdevname); } } @@ -918,12 +883,11 @@ virNWFilterInstantiateFilterLate(virNWFilterDriverStatePtr driver, int virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver, - const unsigned char *vmuuid, - const virDomainNetDef *net) + virNWFilterBindingDefPtr binding) { bool foundNewFilter = false; - return virNWFilterInstantiateFilterInternal(driver, vmuuid, net, + return virNWFilterInstantiateFilterInternal(driver, binding, 1, INSTANTIATE_ALWAYS, &foundNewFilter); @@ -932,13 +896,12 @@ virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver, int virNWFilterUpdateInstantiateFilter(virNWFilterDriverStatePtr driver, - const unsigned char *vmuuid, - const virDomainNetDef *net, + virNWFilterBindingDefPtr binding, bool *skipIface) { bool foundNewFilter = false; - int rc = virNWFilterInstantiateFilterInternal(driver, vmuuid, net, + int rc = virNWFilterInstantiateFilterInternal(driver, binding, 0, INSTANTIATE_FOLLOW_NEWFILTER, &foundNewFilter); @@ -948,7 +911,7 @@ virNWFilterUpdateInstantiateFilter(virNWFilterDriverStatePtr driver, } static int -virNWFilterRollbackUpdateFilter(const virDomainNetDef *net) +virNWFilterRollbackUpdateFilter(virNWFilterBindingDefPtr binding) { const char *drvname = EBIPTABLES_DRIVER_ID; int ifindex; @@ -964,17 +927,17 @@ virNWFilterRollbackUpdateFilter(const virDomainNetDef *net) } /* don't tear anything while the address is being learned */ - if (virNetDevGetIndex(net->ifname, &ifindex) < 0) + if (virNetDevGetIndex(binding->portdevname, &ifindex) < 0) virResetLastError(); else if (virNWFilterHasLearnReq(ifindex)) return 0; - return techdriver->tearNewRules(net->ifname); + return techdriver->tearNewRules(binding->portdevname); } static int -virNWFilterTearOldFilter(virDomainNetDefPtr net) +virNWFilterTearOldFilter(virNWFilterBindingDefPtr binding) { const char *drvname = EBIPTABLES_DRIVER_ID; int ifindex; @@ -990,12 +953,12 @@ virNWFilterTearOldFilter(virDomainNetDefPtr net) } /* don't tear anything while the address is being learned */ - if (virNetDevGetIndex(net->ifname, &ifindex) < 0) + if (virNetDevGetIndex(binding->portdevname, &ifindex) < 0) virResetLastError(); else if (virNWFilterHasLearnReq(ifindex)) return 0; - return techdriver->tearOldRules(net->ifname); + return techdriver->tearOldRules(binding->portdevname); } @@ -1032,11 +995,11 @@ _virNWFilterTeardownFilter(const char *ifname) int -virNWFilterTeardownFilter(const virDomainNetDef *net) +virNWFilterTeardownFilter(virNWFilterBindingDefPtr binding) { int ret; virMutexLock(&updateMutex); - ret = _virNWFilterTeardownFilter(net->ifname); + ret = _virNWFilterTeardownFilter(binding->portdevname); virMutexUnlock(&updateMutex); return ret; } @@ -1057,12 +1020,16 @@ virNWFilterDomainFWUpdateCB(virDomainObjPtr obj, if (virDomainObjIsActive(obj)) { for (i = 0; i < vm->nnets; i++) { virDomainNetDefPtr net = vm->nets[i]; - if ((net->filter) && (net->ifname)) { + virNWFilterBindingDefPtr binding; + + if ((net->filter) && (net->ifname) && + (binding = virNWFilterBindingDefForNet( + vm->name, vm->uuid, net))) { + switch (cb->step) { case STEP_APPLY_NEW: ret = virNWFilterUpdateInstantiateFilter(cb->opaque, - vm->uuid, - net, + binding, &skipIface); if (ret == 0 && skipIface) { /* filter tree unchanged -- no update needed */ @@ -1074,24 +1041,24 @@ virNWFilterDomainFWUpdateCB(virDomainObjPtr obj, case STEP_TEAR_NEW: if (!virHashLookup(cb->skipInterfaces, net->ifname)) - ret = virNWFilterRollbackUpdateFilter(net); + ret = virNWFilterRollbackUpdateFilter(binding); break; case STEP_TEAR_OLD: if (!virHashLookup(cb->skipInterfaces, net->ifname)) - ret = virNWFilterTearOldFilter(net); + ret = virNWFilterTearOldFilter(binding); break; case STEP_APPLY_CURRENT: ret = virNWFilterInstantiateFilter(cb->opaque, - vm->uuid, - net); + binding); if (ret) virReportError(VIR_ERR_INTERNAL_ERROR, _("Failure while applying current filter on " "VM %s"), vm->name); break; } + virNWFilterBindingDefFree(binding); if (ret) break; } @@ -1101,3 +1068,45 @@ virNWFilterDomainFWUpdateCB(virDomainObjPtr obj, virObjectUnlock(obj); return ret; } + + +virNWFilterBindingDefPtr +virNWFilterBindingDefForNet(const char *vmname, + const unsigned char *vmuuid, + virDomainNetDefPtr net) +{ + virNWFilterBindingDefPtr ret; + + if (VIR_ALLOC(ret) < 0) + return NULL; + + if (VIR_STRDUP(ret->ownername, vmname) < 0) + goto error; + + memcpy(ret->owneruuid, vmuuid, sizeof(ret->owneruuid)); + + if (VIR_STRDUP(ret->portdevname, net->ifname) < 0) + goto error; + + if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT && + VIR_STRDUP(ret->linkdevname, net->data.direct.linkdev) < 0) + goto error; + + ret->mac = net->mac; + + if (VIR_STRDUP(ret->filter, net->filter) < 0) + goto error; + + if (!(ret->filterparams = virNWFilterHashTableCreate(0))) + goto error; + + if (net->filterparams && + virNWFilterHashTablePutAll(net->filterparams, ret->filterparams) < 0) + goto error; + + return ret; + + error: + virNWFilterBindingDefFree(ret); + return NULL; +} diff --git a/src/nwfilter/nwfilter_gentech_driver.h b/src/nwfilter/nwfilter_gentech_driver.h index 9e43a159c3..6b51096a0d 100644 --- a/src/nwfilter/nwfilter_gentech_driver.h +++ b/src/nwfilter/nwfilter_gentech_driver.h @@ -25,6 +25,7 @@ # define __NWFILTER_GENTECH_DRIVER_H # include "virnwfilterobj.h" +# include "virnwfilterbindingdef.h" # include "nwfilter_tech_driver.h" virNWFilterTechDriverPtr virNWFilterTechDriverForName(const char *name); @@ -39,23 +40,16 @@ enum instCase { int virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver, - const unsigned char *vmuuid, - const virDomainNetDef *net); + virNWFilterBindingDefPtr binding); int virNWFilterUpdateInstantiateFilter(virNWFilterDriverStatePtr driver, - const unsigned char *vmuuid, - const virDomainNetDef *net, + virNWFilterBindingDefPtr binding, bool *skipIface); int virNWFilterInstantiateFilterLate(virNWFilterDriverStatePtr driver, - const unsigned char *vmuuid, - const char *ifname, - int ifindex, - const char *linkdev, - const virMacAddr *macaddr, - const char *filtername, - virHashTablePtr filterparams); + virNWFilterBindingDefPtr binding, + int ifindex); -int virNWFilterTeardownFilter(const virDomainNetDef *net); +int virNWFilterTeardownFilter(virNWFilterBindingDefPtr binding); virHashTablePtr virNWFilterCreateVarHashmap(const char *macaddr, const virNWFilterVarValue *value); @@ -63,4 +57,8 @@ virHashTablePtr virNWFilterCreateVarHashmap(const char *macaddr, int virNWFilterDomainFWUpdateCB(virDomainObjPtr vm, void *data); +virNWFilterBindingDefPtr virNWFilterBindingDefForNet(const char *vmname, + const unsigned char *vmuuid, + virDomainNetDefPtr net); + #endif diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index cc3bfd971c..5080b73004 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -643,19 +643,21 @@ learnIPAddressThread(void *arg) virNWFilterUnlockIface(req->ifname); if ((inetaddr = virSocketAddrFormat(&sa)) != NULL) { + virNWFilterBindingDef binding = { + .portdevname = req->ifname, + .linkdevname = req->linkdev, + .mac = req->macaddr, + .filter = req->filtername, + .filterparams = req->filterparams, + }; if (virNWFilterIPAddrMapAddIPAddr(req->ifname, inetaddr) < 0) { VIR_ERROR(_("Failed to add IP address %s to IP address " "cache for interface %s"), inetaddr, req->ifname); } ret = virNWFilterInstantiateFilterLate(req->driver, - NULL, - req->ifname, - req->ifindex, - req->linkdev, - &req->macaddr, - req->filtername, - req->filterparams); + &binding, + req->ifindex); VIR_DEBUG("Result from applying firewall rules on " "%s with IP addr %s : %d", req->ifname, inetaddr, ret); VIR_FREE(inetaddr); -- 2.17.0

Use the virNWFilterBindingDefPTr struct in the IP address learning code directly. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/nwfilter/nwfilter_gentech_driver.c | 7 +- src/nwfilter/nwfilter_learnipaddr.c | 98 +++++++------------------- src/nwfilter/nwfilter_learnipaddr.h | 7 +- 3 files changed, 31 insertions(+), 81 deletions(-) diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index dc925dee16..80b80d3a28 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -653,12 +653,9 @@ virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver, } else if (STRCASEEQ(learning, "any")) { if (!virNWFilterHasLearnReq(ifindex)) { rc = virNWFilterLearnIPAddress(techdriver, - binding->portdevname, + binding, ifindex, - binding->linkdevname, - &binding->mac, - filter->name, - binding->filterparams, driver, + driver, DETECT_DHCP|DETECT_STATIC); } goto err_exit; diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index 5080b73004..2149b4af69 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -137,12 +137,8 @@ typedef struct _virNWFilterIPAddrLearnReq virNWFilterIPAddrLearnReq; typedef virNWFilterIPAddrLearnReq *virNWFilterIPAddrLearnReqPtr; struct _virNWFilterIPAddrLearnReq { virNWFilterTechDriverPtr techdriver; - char ifname[IF_NAMESIZE]; int ifindex; - char linkdev[IF_NAMESIZE]; - virMacAddr macaddr; - char *filtername; - virHashTablePtr filterparams; + virNWFilterBindingDefPtr binding; virNWFilterDriverStatePtr driver; enum howDetect howDetect; @@ -232,8 +228,7 @@ virNWFilterIPAddrLearnReqFree(virNWFilterIPAddrLearnReqPtr req) if (!req) return; - VIR_FREE(req->filtername); - virHashFree(req->filterparams); + virNWFilterBindingDefFree(req->binding); VIR_FREE(req); } @@ -404,8 +399,9 @@ learnIPAddressThread(void *arg) virNWFilterIPAddrLearnReqPtr req = arg; uint32_t vmaddr = 0, bcastaddr = 0; unsigned int ethHdrSize; - char *listen_if = (strlen(req->linkdev) != 0) ? req->linkdev - : req->ifname; + char *listen_if = (req->binding->linkdevname ? + req->binding->linkdevname : + req->binding->portdevname); int dhcp_opts_len; char macaddr[VIR_MAC_STRING_BUFLEN]; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -415,13 +411,13 @@ learnIPAddressThread(void *arg) enum howDetect howDetected = 0; virNWFilterTechDriverPtr techdriver = req->techdriver; - if (virNWFilterLockIface(req->ifname) < 0) + if (virNWFilterLockIface(req->binding->portdevname) < 0) goto err_no_lock; req->status = 0; /* anything change to the VM's interface -- check at least once */ - if (virNetDevValidateConfig(req->ifname, NULL, req->ifindex) <= 0) { + if (virNetDevValidateConfig(req->binding->portdevname, NULL, req->ifindex) <= 0) { virResetLastError(); req->status = ENODEV; goto done; @@ -435,12 +431,12 @@ learnIPAddressThread(void *arg) goto done; } - virMacAddrFormat(&req->macaddr, macaddr); + virMacAddrFormat(&req->binding->mac, macaddr); switch (req->howDetect) { case DETECT_DHCP: - if (techdriver->applyDHCPOnlyRules(req->ifname, - &req->macaddr, + if (techdriver->applyDHCPOnlyRules(req->binding->portdevname, + &req->binding->mac, NULL, false) < 0) { req->status = EINVAL; goto done; @@ -448,8 +444,8 @@ learnIPAddressThread(void *arg) virBufferAddLit(&buf, "src port 67 and dst port 68"); break; case DETECT_STATIC: - if (techdriver->applyBasicRules(req->ifname, - &req->macaddr) < 0) { + if (techdriver->applyBasicRules(req->binding->portdevname, + &req->binding->mac) < 0) { req->status = EINVAL; goto done; } @@ -495,7 +491,7 @@ learnIPAddressThread(void *arg) } /* check whether VM's dev is still there */ - if (virNetDevValidateConfig(req->ifname, NULL, req->ifindex) <= 0) { + if (virNetDevValidateConfig(req->binding->portdevname, NULL, req->ifindex) <= 0) { virResetLastError(); req->status = ENODEV; showError = false; @@ -527,7 +523,7 @@ learnIPAddressThread(void *arg) continue; } - if (virMacAddrCmpRaw(&req->macaddr, ether_hdr->ether_shost) == 0) { + if (virMacAddrCmpRaw(&req->binding->mac, ether_hdr->ether_shost) == 0) { /* packets from the VM */ if (etherType == ETHERTYPE_IP && @@ -566,7 +562,7 @@ learnIPAddressThread(void *arg) break; } } - } else if (virMacAddrCmpRaw(&req->macaddr, + } else if (virMacAddrCmpRaw(&req->binding->mac, ether_hdr->ether_dhost) == 0 || /* allow Broadcast replies from DHCP server */ virMacAddrIsBroadcastRaw(ether_hdr->ether_dhost)) { @@ -596,7 +592,7 @@ learnIPAddressThread(void *arg) ((char *)udphdr + sizeof(udphdr)); if (dhcp->op == 2 /* BOOTREPLY */ && virMacAddrCmpRaw( - &req->macaddr, + &req->binding->mac, &dhcp->chaddr[0]) == 0) { dhcp_opts_len = header.len - (ethHdrSize + iphdr->ihl * 4 + @@ -640,26 +636,19 @@ learnIPAddressThread(void *arg) * Also it is safe to unlock interface here because we stopped * capturing and applied necessary rules on the interface, while * instantiating a new filter doesn't require a locked interface.*/ - virNWFilterUnlockIface(req->ifname); + virNWFilterUnlockIface(req->binding->portdevname); if ((inetaddr = virSocketAddrFormat(&sa)) != NULL) { - virNWFilterBindingDef binding = { - .portdevname = req->ifname, - .linkdevname = req->linkdev, - .mac = req->macaddr, - .filter = req->filtername, - .filterparams = req->filterparams, - }; - if (virNWFilterIPAddrMapAddIPAddr(req->ifname, inetaddr) < 0) { + if (virNWFilterIPAddrMapAddIPAddr(req->binding->portdevname, inetaddr) < 0) { VIR_ERROR(_("Failed to add IP address %s to IP address " - "cache for interface %s"), inetaddr, req->ifname); + "cache for interface %s"), inetaddr, req->binding->portdevname); } ret = virNWFilterInstantiateFilterLate(req->driver, - &binding, + req->binding, req->ifindex); VIR_DEBUG("Result from applying firewall rules on " - "%s with IP addr %s : %d", req->ifname, inetaddr, ret); + "%s with IP addr %s : %d", req->binding->portdevname, inetaddr, ret); VIR_FREE(inetaddr); } } else { @@ -667,13 +656,13 @@ learnIPAddressThread(void *arg) virReportSystemError(req->status, _("encountered an error on interface %s " "index %d"), - req->ifname, req->ifindex); + req->binding->portdevname, req->ifindex); - techdriver->applyDropAllRules(req->ifname); - virNWFilterUnlockIface(req->ifname); + techdriver->applyDropAllRules(req->binding->portdevname); + virNWFilterUnlockIface(req->binding->portdevname); } - VIR_DEBUG("pcap thread terminating for interface %s", req->ifname); + VIR_DEBUG("pcap thread terminating for interface %s", req->binding->portdevname); err_no_lock: @@ -706,19 +695,14 @@ learnIPAddressThread(void *arg) */ int virNWFilterLearnIPAddress(virNWFilterTechDriverPtr techdriver, - const char *ifname, + virNWFilterBindingDefPtr binding, int ifindex, - const char *linkdev, - const virMacAddr *macaddr, - const char *filtername, - virHashTablePtr filterparams, virNWFilterDriverStatePtr driver, enum howDetect howDetect) { int rc; virThread thread; virNWFilterIPAddrLearnReqPtr req = NULL; - virHashTablePtr ht = NULL; if (howDetect == 0) return -1; @@ -734,37 +718,11 @@ virNWFilterLearnIPAddress(virNWFilterTechDriverPtr techdriver, if (VIR_ALLOC(req) < 0) goto err_no_req; - ht = virNWFilterHashTableCreate(0); - if (ht == NULL) + if (!(req->binding = virNWFilterBindingDefCopy(binding))) goto err_free_req; - if (virNWFilterHashTablePutAll(filterparams, ht) < 0) - goto err_free_ht; - - if (VIR_STRDUP(req->filtername, filtername) < 0) - goto err_free_ht; - - if (virStrcpyStatic(req->ifname, ifname) == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Destination buffer for ifname ('%s') " - "not large enough"), ifname); - goto err_free_ht; - } - - if (linkdev) { - if (virStrcpyStatic(req->linkdev, linkdev) == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Destination buffer for linkdev ('%s') " - "not large enough"), linkdev); - goto err_free_ht; - } - } - req->ifindex = ifindex; - virMacAddrSet(&req->macaddr, macaddr); req->driver = driver; - req->filterparams = ht; - ht = NULL; req->howDetect = howDetect; req->techdriver = techdriver; @@ -783,8 +741,6 @@ virNWFilterLearnIPAddress(virNWFilterTechDriverPtr techdriver, err_dereg_req: virNWFilterDeregisterLearnReq(ifindex); - err_free_ht: - virHashFree(ht); err_free_req: virNWFilterIPAddrLearnReqFree(req); err_no_req: diff --git a/src/nwfilter/nwfilter_learnipaddr.h b/src/nwfilter/nwfilter_learnipaddr.h index 06fea5bff8..412855e66c 100644 --- a/src/nwfilter/nwfilter_learnipaddr.h +++ b/src/nwfilter/nwfilter_learnipaddr.h @@ -28,6 +28,7 @@ # include "conf/nwfilter_params.h" # include "nwfilter_tech_driver.h" +# include "virnwfilterbindingdef.h" # include <net/if.h> enum howDetect { @@ -36,12 +37,8 @@ enum howDetect { }; int virNWFilterLearnIPAddress(virNWFilterTechDriverPtr techdriver, - const char *ifname, + virNWFilterBindingDefPtr binding, int ifindex, - const char *linkdev, - const virMacAddr *macaddr, - const char *filtername, - virHashTablePtr filterparams, virNWFilterDriverStatePtr driver, enum howDetect howDetect); -- 2.17.0

Use the virNWFilterBindingDefPtr struct in the DHCP address snooping code directly. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/nwfilter/nwfilter_dhcpsnoop.c | 150 ++++++++++--------------- src/nwfilter/nwfilter_dhcpsnoop.h | 7 +- src/nwfilter/nwfilter_gentech_driver.c | 7 +- 3 files changed, 61 insertions(+), 103 deletions(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index f24fec1638..f6bcc3bcc7 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -135,13 +135,9 @@ struct _virNWFilterSnoopReq { int refctr; virNWFilterTechDriverPtr techdriver; - char *ifname; + virNWFilterBindingDefPtr binding; int ifindex; - char *linkdev; char ifkey[VIR_IFKEY_LEN]; - virMacAddr macaddr; - char *filtername; - virHashTablePtr vars; virNWFilterDriverStatePtr driver; /* start and end of lease list, ordered by lease time */ virNWFilterSnoopIPLeasePtr start; @@ -484,10 +480,10 @@ virNWFilterSnoopIPLeaseInstallRule(virNWFilterSnoopIPLeasePtr ipl, req = ipl->snoopReq; - /* protect req->ifname */ + /* protect req->binding->portdevname */ virNWFilterSnoopReqLock(req); - if (virNWFilterIPAddrMapAddIPAddr(req->ifname, ipaddr) < 0) + if (virNWFilterIPAddrMapAddIPAddr(req->binding->portdevname, ipaddr) < 0) goto exit_snooprequnlock; if (!instantiate) { @@ -497,16 +493,9 @@ virNWFilterSnoopIPLeaseInstallRule(virNWFilterSnoopIPLeasePtr ipl, /* instantiate the filters */ - if (req->ifname) { - virNWFilterBindingDef binding = { - .portdevname = req->ifname, - .linkdevname = req->linkdev, - .mac = req->macaddr, - .filter = req->filtername, - .filterparams = req->vars, - }; + if (req->binding->portdevname) { rc = virNWFilterInstantiateFilterLate(req->driver, - &binding, + req->binding, req->ifindex); } @@ -647,10 +636,7 @@ virNWFilterSnoopReqFree(virNWFilterSnoopReqPtr req) virNWFilterSnoopReqLeaseDel(req, &ipl->ipAddress, false, false); /* free all req data */ - VIR_FREE(req->ifname); - VIR_FREE(req->linkdev); - VIR_FREE(req->filtername); - virHashFree(req->vars); + virNWFilterBindingDefFree(req->binding); virMutexDestroy(&req->lock); virCondDestroy(&req->threadStatusCond); @@ -881,28 +867,23 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req, if (update_leasefile) virNWFilterSnoopLeaseFileSave(ipl); - ipAddrLeft = virNWFilterIPAddrMapDelIPAddr(req->ifname, ipstr); + ipAddrLeft = virNWFilterIPAddrMapDelIPAddr(req->binding->portdevname, ipstr); if (!req->threadkey || !instantiate) goto skip_instantiate; if (ipAddrLeft) { - virNWFilterBindingDef binding = { - .portdevname = req->ifname, - .linkdevname = req->linkdev, - .mac = req->macaddr, - .filter = req->filtername, - .filterparams = req->vars, - }; ret = virNWFilterInstantiateFilterLate(req->driver, - &binding, + req->binding, req->ifindex); } else { virNWFilterVarValuePtr dhcpsrvrs = - virHashLookup(req->vars, NWFILTER_VARNAME_DHCPSERVER); + virHashLookup(req->binding->filterparams, + NWFILTER_VARNAME_DHCPSERVER); if (req->techdriver && - req->techdriver->applyDHCPOnlyRules(req->ifname, &req->macaddr, + req->techdriver->applyDHCPOnlyRules(req->binding->portdevname, + &req->binding->mac, dhcpsrvrs, false) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("virNWFilterSnoopListDel failed")); @@ -1032,7 +1013,7 @@ virNWFilterSnoopDHCPDecode(virNWFilterSnoopReqPtr req, * inside the DHCP response */ if (!fromVM) { - if (virMacAddrCmpRaw(&req->macaddr, + if (virMacAddrCmpRaw(&req->binding->mac, (unsigned char *)&pd->d_chaddr) != 0) return -2; } @@ -1194,7 +1175,7 @@ static void virNWFilterDHCPDecodeWorker(void *jobdata, void *opaque) virReportError(VIR_ERR_INTERNAL_ERROR, _("Instantiation of rules failed on " - "interface '%s'"), req->ifname); + "interface '%s'"), req->binding->portdevname); } virAtomicIntDecAndTest(job->qCtr); VIR_FREE(job); @@ -1403,13 +1384,14 @@ virNWFilterDHCPSnoopThread(void *req0) /* whoever started us increased the reference counter for the req for us */ - /* protect req->ifname & req->threadkey */ + /* protect req->binding->portdevname & req->threadkey */ virNWFilterSnoopReqLock(req); - if (req->ifname && req->threadkey) { + if (req->binding->portdevname && req->threadkey) { for (i = 0; i < ARRAY_CARDINALITY(pcapConf); i++) { pcapConf[i].handle = - virNWFilterSnoopDHCPOpen(req->ifname, &req->macaddr, + virNWFilterSnoopDHCPOpen(req->binding->portdevname, + &req->binding->mac, pcapConf[i].filter, pcapConf[i].dir); if (!pcapConf[i].handle) { @@ -1418,7 +1400,7 @@ virNWFilterDHCPSnoopThread(void *req0) } fds[i].fd = pcap_fileno(pcapConf[i].handle); } - tmp = virNetDevGetIndex(req->ifname, &ifindex); + tmp = virNetDevGetIndex(req->binding->portdevname, &ifindex); ignore_value(VIR_STRDUP(threadkey, req->threadkey)); worker = virThreadPoolNew(1, 1, 0, virNWFilterDHCPDecodeWorker, @@ -1483,11 +1465,11 @@ virNWFilterDHCPSnoopThread(void *req0) /* error reading from socket */ tmp = -1; - /* protect req->ifname */ + /* protect req->binding->portdevname */ virNWFilterSnoopReqLock(req); - if (req->ifname) - tmp = virNetDevValidateConfig(req->ifname, NULL, ifindex); + if (req->binding->portdevname) + tmp = virNetDevValidateConfig(req->binding->portdevname, NULL, ifindex); virNWFilterSnoopReqUnlock(req); @@ -1500,16 +1482,17 @@ virNWFilterDHCPSnoopThread(void *req0) pcap_close(pcapConf[i].handle); pcapConf[i].handle = NULL; - /* protect req->ifname */ + /* protect req->binding->portdevname */ virNWFilterSnoopReqLock(req); virReportError(VIR_ERR_INTERNAL_ERROR, _("interface '%s' failing; " "reopening"), - req->ifname); - if (req->ifname) + req->binding->portdevname); + if (req->binding->portdevname) pcapConf[i].handle = - virNWFilterSnoopDHCPOpen(req->ifname, &req->macaddr, + virNWFilterSnoopDHCPOpen(req->binding->portdevname, + &req->binding->mac, pcapConf[i].filter, pcapConf[i].dir); @@ -1535,7 +1518,7 @@ virNWFilterDHCPSnoopThread(void *req0) last_displayed_queue = time(0); VIR_WARN("Worker thread for interface '%s' has a " "job queue that is too long", - req->ifname); + req->binding->portdevname); } continue; } @@ -1548,7 +1531,7 @@ virNWFilterDHCPSnoopThread(void *req0) if (time(0) - last_displayed > 10) { last_displayed = time(0); VIR_WARN("Too many DHCP packets on interface '%s'", - req->ifname); + req->binding->portdevname); } continue; } @@ -1559,7 +1542,7 @@ virNWFilterDHCPSnoopThread(void *req0) &pcapConf[i].qCtr) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Job submission failed on " - "interface '%s'"), req->ifname); + "interface '%s'"), req->binding->portdevname); error = true; break; } @@ -1570,15 +1553,15 @@ virNWFilterDHCPSnoopThread(void *req0) /* protect IfNameToKey */ virNWFilterSnoopLock(); - /* protect req->ifname & req->threadkey */ + /* protect req->binding->portdevname & req->threadkey */ virNWFilterSnoopReqLock(req); virNWFilterSnoopCancel(&req->threadkey); ignore_value(virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, - req->ifname)); + req->binding->portdevname)); - VIR_FREE(req->ifname); + VIR_FREE(req->binding->portdevname); virNWFilterSnoopReqUnlock(req); virNWFilterSnoopUnlock(); @@ -1611,12 +1594,7 @@ virNWFilterSnoopIFKeyFMT(char *ifkey, const unsigned char *vmuuid, int virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, - const char *ifname, - const char *linkdev, - const unsigned char *vmuuid, - const virMacAddr *macaddr, - const char *filtername, - virHashTablePtr filterparams, + virNWFilterBindingDefPtr binding, virNWFilterDriverStatePtr driver) { virNWFilterSnoopReqPtr req; @@ -1627,7 +1605,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, virNWFilterVarValuePtr dhcpsrvrs; bool threadPuts = false; - virNWFilterSnoopIFKeyFMT(ifkey, vmuuid, macaddr); + virNWFilterSnoopIFKeyFMT(ifkey, binding->owneruuid, &binding->mac); req = virNWFilterSnoopReqGetByIFKey(ifkey); isnewreq = (req == NULL); @@ -1636,9 +1614,8 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, virNWFilterSnoopReqPut(req); return 0; } - /* a recycled req may still have filtername and vars */ - VIR_FREE(req->filtername); - virHashFree(req->vars); + virNWFilterBindingDefFree(req->binding); + req->binding = NULL; } else { req = virNWFilterSnoopReqNew(ifkey); if (!req) @@ -1647,17 +1624,9 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, req->driver = driver; req->techdriver = techdriver; - tmp = virNetDevGetIndex(ifname, &req->ifindex); - virMacAddrSet(&req->macaddr, macaddr); - req->vars = virNWFilterHashTableCreate(0); - req->linkdev = NULL; - - if (VIR_STRDUP(req->ifname, ifname) < 0 || - VIR_STRDUP(req->filtername, filtername) < 0 || - VIR_STRDUP(req->linkdev, linkdev) < 0) + if ((tmp = virNetDevGetIndex(binding->portdevname, &req->ifindex)) < 0) goto exit_snoopreqput; - - if (!req->vars || tmp < 0) + if (!(req->binding = virNWFilterBindingDefCopy(binding))) goto exit_snoopreqput; /* check that all tools are available for applying the filters (late) */ @@ -1669,10 +1638,11 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, goto exit_snoopreqput; } - dhcpsrvrs = virHashLookup(filterparams, + dhcpsrvrs = virHashLookup(binding->filterparams, NWFILTER_VARNAME_DHCPSERVER); - if (techdriver->applyDHCPOnlyRules(req->ifname, &req->macaddr, + if (techdriver->applyDHCPOnlyRules(req->binding->portdevname, + &req->binding->mac, dhcpsrvrs, false) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("applyDHCPOnlyRules " @@ -1680,20 +1650,14 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, goto exit_snoopreqput; } - if (virNWFilterHashTablePutAll(filterparams, req->vars) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("virNWFilterDHCPSnoopReq: can't copy variables" - " on if %s"), ifkey); - goto exit_snoopreqput; - } - virNWFilterSnoopLock(); - if (virHashAddEntry(virNWFilterSnoopState.ifnameToKey, ifname, + if (virHashAddEntry(virNWFilterSnoopState.ifnameToKey, + req->binding->portdevname, req->ifkey) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("virNWFilterDHCPSnoopReq ifname map failed" - " on interface \"%s\" key \"%s\""), ifname, + " on interface \"%s\" key \"%s\""), binding->portdevname, ifkey); goto exit_snoopunlock; } @@ -1702,7 +1666,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, virHashAddEntry(virNWFilterSnoopState.snoopReqs, ifkey, req) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("virNWFilterDHCPSnoopReq req add failed on" - " interface \"%s\" ifkey \"%s\""), ifname, + " interface \"%s\" ifkey \"%s\""), binding->portdevname, ifkey); goto exit_rem_ifnametokey; } @@ -1714,7 +1678,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, req) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("virNWFilterDHCPSnoopReq virThreadCreate " - "failed on interface '%s'"), ifname); + "failed on interface '%s'"), binding->portdevname); goto exit_snoopreq_unlock; } @@ -1726,14 +1690,14 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, if (!req->threadkey) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Activation of snoop request failed on " - "interface '%s'"), req->ifname); + "interface '%s'"), req->binding->portdevname); goto exit_snoopreq_unlock; } if (virNWFilterSnoopReqRestore(req) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Restoring of leases failed on " - "interface '%s'"), req->ifname); + "interface '%s'"), req->binding->portdevname); goto exit_snoop_cancel; } @@ -1762,7 +1726,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, exit_snoopreq_unlock: virNWFilterSnoopReqUnlock(req); exit_rem_ifnametokey: - virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, ifname); + virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, binding->portdevname); exit_snoopunlock: virNWFilterSnoopUnlock(); exit_snoopreqput: @@ -2070,21 +2034,21 @@ virNWFilterSnoopRemAllReqIter(const void *payload, { virNWFilterSnoopReqPtr req = (virNWFilterSnoopReqPtr)payload; - /* protect req->ifname */ + /* protect req->binding->portdevname */ virNWFilterSnoopReqLock(req); - if (req->ifname) { + if (req->binding->portdevname) { ignore_value(virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, - req->ifname)); + req->binding->portdevname)); /* * Remove all IP addresses known to be associated with this * interface so that a new thread will be started on this * interface */ - virNWFilterIPAddrMapDelIPAddr(req->ifname, NULL); + virNWFilterIPAddrMapDelIPAddr(req->binding->portdevname, NULL); - VIR_FREE(req->ifname); + VIR_FREE(req->binding->portdevname); } virNWFilterSnoopReqUnlock(req); @@ -2187,13 +2151,13 @@ virNWFilterDHCPSnoopEnd(const char *ifname) goto cleanup; } - /* protect req->ifname & req->threadkey */ + /* protect req->binding->portdevname & req->threadkey */ virNWFilterSnoopReqLock(req); /* keep valid lease req; drop interface association */ virNWFilterSnoopCancel(&req->threadkey); - VIR_FREE(req->ifname); + VIR_FREE(req->binding->portdevname); virNWFilterSnoopReqUnlock(req); diff --git a/src/nwfilter/nwfilter_dhcpsnoop.h b/src/nwfilter/nwfilter_dhcpsnoop.h index a5925de40a..c693e1adbd 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.h +++ b/src/nwfilter/nwfilter_dhcpsnoop.h @@ -30,12 +30,7 @@ int virNWFilterDHCPSnoopInit(void); void virNWFilterDHCPSnoopShutdown(void); int virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, - const char *ifname, - const char *linkdev, - const unsigned char *vmuuid, - const virMacAddr *macaddr, - const char *filtername, - virHashTablePtr filterparams, + virNWFilterBindingDefPtr binding, virNWFilterDriverStatePtr driver); void virNWFilterDHCPSnoopEnd(const char *ifname); #endif /* __NWFILTER_DHCPSNOOP_H */ diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 80b80d3a28..30ae3970fb 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -645,10 +645,9 @@ virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver, goto err_unresolvable_vars; } if (STRCASEEQ(learning, "dhcp")) { - rc = virNWFilterDHCPSnoopReq(techdriver, binding->portdevname, - binding->linkdevname, - binding->owneruuid, &binding->mac, - filter->name, binding->filterparams, driver); + rc = virNWFilterDHCPSnoopReq(techdriver, + binding, + driver); goto err_exit; } else if (STRCASEEQ(learning, "any")) { if (!virNWFilterHasLearnReq(ifindex)) { -- 2.17.0

If a <interface> includes a filter name but the nwfilter driver is not present we silently do nothing. This is very bad, because an application that thinks it is protected by malicious guest traffic will in fact be vulnerable. Reporting an error gives the administrator the ability to know there is a problem and fix it. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/conf/domain_nwfilter.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c index e360aceeba..7570e0ae83 100644 --- a/src/conf/domain_nwfilter.c +++ b/src/conf/domain_nwfilter.c @@ -28,6 +28,9 @@ #include "datatypes.h" #include "domain_conf.h" #include "domain_nwfilter.h" +#include "virerror.h" + +#define VIR_FROM_THIS VIR_FROM_NWFILTER static virDomainConfNWFilterDriverPtr nwfilterDriver; @@ -44,8 +47,10 @@ virDomainConfNWFilterInstantiate(const char *vmname, { if (nwfilterDriver != NULL) return nwfilterDriver->instantiateFilter(vmname, vmuuid, net); - /* driver module not available -- don't indicate failure */ - return 0; + + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("No network filter driver available")); + return -1; } void -- 2.17.0

Introduce a new struct to act as the stateful owner of the virNWFilterBindingDefPtr objects. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/conf/Makefile.inc.am | 2 + src/conf/virnwfilterbindingobj.c | 260 +++++++++++++++++++++++++++++++ src/conf/virnwfilterbindingobj.h | 60 +++++++ src/libvirt_private.syms | 10 ++ 4 files changed, 332 insertions(+) create mode 100644 src/conf/virnwfilterbindingobj.c create mode 100644 src/conf/virnwfilterbindingobj.h diff --git a/src/conf/Makefile.inc.am b/src/conf/Makefile.inc.am index f5fb323233..3d55ba688d 100644 --- a/src/conf/Makefile.inc.am +++ b/src/conf/Makefile.inc.am @@ -87,6 +87,8 @@ NWFILTER_CONF_SOURCES = \ conf/virnwfilterobj.h \ conf/virnwfilterbindingdef.c \ conf/virnwfilterbindingdef.h \ + conf/virnwfilterbindingobj.c \ + conf/virnwfilterbindingobj.h \ $(NULL) STORAGE_CONF_SOURCES = \ diff --git a/src/conf/virnwfilterbindingobj.c b/src/conf/virnwfilterbindingobj.c new file mode 100644 index 0000000000..15aaf89b5c --- /dev/null +++ b/src/conf/virnwfilterbindingobj.c @@ -0,0 +1,260 @@ +/* + * virnwfilterbindingobj.c: network filter binding XML processing + * + * Copyright (C) 2018 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#include <config.h> + +#include "viralloc.h" +#include "virerror.h" +#include "virstring.h" +#include "nwfilter_params.h" +#include "virnwfilterbindingobj.h" +#include "viruuid.h" +#include "virfile.h" + + +#define VIR_FROM_THIS VIR_FROM_NWFILTER + +static virClassPtr virNWFilterBindingObjClass; +static void virNWFilterBindingObjDispose(void *obj); + +static int virNWFilterBindingObjOnceInit(void) +{ + if (!VIR_CLASS_NEW(virNWFilterBindingObj, virClassForObjectLockable())) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNWFilterBindingObj) + +virNWFilterBindingObjPtr +virNWFilterBindingObjNew(void) +{ + if (virNWFilterBindingObjInitialize() < 0) + return NULL; + + return virObjectNew(virNWFilterBindingObjClass); +} + +void +virNWFilterBindingObjDispose(void *obj) +{ + virNWFilterBindingObjPtr bobj = obj; + + virNWFilterBindingDefFree(bobj->def); +} + + +/** + * virNWFilterBindingnObjEndAPI: + * @obj: binding object + * + * Finish working with a binding object in an API. This function + * clears whatever was left of a domain that was gathered using + * virNWFilterBindingObjListFindByPortDev(). Currently that means + * only unlocking and decrementing the reference counter of that + * object. And in order to make sure the caller does not access + * the object, the pointer is cleared. + */ +void +virNWFilterBindingObjEndAPI(virNWFilterBindingObjPtr *obj) +{ + if (!*obj) + return; + + virObjectUnlock(*obj); + virObjectUnref(*obj); + *obj = NULL; +} + + +char * +virNWFilterBindingObjConfigFile(const char *dir, + const char *name) +{ + char *ret; + + ignore_value(virAsprintf(&ret, "%s/%s.xml", dir, name)); + return ret; +} + + +int +virNWFilterBindingObjSave(const virNWFilterBindingObj *obj, + const char *statusDir) +{ + char *filename; + char *xml = NULL; + int ret = -1; + + if (!(filename = virNWFilterBindingObjConfigFile(statusDir, + obj->def->portdevname))) + return -1; + + if (!(xml = virNWFilterBindingObjFormat(obj))) + goto cleanup; + + if (virFileMakePath(statusDir) < 0) { + virReportSystemError(errno, + _("cannot create config directory '%s'"), + statusDir); + goto cleanup; + } + + ret = virXMLSaveFile(filename, + obj->def->portdevname, "nwfilter-binding-create", + xml); + + cleanup: + VIR_FREE(xml); + VIR_FREE(filename); + return ret; +} + + +int +virNWFilterBindingObjDelete(const virNWFilterBindingObj *obj, + const char *statusDir) +{ + char *filename; + int ret = -1; + + if (!(filename = virNWFilterBindingObjConfigFile(statusDir, + obj->def->portdevname))) + return -1; + + if (unlink(filename) < 0 && + errno != ENOENT) { + virReportSystemError(errno, + _("Unable to remove status '%s' for nwfilter binding %s'"), + filename, obj->def->portdevname); + goto cleanup; + } + + ret = 0; + + cleanup: + VIR_FREE(filename); + return ret; +} + + +static virNWFilterBindingObjPtr +virNWFilterBindingObjParseXML(xmlDocPtr doc, + xmlXPathContextPtr ctxt) +{ + virNWFilterBindingObjPtr ret; + xmlNodePtr node; + + if (VIR_ALLOC(ret) < 0) + return NULL; + + if (!(node = virXPathNode("./filterbinding", ctxt))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("filter binding status missing binding")); + goto cleanup; + } + + if (!(ret->def = virNWFilterBindingDefParseNode(doc, node))) + goto cleanup; + + return ret; + + cleanup: + virObjectUnref(ret); + return NULL; +} + + +static virNWFilterBindingObjPtr +virNWFilterBindingObjParseNode(xmlDocPtr doc, + xmlNodePtr root) +{ + xmlXPathContextPtr ctxt = NULL; + virNWFilterBindingObjPtr obj = NULL; + + if (STRNEQ((const char *)root->name, "filterbindingb")) { + virReportError(VIR_ERR_XML_ERROR, + "%s", + _("unknown root element for nw filter")); + goto cleanup; + } + + ctxt = xmlXPathNewContext(doc); + if (ctxt == NULL) { + virReportOOMError(); + goto cleanup; + } + + ctxt->node = root; + obj = virNWFilterBindingObjParseXML(doc, ctxt); + + cleanup: + xmlXPathFreeContext(ctxt); + return obj; +} + + +static virNWFilterBindingObjPtr +virNWFilterBindingObjParse(const char *xmlStr, + const char *filename) +{ + virNWFilterBindingObjPtr obj = NULL; + xmlDocPtr xml; + + if ((xml = virXMLParse(filename, xmlStr, _("(nwfilterbinding_status)")))) { + obj = virNWFilterBindingObjParseNode(xml, xmlDocGetRootElement(xml)); + xmlFreeDoc(xml); + } + + return obj; +} + + + +virNWFilterBindingObjPtr +virNWFilterBindingObjParseFile(const char *filename) +{ + return virNWFilterBindingObjParse(NULL, filename); +} + + +char * +virNWFilterBindingObjFormat(const virNWFilterBindingObj *obj) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + virBufferAddLit(&buf, "<filterbindingstatus>\n"); + + virBufferAdjustIndent(&buf, 2); + + if (virNWFilterBindingDefFormatBuf(&buf, obj->def) < 0) { + virBufferFreeAndReset(&buf); + return NULL; + } + + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</filterbindingstatus>\n"); + + if (virBufferCheckError(&buf) < 0) + return NULL; + + return virBufferContentAndReset(&buf); +} diff --git a/src/conf/virnwfilterbindingobj.h b/src/conf/virnwfilterbindingobj.h new file mode 100644 index 0000000000..e0f1806ca0 --- /dev/null +++ b/src/conf/virnwfilterbindingobj.h @@ -0,0 +1,60 @@ +/* + * virnwfilterbindingobj.h: network filter binding XML processing + * + * Copyright (C) 2018 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + */ +#ifndef VIR_NWFILTER_BINDING_OBJ_H +# define VIR_NWFILTER_BINDING_OBJ_H + +# include "internal.h" +# include "virnwfilterbindingdef.h" +# include "virobject.h" + +typedef struct _virNWFilterBindingObj virNWFilterBindingObj; +typedef virNWFilterBindingObj *virNWFilterBindingObjPtr; + +struct _virNWFilterBindingObj { + virObjectLockable parent; + + bool removing; + virNWFilterBindingDefPtr def; +}; + +virNWFilterBindingObjPtr +virNWFilterBindingObjNew(void); + +void virNWFilterBindingObjEndAPI(virNWFilterBindingObjPtr *obj); + +char *virNWFilterBindingObjConfigFile(const char *dir, + const char *name); + +int +virNWFilterBindingObjSave(const virNWFilterBindingObj *obj, + const char *statusDir); + +int +virNWFilterBindingObjDelete(const virNWFilterBindingObj *obj, + const char *statusDir); + +virNWFilterBindingObjPtr +virNWFilterBindingObjParseFile(const char *filename); + +char * +virNWFilterBindingObjFormat(const virNWFilterBindingObj *obj); + +#endif /* VIR_NWFILTER_BINDING_OBJ_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0ce685b6f2..92ad2e983f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1051,6 +1051,16 @@ virNWFilterBindingDefParseNode; virNWFilterBindingDefParseString; +# conf/virnwfilterbindingobj.h +virNWFilterBindingObjConfigFile; +virNWFilterBindingObjDelete; +virNWFilterBindingObjEndAPI; +virNWFilterBindingObjFormat; +virNWFilterBindingObjNew; +virNWFilterBindingObjParseFile; +virNWFilterBindingObjSave; + + # conf/virnwfilterobj.h virNWFilterObjGetDef; virNWFilterObjGetNewDef; -- 2.17.0

Introduce a new struct to act as the manager of a collection of virNWFilterBindingObjPtr objects. --- src/conf/Makefile.inc.am | 2 + src/conf/virnwfilterbindingobjlist.c | 475 +++++++++++++++++++++++++++ src/conf/virnwfilterbindingobjlist.h | 66 ++++ src/libvirt_private.syms | 11 + 4 files changed, 554 insertions(+) create mode 100644 src/conf/virnwfilterbindingobjlist.c create mode 100644 src/conf/virnwfilterbindingobjlist.h diff --git a/src/conf/Makefile.inc.am b/src/conf/Makefile.inc.am index 3d55ba688d..af23810640 100644 --- a/src/conf/Makefile.inc.am +++ b/src/conf/Makefile.inc.am @@ -89,6 +89,8 @@ NWFILTER_CONF_SOURCES = \ conf/virnwfilterbindingdef.h \ conf/virnwfilterbindingobj.c \ conf/virnwfilterbindingobj.h \ + conf/virnwfilterbindingobjlist.c \ + conf/virnwfilterbindingobjlist.h \ $(NULL) STORAGE_CONF_SOURCES = \ diff --git a/src/conf/virnwfilterbindingobjlist.c b/src/conf/virnwfilterbindingobjlist.c new file mode 100644 index 0000000000..bf0869a216 --- /dev/null +++ b/src/conf/virnwfilterbindingobjlist.c @@ -0,0 +1,475 @@ +/* + * virnwfilterbindingobjlist.c: binding objects list utilities + * + * Copyright (C) 2006-2018 Red Hat, Inc. + * Copyright (C) 2006-2008 Daniel P. Berrange + * Copyright (c) 2015 SUSE LINUX Products GmbH, Nuernberg, Germany. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Daniel P. Berrange <berrange(a)redhat.com&gt; + */ + +#include <config.h> + +#include "internal.h" +#include "datatypes.h" +#include "virnwfilterbindingobjlist.h" +#include "viralloc.h" +#include "virfile.h" +#include "virlog.h" +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_NWFILTER + +VIR_LOG_INIT("conf.virnwfilterbindingobjlist"); + +static virClassPtr virNWFilterBindingObjListClass; +static void virNWFilterBindingObjListDispose(void *obj); + +struct _virNWFilterBindingObjList { + virObjectRWLockable parent; + + /* port dev name -> virNWFilterBindingObj mapping + * for O(1), lockless lookup-by-port dev */ + virHashTable *objs; +}; + + +static int virNWFilterBindingObjListOnceInit(void) +{ + if (!VIR_CLASS_NEW(virNWFilterBindingObjList, virClassForObjectRWLockable())) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNWFilterBindingObjList) + +virNWFilterBindingObjListPtr virNWFilterBindingObjListNew(void) +{ + virNWFilterBindingObjListPtr bindings; + + if (virNWFilterBindingObjListInitialize() < 0) + return NULL; + + if (!(bindings = virObjectRWLockableNew(virNWFilterBindingObjListClass))) + return NULL; + + if (!(bindings->objs = virHashCreate(50, virObjectFreeHashData))) { + virObjectUnref(bindings); + return NULL; + } + + return bindings; +} + +static void virNWFilterBindingObjListDispose(void *obj) +{ + virNWFilterBindingObjListPtr bindings = obj; + + virHashFree(bindings->objs); +} + + +static virNWFilterBindingObjPtr +virNWFilterBindingObjListFindByPortDevLocked(virNWFilterBindingObjListPtr bindings, + const char *name) +{ + virNWFilterBindingObjPtr obj; + + obj = virHashLookup(bindings->objs, name); + virObjectRef(obj); + if (obj) { + virObjectLock(obj); + if (obj->removing) { + virObjectUnlock(obj); + virObjectUnref(obj); + obj = NULL; + } + } + return obj; +} + + +/** + * @bindings: NWFilterBinding object list + * @name: Name to search the bindings->objs table + * + * Lookup the @name in the bindings->objs hash table and return a + * locked and ref counted binding object if found. Caller is expected + * to use the virNWFilterBindingObjEndAPI when done with the object. + */ +virNWFilterBindingObjPtr +virNWFilterBindingObjListFindByPortDev(virNWFilterBindingObjListPtr bindings, + const char *name) +{ + virNWFilterBindingObjPtr obj; + + virObjectRWLockRead(bindings); + obj = virNWFilterBindingObjListFindByPortDevLocked(bindings, name); + virObjectRWUnlock(bindings); + + return obj; +} + + +/** + * @bindings: NWFilterBinding object list pointer + * @binding: NWFilterBinding object to be added + * + * Upon entry @binding should have at least 1 ref and be locked. + * + * Add the @binding into the @bindings->objs hash + * tables. Once successfully added into a table, increase the + * reference count since upon removal in virHashRemoveEntry + * the virObjectUnref will be called since the hash tables were + * configured to call virObjectFreeHashData when the object is + * removed from the hash table. + * + * Returns 0 on success with 3 references and locked + * -1 on failure with 1 reference and locked + */ +static int +virNWFilterBindingObjListAddObjLocked(virNWFilterBindingObjListPtr bindings, + virNWFilterBindingObjPtr binding) +{ + if (virHashAddEntry(bindings->objs, binding->def->portdevname, binding) < 0) + return -1; + virObjectRef(binding); + + return 0; +} + + +/* + * virNWFilterBindingObjListAddLocked: + * + * The returned @binding from this function will be locked and ref + * counted. The caller is expected to use virNWFilterBindingObjEndAPI + * when it completes usage. + */ +static virNWFilterBindingObjPtr +virNWFilterBindingObjListAddLocked(virNWFilterBindingObjListPtr bindings, + virNWFilterBindingDefPtr def) +{ + virNWFilterBindingObjPtr binding; + + /* See if a BINDING with matching portdev already exists */ + if ((binding = virNWFilterBindingObjListFindByPortDevLocked(bindings, def->portdevname))) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("binding '%s' already exists"), + def->portdevname); + return NULL; + } + + if (!(binding = virNWFilterBindingObjNew())) + goto cleanup; + binding->def = def; + + if (virNWFilterBindingObjListAddObjLocked(bindings, binding) < 0) + goto error; + + cleanup: + return binding; + + error: + virNWFilterBindingObjEndAPI(&binding); + return NULL; +} + + +virNWFilterBindingObjPtr virNWFilterBindingObjListAdd(virNWFilterBindingObjListPtr bindings, + virNWFilterBindingDefPtr def) +{ + virNWFilterBindingObjPtr ret; + + virObjectRWLockWrite(bindings); + ret = virNWFilterBindingObjListAddLocked(bindings, def); + virObjectRWUnlock(bindings); + return ret; +} + + +/* The caller must hold lock on 'bindings' in addition to 'virNWFilterBindingObjListRemove' + * requirements + * + * Can be used to remove current element while iterating with + * virNWFilterBindingObjListForEach + */ +void +virNWFilterBindingObjListRemoveLocked(virNWFilterBindingObjListPtr bindings, + virNWFilterBindingObjPtr binding) +{ + virHashRemoveEntry(bindings->objs, binding->def->portdevname); +} + + +/** + * @bindings: Pointer to the binding object list + * @binding: NWFilterBinding pointer from either after Add or FindBy* API where the + * @binding was successfully added to both the bindings->objs + * hash tables that now would need to be removed. + * + * The caller must hold a lock on the driver owning 'bindings', + * and must also have locked and ref counted 'binding', to ensure + * no one else is either waiting for 'binding' or still using it. + * + * When this function returns, @binding will be removed from the hash + * tables and returned with lock and refcnt that was present upon entry. + */ +void +virNWFilterBindingObjListRemove(virNWFilterBindingObjListPtr bindings, + virNWFilterBindingObjPtr binding) +{ + binding->removing = true; + virObjectRef(binding); + virObjectUnlock(binding); + virObjectRWLockWrite(bindings); + virObjectLock(binding); + virNWFilterBindingObjListRemoveLocked(bindings, binding); + virObjectUnref(binding); + virObjectRWUnlock(bindings); +} + + +static virNWFilterBindingObjPtr +virNWFilterBindingObjListLoadStatus(virNWFilterBindingObjListPtr bindings, + const char *statusDir, + const char *name) +{ + char *statusFile = NULL; + virNWFilterBindingObjPtr obj = NULL; + + if ((statusFile = virNWFilterBindingObjConfigFile(statusDir, name)) == NULL) + goto error; + + if (!(obj = virNWFilterBindingObjParseFile(statusFile))) + goto error; + + if (virHashLookup(bindings->objs, obj->def->portdevname) != NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected binding %s already exists"), + obj->def->portdevname); + goto error; + } + + if (virNWFilterBindingObjListAddObjLocked(bindings, obj) < 0) + goto error; + + VIR_FREE(statusFile); + return obj; + + error: + virNWFilterBindingObjEndAPI(&obj); + VIR_FREE(statusFile); + return NULL; +} + + +int +virNWFilterBindingObjListLoadAllConfigs(virNWFilterBindingObjListPtr bindings, + const char *configDir) +{ + DIR *dir; + struct dirent *entry; + int ret = -1; + int rc; + + VIR_INFO("Scanning for configs in %s", configDir); + + if ((rc = virDirOpenIfExists(&dir, configDir)) <= 0) + return rc; + + virObjectRWLockWrite(bindings); + + while ((ret = virDirRead(dir, &entry, configDir)) > 0) { + virNWFilterBindingObjPtr binding; + + if (!virFileStripSuffix(entry->d_name, ".xml")) + continue; + + /* NB: ignoring errors, so one malformed config doesn't + kill the whole process */ + VIR_INFO("Loading config file '%s.xml'", entry->d_name); + binding = virNWFilterBindingObjListLoadStatus(bindings, + configDir, + entry->d_name); + if (binding) { + virNWFilterBindingObjEndAPI(&binding); + } else { + VIR_ERROR(_("Failed to load config for binding '%s'"), entry->d_name); + } + } + + VIR_DIR_CLOSE(dir); + virObjectRWUnlock(bindings); + return ret; +} + + +struct virNWFilterBindingListIterData { + virNWFilterBindingObjListIterator callback; + void *opaque; + int ret; +}; + + +static int +virNWFilterBindingObjListHelper(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) +{ + struct virNWFilterBindingListIterData *data = opaque; + + if (data->callback(payload, data->opaque) < 0) + data->ret = -1; + return 0; +} + + +int +virNWFilterBindingObjListForEach(virNWFilterBindingObjListPtr bindings, + virNWFilterBindingObjListIterator callback, + void *opaque) +{ + struct virNWFilterBindingListIterData data = { + callback, opaque, 0, + }; + virObjectRWLockRead(bindings); + virHashForEach(bindings->objs, virNWFilterBindingObjListHelper, &data); + virObjectRWUnlock(bindings); + return data.ret; +} + + +struct virNWFilterBindingListData { + virNWFilterBindingObjPtr *bindings; + size_t nbindings; +}; + + +static int +virNWFilterBindingObjListCollectIterator(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) +{ + struct virNWFilterBindingListData *data = opaque; + + data->bindings[data->nbindings++] = virObjectRef(payload); + return 0; +} + + +static void +virNWFilterBindingObjListFilter(virNWFilterBindingObjPtr **list, + size_t *nbindings, + virConnectPtr conn, + virNWFilterBindingObjListACLFilter filter) +{ + size_t i = 0; + + while (i < *nbindings) { + virNWFilterBindingObjPtr binding = (*list)[i]; + + virObjectLock(binding); + + /* do not list the object if: + * 1) it's being removed. + * 2) connection does not have ACL to see it + * 3) it doesn't match the filter + */ + if (binding->removing || + (filter && !filter(conn, binding->def))) { + virObjectUnlock(binding); + virObjectUnref(binding); + VIR_DELETE_ELEMENT(*list, i, *nbindings); + continue; + } + + virObjectUnlock(binding); + i++; + } +} + + +static int +virNWFilterBindingObjListCollect(virNWFilterBindingObjListPtr domlist, + virConnectPtr conn, + virNWFilterBindingObjPtr **bindings, + size_t *nbindings, + virNWFilterBindingObjListACLFilter filter) +{ + struct virNWFilterBindingListData data = { NULL, 0 }; + + virObjectRWLockRead(domlist); + sa_assert(domlist->objs); + if (VIR_ALLOC_N(data.bindings, virHashSize(domlist->objs)) < 0) { + virObjectRWUnlock(domlist); + return -1; + } + + virHashForEach(domlist->objs, virNWFilterBindingObjListCollectIterator, &data); + virObjectRWUnlock(domlist); + + virNWFilterBindingObjListFilter(&data.bindings, &data.nbindings, conn, filter); + + *nbindings = data.nbindings; + *bindings = data.bindings; + + return 0; +} + + +int +virNWFilterBindingObjListExport(virNWFilterBindingObjListPtr bindings, + virConnectPtr conn, + virNWFilterBindingPtr **bindinglist, + virNWFilterBindingObjListACLFilter filter) +{ + virNWFilterBindingObjPtr *bindingobjs = NULL; + size_t nbindings = 0; + size_t i; + int ret = -1; + + if (virNWFilterBindingObjListCollect(bindings, conn, &bindingobjs, &nbindings, filter) < 0) + return -1; + + if (bindinglist) { + if (VIR_ALLOC_N(*bindinglist, nbindings + 1) < 0) + goto cleanup; + + for (i = 0; i < nbindings; i++) { + virNWFilterBindingObjPtr binding = bindingobjs[i]; + + virObjectLock(binding); + (*bindinglist)[i] = virGetNWFilterBinding(conn, binding->def->portdevname, binding->def->filter); + virObjectUnlock(binding); + + if (!(*bindinglist)[i]) + goto cleanup; + } + } + + ret = nbindings; + + cleanup: + virObjectListFreeCount(bindingobjs, nbindings); + if (ret < 0) { + virObjectListFreeCount(*bindinglist, nbindings); + *bindinglist = NULL; + } + return ret; +} diff --git a/src/conf/virnwfilterbindingobjlist.h b/src/conf/virnwfilterbindingobjlist.h new file mode 100644 index 0000000000..252da90baa --- /dev/null +++ b/src/conf/virnwfilterbindingobjlist.h @@ -0,0 +1,66 @@ +/* + * virnwfilterbindingobjlist.h: domain objects list utilities + * + * Copyright (C) 2006-2018 Red Hat, Inc. + * Copyright (C) 2006-2008 Daniel P. Berrange + * Copyright (c) 2015 SUSE LINUX Products GmbH, Nuernberg, Germany. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Daniel P. Berrange <berrange(a)redhat.com&gt; + */ + +#ifndef __VIRNWFILTERBINDINGOBJ_H__ +# define __VIRNWFILTERBINDINGOBJT_H__ + +# include "virnwfilterbindingobj.h" + +typedef struct _virNWFilterBindingObjList virNWFilterBindingObjList; +typedef virNWFilterBindingObjList *virNWFilterBindingObjListPtr; + +virNWFilterBindingObjListPtr virNWFilterBindingObjListNew(void); + +virNWFilterBindingObjPtr virNWFilterBindingObjListFindByPortDev(virNWFilterBindingObjListPtr bindings, + const char *name); + +virNWFilterBindingObjPtr virNWFilterBindingObjListAdd(virNWFilterBindingObjListPtr bindings, + virNWFilterBindingDefPtr def); + +void virNWFilterBindingObjListRemove(virNWFilterBindingObjListPtr bindings, + virNWFilterBindingObjPtr binding); +void virNWFilterBindingObjListRemoveLocked(virNWFilterBindingObjListPtr bindings, + virNWFilterBindingObjPtr binding); + +int virNWFilterBindingObjListLoadAllConfigs(virNWFilterBindingObjListPtr bindings, + const char *configDir); + + +typedef int (*virNWFilterBindingObjListIterator)(virNWFilterBindingObjPtr binding, + void *opaque); + +int virNWFilterBindingObjListForEach(virNWFilterBindingObjListPtr bindings, + virNWFilterBindingObjListIterator callback, + void *opaque); + +typedef bool (*virNWFilterBindingObjListACLFilter)(virConnectPtr conn, + virNWFilterBindingDefPtr def); + +int virNWFilterBindingObjListExport(virNWFilterBindingObjListPtr bindings, + virConnectPtr conn, + virNWFilterBindingPtr **bindinglist, + virNWFilterBindingObjListACLFilter filter); + + +#endif /* __VIRNWFILTERBINDINGOBJLIST_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 92ad2e983f..b6754140bd 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1061,6 +1061,17 @@ virNWFilterBindingObjParseFile; virNWFilterBindingObjSave; +# conf/virnwfilterbindingobjlist.h +virNWFilterBindingObjListAdd; +virNWFilterBindingObjListExport; +virNWFilterBindingObjListFindByPortDev; +virNWFilterBindingObjListForEach; +virNWFilterBindingObjListLoadAllConfigs; +virNWFilterBindingObjListNew; +virNWFilterBindingObjListRemove; +virNWFilterBindingObjListRemoveLocked; + + # conf/virnwfilterobj.h virNWFilterObjGetDef; virNWFilterObjGetNewDef; -- 2.17.0

Currently the nwfilter driver does not keep any record of what filter bindings it has active. This means that when it needs to recreate filters, it has to rely on triggering callbacks provided by the virt drivers. This introduces a hash table recording the virNWFilterBinding objects so the driver has a record of all active filters. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/conf/virnwfilterobj.h | 4 ++ src/nwfilter/nwfilter_driver.c | 83 ++++++++++++++++++++++++---------- 2 files changed, 64 insertions(+), 23 deletions(-) diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h index 433b0402d0..4a54dd50da 100644 --- a/src/conf/virnwfilterobj.h +++ b/src/conf/virnwfilterobj.h @@ -22,6 +22,7 @@ # include "internal.h" # include "nwfilter_conf.h" +# include "virnwfilterbindingobjlist.h" typedef struct _virNWFilterObj virNWFilterObj; typedef virNWFilterObj *virNWFilterObjPtr; @@ -37,7 +38,10 @@ struct _virNWFilterDriverState { virNWFilterObjListPtr nwfilters; + virNWFilterBindingObjListPtr bindings; + char *configDir; + char *bindingDir; }; virNWFilterDefPtr diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index b57e5dd00d..67e07d2dec 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -38,7 +38,6 @@ #include "domain_conf.h" #include "domain_nwfilter.h" #include "nwfilter_driver.h" -#include "virnwfilterbindingdef.h" #include "nwfilter_gentech_driver.h" #include "configmake.h" #include "virfile.h" @@ -174,7 +173,6 @@ nwfilterStateInitialize(bool privileged, virStateInhibitCallback callback ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { - char *base = NULL; DBusConnection *sysbus = NULL; if (virDBusHasSystemBus() && @@ -191,6 +189,9 @@ nwfilterStateInitialize(bool privileged, if (!(driver->nwfilters = virNWFilterObjListNew())) goto error; + if (!(driver->bindings = virNWFilterBindingObjListNew())) + goto error; + if (!privileged) return 0; @@ -230,30 +231,35 @@ nwfilterStateInitialize(bool privileged, goto error; } - if (VIR_STRDUP(base, SYSCONFDIR "/libvirt") < 0) + if (VIR_STRDUP(driver->configDir, SYSCONFDIR "/libvirt/nwfilter") < 0) goto error; - if (virAsprintf(&driver->configDir, - "%s/nwfilter", base) == -1) + if (virFileMakePathWithMode(driver->configDir, S_IRWXU) < 0) { + virReportSystemError(errno, _("cannot create config directory '%s'"), + driver->configDir); goto error; + } - VIR_FREE(base); + if (VIR_STRDUP(driver->bindingDir, LOCALSTATEDIR "/run/libvirt/nwfilter-binding") < 0) + goto error; - if (virFileMakePathWithMode(driver->configDir, S_IRWXU) < 0) { + if (virFileMakePathWithMode(driver->bindingDir, S_IRWXU) < 0) { virReportSystemError(errno, _("cannot create config directory '%s'"), - driver->configDir); + driver->bindingDir); goto error; } if (virNWFilterObjListLoadAllConfigs(driver->nwfilters, driver->configDir) < 0) goto error; + if (virNWFilterBindingObjListLoadAllConfigs(driver->bindings, driver->bindingDir) < 0) + goto error; + nwfilterDriverUnlock(); return 0; error: - VIR_FREE(base); nwfilterDriverUnlock(); nwfilterStateCleanup(); @@ -333,9 +339,12 @@ nwfilterStateCleanup(void) nwfilterDriverRemoveDBusMatches(); VIR_FREE(driver->configDir); + VIR_FREE(driver->bindingDir); nwfilterDriverUnlock(); } + virObjectUnref(driver->bindings); + /* free inactive nwfilters */ virNWFilterObjListFree(driver->nwfilters); @@ -647,13 +656,38 @@ nwfilterInstantiateFilter(const char *vmname, const unsigned char *vmuuid, virDomainNetDefPtr net) { - virNWFilterBindingDefPtr binding; + virNWFilterBindingObjPtr obj; + virNWFilterBindingDefPtr def; int ret; - if (!(binding = virNWFilterBindingDefForNet(vmname, vmuuid, net))) + obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, net->ifname); + if (obj) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Filter already present for NIC %s"), net->ifname); + virNWFilterBindingObjEndAPI(&obj); + return -1; + } + + if (!(def = virNWFilterBindingDefForNet(vmname, vmuuid, net))) + return -1; + + obj = virNWFilterBindingObjListAdd(driver->bindings, + def); + if (!obj) { + virNWFilterBindingDefFree(def); return -1; - ret = virNWFilterInstantiateFilter(driver, binding); - virNWFilterBindingDefFree(binding); + } + def = NULL; + + ret = virNWFilterInstantiateFilter(driver, obj->def); + + if (ret < 0) + virNWFilterBindingObjListRemove(driver->bindings, obj); + + virNWFilterBindingObjSave(obj, driver->bindingDir); + + virNWFilterBindingObjEndAPI(&obj); + return ret; } @@ -661,16 +695,19 @@ nwfilterInstantiateFilter(const char *vmname, static void nwfilterTeardownFilter(virDomainNetDefPtr net) { - virNWFilterBindingDef binding = { - .portdevname = net->ifname, - .linkdevname = (net->type == VIR_DOMAIN_NET_TYPE_DIRECT ? - net->data.direct.linkdev : NULL), - .mac = net->mac, - .filter = net->filter, - .filterparams = net->filterparams, - }; - if ((net->ifname) && (net->filter)) - virNWFilterTeardownFilter(&binding); + virNWFilterBindingObjPtr obj; + if (!net->ifname) + return; + + obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, net->ifname); + if (!obj) + return; + + virNWFilterTeardownFilter(obj->def); + virNWFilterBindingObjDelete(obj, driver->bindingDir); + + virNWFilterBindingObjListRemove(driver->bindings, obj); + virNWFilterBindingObjEndAPI(&obj); } -- 2.17.0

Now that the nwfilter driver keeps a list of bindings that it has created, there is no need for the complex virt driver callbacks. It is possible to simply iterate of the list of recorded filter bindings. This means that rebuilding filters no longer has to acquire any locks on the virDomainObj objects, as they're never touched. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/conf/nwfilter_conf.c | 169 +++++++------------------ src/conf/nwfilter_conf.h | 51 +------- src/conf/virnwfilterobj.c | 4 +- src/libvirt_private.syms | 7 +- src/lxc/lxc_driver.c | 28 ---- src/nwfilter/nwfilter_driver.c | 21 +-- src/nwfilter/nwfilter_gentech_driver.c | 164 +++++++++++++++--------- src/nwfilter/nwfilter_gentech_driver.h | 4 +- src/qemu/qemu_driver.c | 25 ---- src/uml/uml_driver.c | 29 ----- 10 files changed, 173 insertions(+), 329 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index de26a6d034..29aacba98d 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2819,121 +2819,6 @@ virNWFilterSaveConfig(const char *configDir, } -int nCallbackDriver; -#define MAX_CALLBACK_DRIVER 10 -static virNWFilterCallbackDriverPtr callbackDrvArray[MAX_CALLBACK_DRIVER]; - -void -virNWFilterRegisterCallbackDriver(virNWFilterCallbackDriverPtr cbd) -{ - if (nCallbackDriver < MAX_CALLBACK_DRIVER) - callbackDrvArray[nCallbackDriver++] = cbd; -} - - -void -virNWFilterUnRegisterCallbackDriver(virNWFilterCallbackDriverPtr cbd) -{ - size_t i = 0; - - while (i < nCallbackDriver && callbackDrvArray[i] != cbd) - i++; - - if (i < nCallbackDriver) { - memmove(&callbackDrvArray[i], &callbackDrvArray[i+1], - (nCallbackDriver - i - 1) * sizeof(callbackDrvArray[i])); - callbackDrvArray[i] = 0; - nCallbackDriver--; - } -} - - -void -virNWFilterCallbackDriversLock(void) -{ - size_t i; - - for (i = 0; i < nCallbackDriver; i++) - callbackDrvArray[i]->vmDriverLock(); -} - - -void -virNWFilterCallbackDriversUnlock(void) -{ - size_t i; - - for (i = 0; i < nCallbackDriver; i++) - callbackDrvArray[i]->vmDriverUnlock(); -} - - -static virDomainObjListIterator virNWFilterDomainFWUpdateCB; -static void *virNWFilterDomainFWUpdateOpaque; - -/** - * virNWFilterInstFiltersOnAllVMs: - * Apply all filters on all running VMs. Don't terminate in case of an - * error. This should be called upon reloading of the driver. - */ -int -virNWFilterInstFiltersOnAllVMs(void) -{ - size_t i; - struct domUpdateCBStruct cb = { - .opaque = virNWFilterDomainFWUpdateOpaque, - .step = STEP_APPLY_CURRENT, - .skipInterfaces = NULL, /* not needed */ - }; - - for (i = 0; i < nCallbackDriver; i++) - callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB, - &cb); - - return 0; -} - - -int -virNWFilterTriggerVMFilterRebuild(void) -{ - size_t i; - int ret = 0; - struct domUpdateCBStruct cb = { - .opaque = virNWFilterDomainFWUpdateOpaque, - .step = STEP_APPLY_NEW, - .skipInterfaces = virHashCreate(0, NULL), - }; - - if (!cb.skipInterfaces) - return -1; - - for (i = 0; i < nCallbackDriver; i++) { - if (callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB, - &cb) < 0) - ret = -1; - } - - if (ret < 0) { - cb.step = STEP_TEAR_NEW; /* rollback */ - - for (i = 0; i < nCallbackDriver; i++) - callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB, - &cb); - } else { - cb.step = STEP_TEAR_OLD; /* switch over */ - - for (i = 0; i < nCallbackDriver; i++) - callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB, - &cb); - } - - virHashFree(cb.skipInterfaces); - - return ret; -} - - int virNWFilterDeleteDef(const char *configDir, virNWFilterDefPtr def) @@ -3204,16 +3089,18 @@ virNWFilterDefFormat(const virNWFilterDef *def) return NULL; } +static virNWFilterTriggerRebuildCallback rebuildCallback; +static void *rebuildOpaque; int -virNWFilterConfLayerInit(virDomainObjListIterator domUpdateCB, +virNWFilterConfLayerInit(virNWFilterTriggerRebuildCallback cb, void *opaque) { if (initialized) return -1; - virNWFilterDomainFWUpdateCB = domUpdateCB; - virNWFilterDomainFWUpdateOpaque = opaque; + rebuildCallback = cb; + rebuildOpaque = opaque; initialized = true; @@ -3233,8 +3120,50 @@ virNWFilterConfLayerShutdown(void) virRWLockDestroy(&updateLock); initialized = false; - virNWFilterDomainFWUpdateOpaque = NULL; - virNWFilterDomainFWUpdateCB = NULL; + rebuildCallback = NULL; + rebuildOpaque = NULL; +} + +int +virNWFilterTriggerRebuild(void) +{ +#if 0 + size_t i; + int ret = 0; + struct domUpdateCBStruct cb = { + .opaque = virNWFilterDomainFWUpdateOpaque, + .step = STEP_APPLY_NEW, + .skipInterfaces = virHashCreate(0, NULL), + }; + + if (!cb.skipInterfaces) + return -1; + + for (i = 0; i < nCallbackDriver; i++) { + if (callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB, + &cb) < 0) + ret = -1; + } + + if (ret < 0) { + cb.step = STEP_TEAR_NEW; /* rollback */ + + for (i = 0; i < nCallbackDriver; i++) + callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB, + &cb); + } else { + cb.step = STEP_TEAR_OLD; /* switch over */ + + for (i = 0; i < nCallbackDriver; i++) + callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB, + &cb); + } + + virHashFree(cb.skipInterfaces); + + return ret; +#endif + return rebuildCallback(rebuildOpaque); } diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h index 08fc07c55c..9f8ad51bf2 100644 --- a/src/conf/nwfilter_conf.h +++ b/src/conf/nwfilter_conf.h @@ -546,20 +546,6 @@ struct _virNWFilterDef { }; -typedef enum { - STEP_APPLY_NEW, - STEP_TEAR_NEW, - STEP_TEAR_OLD, - STEP_APPLY_CURRENT, -} UpdateStep; - -struct domUpdateCBStruct { - void *opaque; - UpdateStep step; - virHashTablePtr skipInterfaces; -}; - - void virNWFilterRuleDefFree(virNWFilterRuleDefPtr def); @@ -567,7 +553,7 @@ void virNWFilterDefFree(virNWFilterDefPtr def); int -virNWFilterTriggerVMFilterRebuild(void); +virNWFilterTriggerRebuild(void); int virNWFilterDeleteDef(const char *configDir, @@ -599,44 +585,15 @@ virNWFilterReadLockFilterUpdates(void); void virNWFilterUnlockFilterUpdates(void); +typedef int (*virNWFilterTriggerRebuildCallback)(void *opaque); + int -virNWFilterConfLayerInit(virDomainObjListIterator domUpdateCB, +virNWFilterConfLayerInit(virNWFilterTriggerRebuildCallback cb, void *opaque); void virNWFilterConfLayerShutdown(void); -int -virNWFilterInstFiltersOnAllVMs(void); - -typedef int -(*virNWFilterRebuild)(virDomainObjListIterator domUpdateCB, - void *data); - -typedef void -(*virNWFilterVoidCall)(void); - -typedef struct _virNWFilterCallbackDriver virNWFilterCallbackDriver; -typedef virNWFilterCallbackDriver *virNWFilterCallbackDriverPtr; -struct _virNWFilterCallbackDriver { - const char *name; - - virNWFilterRebuild vmFilterRebuild; - virNWFilterVoidCall vmDriverLock; - virNWFilterVoidCall vmDriverUnlock; -}; - -void -virNWFilterRegisterCallbackDriver(virNWFilterCallbackDriverPtr); - -void -virNWFilterUnRegisterCallbackDriver(virNWFilterCallbackDriverPtr); - -void -virNWFilterCallbackDriversLock(void); - -void -virNWFilterCallbackDriversUnlock(void); char * virNWFilterPrintTCPFlags(uint8_t flags); diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 87d7e72703..0136a0d56c 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -276,7 +276,7 @@ virNWFilterObjTestUnassignDef(virNWFilterObjPtr obj) obj->wantRemoved = true; /* trigger the update on VMs referencing the filter */ - if (virNWFilterTriggerVMFilterRebuild() < 0) + if (virNWFilterTriggerRebuild() < 0) rc = -1; obj->wantRemoved = false; @@ -358,7 +358,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters, obj->newDef = def; /* trigger the update on VMs referencing the filter */ - if (virNWFilterTriggerVMFilterRebuild() < 0) { + if (virNWFilterTriggerRebuild() < 0) { obj->newDef = NULL; virNWFilterObjUnlock(obj); return NULL; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b6754140bd..a34f899379 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -781,8 +781,6 @@ virDomainNumatuneSpecifiedMaxNode; # conf/nwfilter_conf.h -virNWFilterCallbackDriversLock; -virNWFilterCallbackDriversUnlock; virNWFilterChainSuffixTypeToString; virNWFilterConfLayerInit; virNWFilterConfLayerShutdown; @@ -791,12 +789,10 @@ virNWFilterDefFree; virNWFilterDefParseFile; virNWFilterDefParseString; virNWFilterDeleteDef; -virNWFilterInstFiltersOnAllVMs; virNWFilterJumpTargetTypeToString; virNWFilterPrintStateMatchFlags; virNWFilterPrintTCPFlags; virNWFilterReadLockFilterUpdates; -virNWFilterRegisterCallbackDriver; virNWFilterRuleActionTypeToString; virNWFilterRuleDirectionTypeToString; virNWFilterRuleIsProtocolEthernet; @@ -804,9 +800,8 @@ virNWFilterRuleIsProtocolIPv4; virNWFilterRuleIsProtocolIPv6; virNWFilterRuleProtocolTypeToString; virNWFilterSaveConfig; -virNWFilterTriggerVMFilterRebuild; +virNWFilterTriggerRebuild; virNWFilterUnlockFilterUpdates; -virNWFilterUnRegisterCallbackDriver; virNWFilterWriteLockFilterUpdates; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index a2e29a2121..8cb8d907ff 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -66,7 +66,6 @@ #include "virfdstream.h" #include "domain_audit.h" #include "domain_nwfilter.h" -#include "nwfilter_conf.h" #include "virinitctl.h" #include "virnetdev.h" #include "virnetdevtap.h" @@ -95,31 +94,6 @@ static int lxcStateInitialize(bool privileged, static int lxcStateCleanup(void); virLXCDriverPtr lxc_driver = NULL; -/* callbacks for nwfilter */ -static int -lxcVMFilterRebuild(virDomainObjListIterator iter, void *data) -{ - return virDomainObjListForEach(lxc_driver->domains, iter, data); -} - -static void -lxcVMDriverLock(void) -{ - lxcDriverLock(lxc_driver); -} - -static void -lxcVMDriverUnlock(void) -{ - lxcDriverUnlock(lxc_driver); -} - -static virNWFilterCallbackDriver lxcCallbackDriver = { - .name = "LXC", - .vmFilterRebuild = lxcVMFilterRebuild, - .vmDriverLock = lxcVMDriverLock, - .vmDriverUnlock = lxcVMDriverUnlock, -}; /** * lxcDomObjFromDomain: @@ -1677,7 +1651,6 @@ static int lxcStateInitialize(bool privileged, NULL, NULL) < 0) goto cleanup; - virNWFilterRegisterCallbackDriver(&lxcCallbackDriver); virObjectUnref(caps); return 0; @@ -1750,7 +1723,6 @@ static int lxcStateCleanup(void) if (lxc_driver == NULL) return -1; - virNWFilterUnRegisterCallbackDriver(&lxcCallbackDriver); virObjectUnref(lxc_driver->domains); virObjectUnref(lxc_driver->domainEventState); diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 67e07d2dec..caccecadda 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -163,6 +163,14 @@ nwfilterDriverInstallDBusMatches(DBusConnection *sysbus ATTRIBUTE_UNUSED) #endif /* HAVE_FIREWALLD */ +static int virNWFilterTriggerRebuildImpl(void *opaque) +{ + virNWFilterDriverStatePtr nwdriver = opaque; + + return virNWFilterBuildAll(nwdriver, true); +} + + /** * nwfilterStateInitialize: * @@ -207,7 +215,7 @@ nwfilterStateInitialize(bool privileged, if (virNWFilterTechDriversInit(privileged) < 0) goto err_dhcpsnoop_shutdown; - if (virNWFilterConfLayerInit(virNWFilterDomainFWUpdateCB, + if (virNWFilterConfLayerInit(virNWFilterTriggerRebuildImpl, driver) < 0) goto err_techdrivers_shutdown; @@ -302,15 +310,14 @@ nwfilterStateReload(void) nwfilterDriverLock(); virNWFilterWriteLockFilterUpdates(); - virNWFilterCallbackDriversLock(); virNWFilterObjListLoadAllConfigs(driver->nwfilters, driver->configDir); - virNWFilterCallbackDriversUnlock(); virNWFilterUnlockFilterUpdates(); - nwfilterDriverUnlock(); - virNWFilterInstFiltersOnAllVMs(); + virNWFilterBuildAll(driver, false); + + nwfilterDriverUnlock(); return 0; } @@ -547,7 +554,6 @@ nwfilterDefineXML(virConnectPtr conn, nwfilterDriverLock(); virNWFilterWriteLockFilterUpdates(); - virNWFilterCallbackDriversLock(); if (!(def = virNWFilterDefParseString(xml))) goto cleanup; @@ -572,7 +578,6 @@ nwfilterDefineXML(virConnectPtr conn, if (obj) virNWFilterObjUnlock(obj); - virNWFilterCallbackDriversUnlock(); virNWFilterUnlockFilterUpdates(); nwfilterDriverUnlock(); return nwfilter; @@ -588,7 +593,6 @@ nwfilterUndefine(virNWFilterPtr nwfilter) nwfilterDriverLock(); virNWFilterWriteLockFilterUpdates(); - virNWFilterCallbackDriversLock(); if (!(obj = nwfilterObjFromNWFilter(nwfilter->uuid))) goto cleanup; @@ -615,7 +619,6 @@ nwfilterUndefine(virNWFilterPtr nwfilter) if (obj) virNWFilterObjUnlock(obj); - virNWFilterCallbackDriversUnlock(); virNWFilterUnlockFilterUpdates(); nwfilterDriverUnlock(); return ret; diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 30ae3970fb..de7361f3dd 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -153,9 +153,9 @@ virNWFilterVarHashmapAddStdValues(virHashTablePtr table, if (!val) return -1; - if (virHashAddEntry(table, - NWFILTER_STD_VAR_MAC, - val) < 0) { + if (virHashUpdateEntry(table, + NWFILTER_STD_VAR_MAC, + val) < 0) { virNWFilterVarValueFree(val); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add variable 'MAC' to hashmap")); @@ -168,9 +168,9 @@ virNWFilterVarHashmapAddStdValues(virHashTablePtr table, if (!val) return -1; - if (virHashAddEntry(table, - NWFILTER_STD_VAR_IP, - val) < 0) { + if (virHashUpdateEntry(table, + NWFILTER_STD_VAR_IP, + val) < 0) { virNWFilterVarValueFree(val); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add variable 'IP' to hashmap")); @@ -1000,68 +1000,110 @@ virNWFilterTeardownFilter(virNWFilterBindingDefPtr binding) return ret; } +enum { + STEP_APPLY_NEW, + STEP_TEAR_NEW, + STEP_TEAR_OLD, + STEP_APPLY_CURRENT, +}; -int -virNWFilterDomainFWUpdateCB(virDomainObjPtr obj, - void *data) +static int +virNWFilterBuildOne(virNWFilterDriverStatePtr driver, + virNWFilterBindingDefPtr binding, + virHashTablePtr skipInterfaces, + int step) { - virDomainDefPtr vm = obj->def; - struct domUpdateCBStruct *cb = data; - size_t i; bool skipIface; int ret = 0; - - virObjectLock(obj); - - if (virDomainObjIsActive(obj)) { - for (i = 0; i < vm->nnets; i++) { - virDomainNetDefPtr net = vm->nets[i]; - virNWFilterBindingDefPtr binding; - - if ((net->filter) && (net->ifname) && - (binding = virNWFilterBindingDefForNet( - vm->name, vm->uuid, net))) { - - switch (cb->step) { - case STEP_APPLY_NEW: - ret = virNWFilterUpdateInstantiateFilter(cb->opaque, - binding, - &skipIface); - if (ret == 0 && skipIface) { - /* filter tree unchanged -- no update needed */ - ret = virHashAddEntry(cb->skipInterfaces, - net->ifname, - (void *)~0); - } - break; - - case STEP_TEAR_NEW: - if (!virHashLookup(cb->skipInterfaces, net->ifname)) - ret = virNWFilterRollbackUpdateFilter(binding); - break; - - case STEP_TEAR_OLD: - if (!virHashLookup(cb->skipInterfaces, net->ifname)) - ret = virNWFilterTearOldFilter(binding); - break; - - case STEP_APPLY_CURRENT: - ret = virNWFilterInstantiateFilter(cb->opaque, - binding); - if (ret) - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failure while applying current filter on " - "VM %s"), vm->name); - break; - } - virNWFilterBindingDefFree(binding); - if (ret) - break; - } + VIR_DEBUG("Building filter for portdev=%s step=%d", binding->portdevname, step); + + switch (step) { + case STEP_APPLY_NEW: + ret = virNWFilterUpdateInstantiateFilter(driver, + binding, + &skipIface); + if (ret == 0 && skipIface) { + /* filter tree unchanged -- no update needed */ + ret = virHashAddEntry(skipInterfaces, + binding->portdevname, + (void *)~0); } + break; + + case STEP_TEAR_NEW: + if (!virHashLookup(skipInterfaces, binding->portdevname)) + ret = virNWFilterRollbackUpdateFilter(binding); + break; + + case STEP_TEAR_OLD: + if (!virHashLookup(skipInterfaces, binding->portdevname)) + ret = virNWFilterTearOldFilter(binding); + break; + + case STEP_APPLY_CURRENT: + ret = virNWFilterInstantiateFilter(driver, + binding); + break; } - virObjectUnlock(obj); + return ret; +} + + +struct virNWFilterBuildData { + virNWFilterDriverStatePtr driver; + virHashTablePtr skipInterfaces; + int step; +}; + +static int +virNWFilterBuildIter(virNWFilterBindingObjPtr binding, void *opaque) +{ + struct virNWFilterBuildData *data = opaque; + + return virNWFilterBuildOne(data->driver, binding->def, + data->skipInterfaces, data->step); +} + +int +virNWFilterBuildAll(virNWFilterDriverStatePtr driver, + bool newFilters) +{ + struct virNWFilterBuildData data = { + .driver = driver, + }; + int ret = 0; + + VIR_DEBUG("Build all filters newFilters=%d", newFilters); + + if (newFilters) { + if (!(data.skipInterfaces = virHashCreate(0, NULL))) + return -1; + + data.step = STEP_APPLY_NEW; + if (virNWFilterBindingObjListForEach(driver->bindings, + virNWFilterBuildIter, + &data) < 0) + ret = -1; + + if (ret == -1) { + data.step = STEP_TEAR_NEW; + virNWFilterBindingObjListForEach(driver->bindings, + virNWFilterBuildIter, + &data); + } else { + data.step = STEP_TEAR_OLD; + virNWFilterBindingObjListForEach(driver->bindings, + virNWFilterBuildIter, + &data); + } + } else { + data.step = STEP_APPLY_CURRENT; + if (virNWFilterBindingObjListForEach(driver->bindings, + virNWFilterBuildIter, + &data) < 0) + ret = -1; + } return ret; } diff --git a/src/nwfilter/nwfilter_gentech_driver.h b/src/nwfilter/nwfilter_gentech_driver.h index 6b51096a0d..481fdd2413 100644 --- a/src/nwfilter/nwfilter_gentech_driver.h +++ b/src/nwfilter/nwfilter_gentech_driver.h @@ -54,8 +54,8 @@ int virNWFilterTeardownFilter(virNWFilterBindingDefPtr binding); virHashTablePtr virNWFilterCreateVarHashmap(const char *macaddr, const virNWFilterVarValue *value); -int virNWFilterDomainFWUpdateCB(virDomainObjPtr vm, - void *data); +int virNWFilterBuildAll(virNWFilterDriverStatePtr driver, + bool newFilters); virNWFilterBindingDefPtr virNWFilterBindingDefForNet(const char *vmname, const unsigned char *vmuuid, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9037818e2a..edc2d214e4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -84,7 +84,6 @@ #include "cpu/cpu.h" #include "virsysinfo.h" #include "domain_nwfilter.h" -#include "nwfilter_conf.h" #include "virhook.h" #include "virstoragefile.h" #include "virfile.h" @@ -164,28 +163,6 @@ static int qemuARPGetInterfaces(virDomainObjPtr vm, static virQEMUDriverPtr qemu_driver; - -static void -qemuVMDriverLock(void) -{} -static void -qemuVMDriverUnlock(void) -{} - -static int -qemuVMFilterRebuild(virDomainObjListIterator iter, void *data) -{ - return virDomainObjListForEach(qemu_driver->domains, iter, data); -} - -static virNWFilterCallbackDriver qemuCallbackDriver = { - .name = QEMU_DRIVER_NAME, - .vmFilterRebuild = qemuVMFilterRebuild, - .vmDriverLock = qemuVMDriverLock, - .vmDriverUnlock = qemuVMDriverUnlock, -}; - - /** * qemuDomObjFromDomain: * @domain: Domain pointer that has to be looked up @@ -940,7 +917,6 @@ qemuStateInitialize(bool privileged, if (!qemu_driver->workerPool) goto error; - virNWFilterRegisterCallbackDriver(&qemuCallbackDriver); return 0; error: @@ -1080,7 +1056,6 @@ qemuStateCleanup(void) if (!qemu_driver) return -1; - virNWFilterUnRegisterCallbackDriver(&qemuCallbackDriver); virThreadPoolFree(qemu_driver->workerPool); virObjectUnref(qemu_driver->config); virObjectUnref(qemu_driver->hostdevMgr); diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 53ec64e10f..7419310a6d 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -55,7 +55,6 @@ #include "datatypes.h" #include "virlog.h" #include "domain_nwfilter.h" -#include "nwfilter_conf.h" #include "virfile.h" #include "virfdstream.h" #include "configmake.h" @@ -145,25 +144,6 @@ static int umlMonitorCommand(const struct uml_driver *driver, static struct uml_driver *uml_driver; -static int -umlVMFilterRebuild(virDomainObjListIterator iter, void *data) -{ - return virDomainObjListForEach(uml_driver->domains, iter, data); -} - -static void -umlVMDriverLock(void) -{ - umlDriverLock(uml_driver); -} - -static void -umlVMDriverUnlock(void) -{ - umlDriverUnlock(uml_driver); -} - - static virDomainObjPtr umlDomObjFromDomainLocked(struct uml_driver *driver, const unsigned char *uuid) @@ -196,13 +176,6 @@ umlDomObjFromDomain(struct uml_driver *driver, } -static virNWFilterCallbackDriver umlCallbackDriver = { - .name = "UML", - .vmFilterRebuild = umlVMFilterRebuild, - .vmDriverLock = umlVMDriverLock, - .vmDriverUnlock = umlVMDriverUnlock, -}; - struct umlAutostartData { struct uml_driver *driver; virConnectPtr conn; @@ -609,7 +582,6 @@ umlStateInitialize(bool privileged, VIR_FREE(userdir); - virNWFilterRegisterCallbackDriver(&umlCallbackDriver); return 0; out_of_memory: @@ -703,7 +675,6 @@ umlStateCleanup(void) return -1; umlDriverLock(uml_driver); - virNWFilterRegisterCallbackDriver(&umlCallbackDriver); if (uml_driver->inotifyWatch != -1) virEventRemoveHandle(uml_driver->inotifyWatch); VIR_FORCE_CLOSE(uml_driver->inotifyFD); -- 2.17.0

Wire up the ListAll, LookupByPortDev and GetXMLDesc APIs to allow the virsh nwfilter-binding-list & nwfilter-binding-dumpxml commands to work. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/nwfilter/nwfilter_driver.c | 71 ++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index caccecadda..6544261b38 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -714,6 +714,74 @@ nwfilterTeardownFilter(virDomainNetDefPtr net) } +static virNWFilterBindingPtr +nwfilterBindingLookupByPortDev(virConnectPtr conn, + const char *portdev) +{ + virNWFilterBindingPtr ret = NULL; + virNWFilterBindingObjPtr obj; + + obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, + portdev); + if (!obj) + goto cleanup; + + if (virNWFilterBindingLookupByPortDevEnsureACL(conn, obj->def) < 0) + goto cleanup; + + ret = virGetNWFilterBinding(conn, obj->def->portdevname, obj->def->filter); + + cleanup: + virNWFilterBindingObjEndAPI(&obj); + return ret; +} + + +static int +nwfilterConnectListAllNWFilterBindings(virConnectPtr conn, + virNWFilterBindingPtr **bindings, + unsigned int flags) +{ + int ret; + + virCheckFlags(0, -1); + + if (virConnectListAllNWFilterBindingsEnsureACL(conn) < 0) + return -1; + + ret = virNWFilterBindingObjListExport(driver->bindings, + conn, + bindings, + virConnectListAllNWFilterBindingsCheckACL); + + return ret; +} + +static char * +nwfilterBindingGetXMLDesc(virNWFilterBindingPtr binding, + unsigned int flags) +{ + virNWFilterBindingObjPtr obj; + char *ret = NULL; + + virCheckFlags(0, NULL); + + obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, + binding->portdev); + if (!obj) + goto cleanup; + + if (virNWFilterBindingGetXMLDescEnsureACL(binding->conn, obj->def) < 0) + goto cleanup; + + ret = virNWFilterBindingDefFormat(obj->def); + + cleanup: + virNWFilterBindingObjEndAPI(&obj); + return ret; +} + + static virNWFilterDriver nwfilterDriver = { .name = "nwfilter", .connectNumOfNWFilters = nwfilterConnectNumOfNWFilters, /* 0.8.0 */ @@ -724,6 +792,9 @@ static virNWFilterDriver nwfilterDriver = { .nwfilterDefineXML = nwfilterDefineXML, /* 0.8.0 */ .nwfilterUndefine = nwfilterUndefine, /* 0.8.0 */ .nwfilterGetXMLDesc = nwfilterGetXMLDesc, /* 0.8.0 */ + .nwfilterBindingLookupByPortDev = nwfilterBindingLookupByPortDev, /* 4.4.0 */ + .connectListAllNWFilterBindings = nwfilterConnectListAllNWFilterBindings, /* 4.4.0 */ + .nwfilterBindingGetXMLDesc = nwfilterBindingGetXMLDesc, /* 4.4.0 */ }; -- 2.17.0

This allows the virsh commands nwfilter-binding-create and nwfilter-binding-delete to be used. Note using these commands lets you delete filters that were previously created automatically by the virt drivers, or add filters for VM nics that were not there before. Generally it is expected these new APIs will only be used by virt drivers. It is the admin's responsibility to not shoot themselves in the foot. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/nwfilter/nwfilter_driver.c | 77 ++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 6544261b38..c3c52ae5f3 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -782,6 +782,81 @@ nwfilterBindingGetXMLDesc(virNWFilterBindingPtr binding, } +static virNWFilterBindingPtr +nwfilterBindingCreateXML(virConnectPtr conn, + const char *xml, + unsigned int flags) +{ + virNWFilterBindingObjPtr obj; + virNWFilterBindingDefPtr def; + virNWFilterBindingPtr ret = NULL; + + virCheckFlags(0, NULL); + + def = virNWFilterBindingDefParseString(xml); + if (!def) + return NULL; + + if (virNWFilterBindingCreateXMLEnsureACL(conn, def) < 0) + goto cleanup; + + obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, def->portdevname); + if (obj) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Filter already present for NIC %s"), def->portdevname); + goto cleanup; + } + + obj = virNWFilterBindingObjListAdd(driver->bindings, + def); + if (!obj) + goto cleanup; + def = NULL; + + if (!(ret = virGetNWFilterBinding(conn, obj->def->portdevname, obj->def->filter))) + goto cleanup; + + if (virNWFilterInstantiateFilter(driver, obj->def) < 0) { + virNWFilterBindingObjListRemove(driver->bindings, obj); + virObjectUnref(ret); + ret = NULL; + goto cleanup; + } + virNWFilterBindingObjSave(obj, driver->bindingDir); + + cleanup: + virNWFilterBindingDefFree(def); + virNWFilterBindingObjEndAPI(&obj); + + return ret; +} + + +static int +nwfilterBindingDelete(virNWFilterBindingPtr binding) +{ + virNWFilterBindingObjPtr obj; + int ret = -1; + + obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, binding->portdev); + if (!obj) + return -1; + + if (virNWFilterBindingDeleteEnsureACL(binding->conn, obj->def) < 0) + goto cleanup; + + virNWFilterTeardownFilter(obj->def); + virNWFilterBindingObjDelete(obj, driver->bindingDir); + virNWFilterBindingObjListRemove(driver->bindings, obj); + + ret = 0; + + cleanup: + virNWFilterBindingObjEndAPI(&obj); + return ret; +} + + static virNWFilterDriver nwfilterDriver = { .name = "nwfilter", .connectNumOfNWFilters = nwfilterConnectNumOfNWFilters, /* 0.8.0 */ @@ -795,6 +870,8 @@ static virNWFilterDriver nwfilterDriver = { .nwfilterBindingLookupByPortDev = nwfilterBindingLookupByPortDev, /* 4.4.0 */ .connectListAllNWFilterBindings = nwfilterConnectListAllNWFilterBindings, /* 4.4.0 */ .nwfilterBindingGetXMLDesc = nwfilterBindingGetXMLDesc, /* 4.4.0 */ + .nwfilterBindingCreateXML = nwfilterBindingCreateXML, /* 4.4.0 */ + .nwfilterBindingDelete = nwfilterBindingDelete, /* 4.4.0 */ }; -- 2.17.0

Remove the callbacks that the nwfilter driver registers with the domain object config layer. Instead make the current helper methods call into the public API for creating/deleting nwfilter bindings. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/conf/domain_nwfilter.c | 124 +++++++++++++++++++++---- src/conf/domain_nwfilter.h | 13 --- src/libvirt_private.syms | 1 - src/nwfilter/nwfilter_driver.c | 83 +++-------------- src/nwfilter/nwfilter_gentech_driver.c | 42 --------- src/nwfilter/nwfilter_gentech_driver.h | 4 - 6 files changed, 120 insertions(+), 147 deletions(-) diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c index 7570e0ae83..ed45394918 100644 --- a/src/conf/domain_nwfilter.c +++ b/src/conf/domain_nwfilter.c @@ -28,45 +28,137 @@ #include "datatypes.h" #include "domain_conf.h" #include "domain_nwfilter.h" +#include "virnwfilterbindingdef.h" #include "virerror.h" +#include "viralloc.h" +#include "virstring.h" +#include "virlog.h" -#define VIR_FROM_THIS VIR_FROM_NWFILTER -static virDomainConfNWFilterDriverPtr nwfilterDriver; +VIR_LOG_INIT("conf.domain_nwfilter"); -void -virDomainConfNWFilterRegister(virDomainConfNWFilterDriverPtr driver) +#define VIR_FROM_THIS VIR_FROM_NWFILTER + +static virNWFilterBindingDefPtr +virNWFilterBindingDefForNet(const char *vmname, + const unsigned char *vmuuid, + virDomainNetDefPtr net) { - nwfilterDriver = driver; + virNWFilterBindingDefPtr ret; + + if (VIR_ALLOC(ret) < 0) + return NULL; + + if (VIR_STRDUP(ret->ownername, vmname) < 0) + goto error; + + memcpy(ret->owneruuid, vmuuid, sizeof(ret->owneruuid)); + + if (VIR_STRDUP(ret->portdevname, net->ifname) < 0) + goto error; + + if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT && + VIR_STRDUP(ret->linkdevname, net->data.direct.linkdev) < 0) + goto error; + + ret->mac = net->mac; + + if (VIR_STRDUP(ret->filter, net->filter) < 0) + goto error; + + if (!(ret->filterparams = virNWFilterHashTableCreate(0))) + goto error; + + if (net->filterparams && + virNWFilterHashTablePutAll(net->filterparams, ret->filterparams) < 0) + goto error; + + return ret; + + error: + virNWFilterBindingDefFree(ret); + return NULL; } + int virDomainConfNWFilterInstantiate(const char *vmname, const unsigned char *vmuuid, virDomainNetDefPtr net) { - if (nwfilterDriver != NULL) - return nwfilterDriver->instantiateFilter(vmname, vmuuid, net); + virConnectPtr conn = virGetConnectNWFilter(); + virNWFilterBindingDefPtr def = NULL; + virNWFilterBindingPtr binding = NULL; + char *xml; + int ret = -1; + + VIR_DEBUG("vmname=%s portdev=%s filter=%s", + vmname, NULLSTR(net->ifname), NULLSTR(net->filter)); + + if (!conn) + goto cleanup; + + if (!(def = virNWFilterBindingDefForNet(vmname, vmuuid, net))) + goto cleanup; + + if (!(xml = virNWFilterBindingDefFormat(def))) + goto cleanup; + + if (!(binding = virNWFilterBindingCreateXML(conn, xml, 0))) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(xml); + virNWFilterBindingDefFree(def); + virObjectUnref(binding); + virObjectUnref(conn); + return ret; +} + + +static void +virDomainConfNWFilterTeardownImpl(virConnectPtr conn, + virDomainNetDefPtr net) +{ + virNWFilterBindingPtr binding; - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("No network filter driver available")); - return -1; + binding = virNWFilterBindingLookupByPortDev(conn, net->ifname); + if (!binding) + return; + + virNWFilterBindingDelete(binding); + + virObjectUnref(binding); } + void virDomainConfNWFilterTeardown(virDomainNetDefPtr net) { - if (nwfilterDriver != NULL) - nwfilterDriver->teardownFilter(net); + virConnectPtr conn = virGetConnectNWFilter(); + + if (!conn) + return; + + virDomainConfNWFilterTeardownImpl(conn, net); + + virObjectUnref(conn); } void virDomainConfVMNWFilterTeardown(virDomainObjPtr vm) { size_t i; + virConnectPtr conn = virGetConnectNWFilter(); + + if (!conn) + return; + + + for (i = 0; i < vm->def->nnets; i++) + virDomainConfNWFilterTeardownImpl(conn, vm->def->nets[i]); - if (nwfilterDriver != NULL) { - for (i = 0; i < vm->def->nnets; i++) - virDomainConfNWFilterTeardown(vm->def->nets[i]); - } + virObjectUnref(conn); } diff --git a/src/conf/domain_nwfilter.h b/src/conf/domain_nwfilter.h index 857cac6c2a..d2ebeff853 100644 --- a/src/conf/domain_nwfilter.h +++ b/src/conf/domain_nwfilter.h @@ -23,19 +23,6 @@ #ifndef DOMAIN_NWFILTER_H # define DOMAIN_NWFILTER_H -typedef int (*virDomainConfInstantiateNWFilter)(const char *vmname, - const unsigned char *vmuuid, - virDomainNetDefPtr net); -typedef void (*virDomainConfTeardownNWFilter)(virDomainNetDefPtr net); - -typedef struct { - virDomainConfInstantiateNWFilter instantiateFilter; - virDomainConfTeardownNWFilter teardownFilter; -} virDomainConfNWFilterDriver; -typedef virDomainConfNWFilterDriver *virDomainConfNWFilterDriverPtr; - -void virDomainConfNWFilterRegister(virDomainConfNWFilterDriverPtr driver); - int virDomainConfNWFilterInstantiate(const char *vmname, const unsigned char *vmuuid, virDomainNetDefPtr net); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a34f899379..f1f7f19014 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -645,7 +645,6 @@ virDomainQemuMonitorEventStateRegisterID; # conf/domain_nwfilter.h virDomainConfNWFilterInstantiate; -virDomainConfNWFilterRegister; virDomainConfNWFilterTeardown; virDomainConfVMNWFilterTeardown; diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index c3c52ae5f3..9ee5c57d9f 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -654,66 +654,6 @@ nwfilterGetXMLDesc(virNWFilterPtr nwfilter, } -static int -nwfilterInstantiateFilter(const char *vmname, - const unsigned char *vmuuid, - virDomainNetDefPtr net) -{ - virNWFilterBindingObjPtr obj; - virNWFilterBindingDefPtr def; - int ret; - - obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, net->ifname); - if (obj) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Filter already present for NIC %s"), net->ifname); - virNWFilterBindingObjEndAPI(&obj); - return -1; - } - - if (!(def = virNWFilterBindingDefForNet(vmname, vmuuid, net))) - return -1; - - obj = virNWFilterBindingObjListAdd(driver->bindings, - def); - if (!obj) { - virNWFilterBindingDefFree(def); - return -1; - } - def = NULL; - - ret = virNWFilterInstantiateFilter(driver, obj->def); - - if (ret < 0) - virNWFilterBindingObjListRemove(driver->bindings, obj); - - virNWFilterBindingObjSave(obj, driver->bindingDir); - - virNWFilterBindingObjEndAPI(&obj); - - return ret; -} - - -static void -nwfilterTeardownFilter(virDomainNetDefPtr net) -{ - virNWFilterBindingObjPtr obj; - if (!net->ifname) - return; - - obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, net->ifname); - if (!obj) - return; - - virNWFilterTeardownFilter(obj->def); - virNWFilterBindingObjDelete(obj, driver->bindingDir); - - virNWFilterBindingObjListRemove(driver->bindings, obj); - virNWFilterBindingObjEndAPI(&obj); -} - - static virNWFilterBindingPtr nwfilterBindingLookupByPortDev(virConnectPtr conn, const char *portdev) @@ -723,8 +663,11 @@ nwfilterBindingLookupByPortDev(virConnectPtr conn, obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, portdev); - if (!obj) + if (!obj) { + virReportError(VIR_ERR_NO_NWFILTER_BINDING, + _("no nwfilter binding for port dev '%s'"), portdev); goto cleanup; + } if (virNWFilterBindingLookupByPortDevEnsureACL(conn, obj->def) < 0) goto cleanup; @@ -768,8 +711,11 @@ nwfilterBindingGetXMLDesc(virNWFilterBindingPtr binding, obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, binding->portdev); - if (!obj) + if (!obj) { + virReportError(VIR_ERR_NO_NWFILTER_BINDING, + _("no nwfilter binding for port dev '%s'"), binding->portdev); goto cleanup; + } if (virNWFilterBindingGetXMLDescEnsureACL(binding->conn, obj->def) < 0) goto cleanup; @@ -839,8 +785,11 @@ nwfilterBindingDelete(virNWFilterBindingPtr binding) int ret = -1; obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, binding->portdev); - if (!obj) + if (!obj) { + virReportError(VIR_ERR_NO_NWFILTER_BINDING, + _("no nwfilter binding for port dev '%s'"), binding->portdev); return -1; + } if (virNWFilterBindingDeleteEnsureACL(binding->conn, obj->def) < 0) goto cleanup; @@ -900,13 +849,6 @@ static virStateDriver stateDriver = { .stateReload = nwfilterStateReload, }; - -static virDomainConfNWFilterDriver domainNWFilterDriver = { - .instantiateFilter = nwfilterInstantiateFilter, - .teardownFilter = nwfilterTeardownFilter, -}; - - int nwfilterRegister(void) { if (virRegisterConnectDriver(&nwfilterConnectDriver, false) < 0) @@ -915,6 +857,5 @@ int nwfilterRegister(void) return -1; if (virRegisterStateDriver(&stateDriver) < 0) return -1; - virDomainConfNWFilterRegister(&domainNWFilterDriver); return 0; } diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index de7361f3dd..f43decb6ea 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -1106,45 +1106,3 @@ virNWFilterBuildAll(virNWFilterDriverStatePtr driver, } return ret; } - - -virNWFilterBindingDefPtr -virNWFilterBindingDefForNet(const char *vmname, - const unsigned char *vmuuid, - virDomainNetDefPtr net) -{ - virNWFilterBindingDefPtr ret; - - if (VIR_ALLOC(ret) < 0) - return NULL; - - if (VIR_STRDUP(ret->ownername, vmname) < 0) - goto error; - - memcpy(ret->owneruuid, vmuuid, sizeof(ret->owneruuid)); - - if (VIR_STRDUP(ret->portdevname, net->ifname) < 0) - goto error; - - if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT && - VIR_STRDUP(ret->linkdevname, net->data.direct.linkdev) < 0) - goto error; - - ret->mac = net->mac; - - if (VIR_STRDUP(ret->filter, net->filter) < 0) - goto error; - - if (!(ret->filterparams = virNWFilterHashTableCreate(0))) - goto error; - - if (net->filterparams && - virNWFilterHashTablePutAll(net->filterparams, ret->filterparams) < 0) - goto error; - - return ret; - - error: - virNWFilterBindingDefFree(ret); - return NULL; -} diff --git a/src/nwfilter/nwfilter_gentech_driver.h b/src/nwfilter/nwfilter_gentech_driver.h index 481fdd2413..2cd19c90fc 100644 --- a/src/nwfilter/nwfilter_gentech_driver.h +++ b/src/nwfilter/nwfilter_gentech_driver.h @@ -57,8 +57,4 @@ virHashTablePtr virNWFilterCreateVarHashmap(const char *macaddr, int virNWFilterBuildAll(virNWFilterDriverStatePtr driver, bool newFilters); -virNWFilterBindingDefPtr virNWFilterBindingDefForNet(const char *vmname, - const unsigned char *vmuuid, - virDomainNetDefPtr net); - #endif -- 2.17.0