On 05/15/2018 01:43 PM, Daniel P. Berrangé wrote:
> 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@xxxxxxxxxx>
> ---
> 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(+)
>
I keep wondering, is there anything else that needs to be in
virNWFilterBindingPtr... Or some other parameter or way we need to get
the data we need. Which further makes me wonder should we wait for
4.5.0 just in case something comes up since we're now more than halfway
through the month?
There's mostly some nits and small adjustments below, but in general
with those taken care of...
Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>
John
[...]
>
> +/**
> + * 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)
One argument per line (this repeats a few times, I'll mention it once -
your call)
> +{
> + virNWFilterBindingPtr ret = NULL;
> +
> + if (virDataTypesInitialize() < 0)
> + return NULL;
> +
> + virCheckConnectGoto(conn, error);
> + virCheckNonNullArgGoto(portdev, error);
Check filtername too...
> +
> + 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);
Your call to print the filtername too
> +
> + VIR_FREE(binding->portdev);
VIR_FREE(binding->filtername);
> + virObjectUnref(binding->conn);
> +}
> +
> +
> /**
> * virGetDomainSnapshot:
> * @domain: the domain to snapshot
[...]
> 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
s/to NULL/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)
[...]
> +
> +/**
> + * 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.
need not be free()'d in its lifetime as it is owned by the binding object.
[or something else - the existing sentence needs some sprucing up]
> + */
> +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.
similar comment...
> + */
> +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
virNWFilterBindingFree
> + * 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();
[...]
> +
> +/**
> + * 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
virNWFilterBindingFree
> + * 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.
> + */
[...]
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
