[libvirt] [PATCH 0/5 v4] Atomic API to list network filters

v3 - v4: * Just rebase on the top, and split the patches from v3's large set. Osier Yang (5): list: Define new API virConnectListAllNWFilters list: Implement RPC calls for virConnectListAllNWFilters list: Implement listAllNWFilters list: Expose virConnectListAllNWFilters to Python binding list: Use virConnectListAllNWFilters in virsh daemon/remote.c | 54 +++++++++++ include/libvirt/libvirt.h.in | 4 +- python/generator.py | 1 + python/libvirt-override-api.xml | 6 + python/libvirt-override-virConnect.py | 12 +++ python/libvirt-override.c | 48 ++++++++++ src/driver.h | 5 + src/libvirt.c | 50 ++++++++++ src/libvirt_public.syms | 1 + src/nwfilter/nwfilter_driver.c | 56 +++++++++++ src/remote/remote_driver.c | 63 +++++++++++++ src/remote/remote_protocol.x | 13 +++- src/remote_protocol-structs | 12 +++ tools/virsh-nwfilter.c | 163 +++++++++++++++++++++++++++------ 14 files changed, 457 insertions(+), 31 deletions(-) -- 1.7.7.3

This is to list the network fitler objects. No flags are supported include/libvirt/libvirt.h.in: Declare enum virConnectListAllNWFilterFlags and virConnectListAllNWFilters. python/generator.py: Skip auto-generating src/driver.h: (virDrvConnectListAllNWFilters) src/libvirt.c: Implement the public API src/libvirt_public.syms: Export the symbol to public --- include/libvirt/libvirt.h.in | 4 ++- python/generator.py | 1 + src/driver.h | 5 ++++ src/libvirt.c | 50 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 5 files changed, 60 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 96d0760..86f640d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4115,7 +4115,9 @@ int virConnectNumOfNWFilters (virConnectPtr conn); int virConnectListNWFilters (virConnectPtr conn, char **const names, int maxnames); - +int virConnectListAllNWFilters(virConnectPtr conn, + virNWFilterPtr **filters, + unsigned int flags); /* * Lookup nwfilter by name or uuid */ diff --git a/python/generator.py b/python/generator.py index a8e4ec6..d3163e4 100755 --- a/python/generator.py +++ b/python/generator.py @@ -465,6 +465,7 @@ skip_function = ( 'virConnectListAllNetworks', # overridden in virConnect.py 'virConnectListAllInterfaces', # overridden in virConnect.py 'virConnectListAllNodeDevices', # overridden in virConnect.py + 'virConnectListAllNWFilters', # overridden in virConnect.py 'virStreamRecvAll', # Pure python libvirt-override-virStream.py 'virStreamSendAll', # Pure python libvirt-override-virStream.py diff --git a/src/driver.h b/src/driver.h index 34a94af..9984a85 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1638,6 +1638,10 @@ typedef int (*virDrvConnectListNWFilters) (virConnectPtr conn, char **const names, int maxnames); +typedef int + (*virDrvConnectListAllNWFilters) (virConnectPtr conn, + virNWFilterPtr **filters, + unsigned int flags); typedef virNWFilterPtr (*virDrvNWFilterLookupByName) (virConnectPtr conn, const char *name); @@ -1675,6 +1679,7 @@ struct _virNWFilterDriver { virDrvConnectNumOfNWFilters numOfNWFilters; virDrvConnectListNWFilters listNWFilters; + virDrvConnectListAllNWFilters listAllNWFilters; virDrvNWFilterLookupByName nwfilterLookupByName; virDrvNWFilterLookupByUUID nwfilterLookupByUUID; virDrvNWFilterDefineXML defineXML; diff --git a/src/libvirt.c b/src/libvirt.c index b8d0bec..55a2f4e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16153,6 +16153,56 @@ error: return -1; } +/** + * virConnectListAllNWFilters: + * @conn: Pointer to the hypervisor connection. + * @filters: 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 +virConnectListAllNWFilters(virConnectPtr conn, + virNWFilterPtr **filters, + unsigned int flags) +{ + VIR_DEBUG("conn=%p, filters=%p, flags=%x", conn, filters, flags); + + virResetLastError(); + + if (filters) + *filters = NULL; + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (conn->nwfilterDriver && + conn->nwfilterDriver->listAllNWFilters) { + int ret; + ret = conn->nwfilterDriver->listAllNWFilters(conn, filters, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return -1; +} /** * virConnectListNWFilters: diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 5a4451b..a918bc8 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -559,6 +559,7 @@ LIBVIRT_0.10.2 { virConnectListAllInterfaces; virConnectListAllNetworks; virConnectListAllNodeDevices; + virConnectListAllNWFilters; virConnectListAllStoragePools; virStoragePoolListAllVolumes; } LIBVIRT_0.10.2; -- 1.7.7.3

On 09/05/2012 12:02 AM, Osier Yang wrote:
This is to list the network fitler objects. No flags are supported
s/fitler/filter/
+/** + * virConnectListAllNWFilters: + * @conn: Pointer to the hypervisor connection. + * @filters: 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.
Maybe it's worth filtering on whether a filter is just references to other filters, contains rules, or contains both. But for that to be useful, it would also be nice to have an API where, given an individual virNWFilterPtr, we can then return a list of all other virNWFilterPtr that were pulled in by reference (that is, similar to snapshots, have an API sufficiently powerful to reconstruct the tree of filter references). So I'm okay with this version going in with no flag support for now. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

The RPC generator doesn't support returning list of object yet, this patch do the work manually. * daemon/remote.c: Implemente the server side handler remoteDispatchConnectListAllNWFilters. * src/remote/remote_driver.c: Add remote driver handler remoteConnectListAllNWFilters. * src/remote/remote_protocol.x: New RPC procedure REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS and structs to represent the args and ret for it. * src/remote_protocol-structs: Likewise. --- daemon/remote.c | 54 ++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 63 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 13 ++++++++- src/remote_protocol-structs | 12 ++++++++ 4 files changed, 141 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index d7cce78..00ce9c8 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -4373,6 +4373,60 @@ cleanup: return rv; } +static int +remoteDispatchConnectListAllNWFilters(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_connect_list_all_nwfilters_args *args, + remote_connect_list_all_nwfilters_ret *ret) +{ + virNWFilterPtr *filters = NULL; + int nfilters = 0; + int i; + int rv = -1; + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if ((nfilters = virConnectListAllNWFilters(priv->conn, + args->need_results ? &filters : NULL, + args->flags)) < 0) + goto cleanup; + + if (filters && nfilters) { + if (VIR_ALLOC_N(ret->filters.filters_val, nfilters) < 0) { + virReportOOMError(); + goto cleanup; + } + + ret->filters.filters_len = nfilters; + + for (i = 0; i < nfilters; i++) + make_nonnull_nwfilter(ret->filters.filters_val + i, filters[i]); + } else { + ret->filters.filters_len = 0; + ret->filters.filters_val = NULL; + } + + ret->ret = nfilters; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (filters) { + for (i = 0; i < nfilters; i++) + virNWFilterFree(filters[i]); + VIR_FREE(filters); + } + return rv; +} + /*----- Helpers. -----*/ /* get_nonnull_domain and get_nonnull_network turn an on-wire diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b1671ae..2afe5b0 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2907,6 +2907,68 @@ done: return rv; } +static int +remoteConnectListAllNWFilters(virConnectPtr conn, + virNWFilterPtr **filters, + unsigned int flags) +{ + int rv = -1; + int i; + virNWFilterPtr *tmp_filters = NULL; + remote_connect_list_all_nwfilters_args args; + remote_connect_list_all_nwfilters_ret ret; + + struct private_data *priv = conn->privateData; + + remoteDriverLock(priv); + + args.need_results = !!filters; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + if (call(conn, + priv, + 0, + REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS, + (xdrproc_t) xdr_remote_connect_list_all_nwfilters_args, + (char *) &args, + (xdrproc_t) xdr_remote_connect_list_all_nwfilters_ret, + (char *) &ret) == -1) + goto done; + + if (filters) { + if (VIR_ALLOC_N(tmp_filters, ret.filters.filters_len + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0; i < ret.filters.filters_len; i++) { + tmp_filters[i] = get_nonnull_nwfilter (conn, ret.filters.filters_val[i]); + if (!tmp_filters[i]) { + virReportOOMError(); + goto cleanup; + } + } + *filters = tmp_filters; + tmp_filters = NULL; + } + + rv = ret.ret; + +cleanup: + if (tmp_filters) { + for (i = 0; i < ret.filters.filters_len; i++) + if (tmp_filters[i]) + virNWFilterFree(tmp_filters[i]); + VIR_FREE(tmp_filters); + } + + xdr_free((xdrproc_t) xdr_remote_connect_list_all_nwfilters_ret, (char *) &ret); + +done: + remoteDriverUnlock(priv); + return rv; +} /*----------------------------------------------------------------------*/ @@ -6012,6 +6074,7 @@ static virNWFilterDriver nwfilter_driver = { .undefine = remoteNWFilterUndefine, /* 0.8.0 */ .numOfNWFilters = remoteNumOfNWFilters, /* 0.8.0 */ .listNWFilters = remoteListNWFilters, /* 0.8.0 */ + .listAllNWFilters = remoteConnectListAllNWFilters, /* 0.10.2 */ }; diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index d467772..46269c8 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2609,6 +2609,16 @@ struct remote_connect_list_all_node_devices_ret { unsigned int ret; }; +struct remote_connect_list_all_nwfilters_args { + int need_results; + unsigned int flags; +}; + +struct remote_connect_list_all_nwfilters_ret { + remote_nonnull_nwfilter filters<>; + unsigned int ret; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -2944,7 +2954,8 @@ enum remote_procedure { REMOTE_PROC_STORAGE_POOL_LIST_ALL_VOLUMES = 282, /* skipgen skipgen priority:high */ REMOTE_PROC_CONNECT_LIST_ALL_NETWORKS = 283, /* skipgen skipgen priority:high */ REMOTE_PROC_CONNECT_LIST_ALL_INTERFACES = 284, /* skipgen skipgen priority:high */ - REMOTE_PROC_CONNECT_LIST_ALL_NODE_DEVICES = 285 /* skipgen skipgen priority:high */ + REMOTE_PROC_CONNECT_LIST_ALL_NODE_DEVICES = 285, /* skipgen skipgen priority:high */ + REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS = 286 /* skipgen skipgen priority:high */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index cd7bc50..0dde9fc 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2067,6 +2067,17 @@ struct remote_connect_list_all_node_devices_ret { } devices; u_int ret; }; +struct remote_connect_list_all_nwfilters_args { + int need_results; + u_int flags; +}; +struct remote_connect_list_all_nwfilters_ret { + struct { + u_int filters_len; + remote_nonnull_nwfilter * filters_val; + } filters; + u_int ret; +}; enum remote_procedure { REMOTE_PROC_OPEN = 1, REMOTE_PROC_CLOSE = 2, @@ -2353,4 +2364,5 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_NETWORKS = 283, REMOTE_PROC_CONNECT_LIST_ALL_INTERFACES = 284, REMOTE_PROC_CONNECT_LIST_ALL_NODE_DEVICES = 285, + REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS = 286, }; -- 1.7.7.3

On 09/05/12 08:02, Osier Yang wrote:
The RPC generator doesn't support returning list of object yet, this patch do the work manually.
* daemon/remote.c: Implemente the server side handler remoteDispatchConnectListAllNWFilters.
* src/remote/remote_driver.c: Add remote driver handler remoteConnectListAllNWFilters.
* src/remote/remote_protocol.x: New RPC procedure REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS and structs to represent the args and ret for it.
* src/remote_protocol-structs: Likewise. --- daemon/remote.c | 54 ++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 63 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 13 ++++++++- src/remote_protocol-structs | 12 ++++++++ 4 files changed, 141 insertions(+), 1 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b1671ae..2afe5b0 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2907,6 +2907,68 @@ done: return rv; }
+static int +remoteConnectListAllNWFilters(virConnectPtr conn, + virNWFilterPtr **filters, + unsigned int flags) +{ + int rv = -1; + int i; + virNWFilterPtr *tmp_filters = NULL; + remote_connect_list_all_nwfilters_args args; + remote_connect_list_all_nwfilters_ret ret; + + struct private_data *priv = conn->privateData; + + remoteDriverLock(priv); + + args.need_results = !!filters; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + if (call(conn, + priv, + 0, + REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS, + (xdrproc_t) xdr_remote_connect_list_all_nwfilters_args, + (char *) &args, + (xdrproc_t) xdr_remote_connect_list_all_nwfilters_ret, + (char *) &ret) == -1) + goto done; + + if (filters) { + if (VIR_ALLOC_N(tmp_filters, ret.filters.filters_len + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0; i < ret.filters.filters_len; i++) { + tmp_filters[i] = get_nonnull_nwfilter (conn, ret.filters.filters_val[i]);
Space before function arguments.
+ if (!tmp_filters[i]) { + virReportOOMError(); + goto cleanup; + } + } + *filters = tmp_filters; + tmp_filters = NULL; + } + + rv = ret.ret; + +cleanup: + if (tmp_filters) { + for (i = 0; i < ret.filters.filters_len; i++) + if (tmp_filters[i]) + virNWFilterFree(tmp_filters[i]); + VIR_FREE(tmp_filters); + } + + xdr_free((xdrproc_t) xdr_remote_connect_list_all_nwfilters_ret, (char *) &ret); + +done: + remoteDriverUnlock(priv); + return rv; +}
/*----------------------------------------------------------------------*/
@@ -6012,6 +6074,7 @@ static virNWFilterDriver nwfilter_driver = { .undefine = remoteNWFilterUndefine, /* 0.8.0 */ .numOfNWFilters = remoteNumOfNWFilters, /* 0.8.0 */ .listNWFilters = remoteListNWFilters, /* 0.8.0 */ + .listAllNWFilters = remoteConnectListAllNWFilters, /* 0.10.2 */ };
Otherwise looks OK. Peter

Simply returns the object list. No filtering. src/nwfilter/nwfilter_driver.c: Implement listAllNWFilters --- src/nwfilter/nwfilter_driver.c | 56 ++++++++++++++++++++++++++++++++++++++++ 1 files changed, 56 insertions(+), 0 deletions(-) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index a30026e..0cf10b8 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -492,6 +492,61 @@ nwfilterListNWFilters(virConnectPtr conn, } +static int +nwfilterListAllNWFilters(virConnectPtr conn, + virNWFilterPtr **filters, + unsigned int flags) { + virNWFilterDriverStatePtr driver = conn->nwfilterPrivateData; + virNWFilterPtr *tmp_filters = NULL; + int nfilters = 0; + virNWFilterPtr filter = NULL; + virNWFilterObjPtr obj = NULL; + int i; + int ret = -1; + + virCheckFlags(0, -1); + + nwfilterDriverLock(driver); + + if (!filters) { + ret = driver->nwfilters.count; + goto cleanup; + } + + if (VIR_ALLOC_N(tmp_filters, driver->nwfilters.count + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < driver->nwfilters.count; i++) { + obj = driver->nwfilters.objs[i]; + virNWFilterObjLock(obj); + if (!(filter = virGetNWFilter(conn, obj->def->name, + obj->def->uuid))) { + virNWFilterObjUnlock(obj); + goto cleanup; + } + virNWFilterObjUnlock(obj); + tmp_filters[nfilters++] = filter; + } + + *filters = tmp_filters; + tmp_filters = NULL; + ret = nfilters; + + cleanup: + nwfilterDriverUnlock(driver); + if (tmp_filters) { + for (i = 0; i < nfilters; i ++) { + if (tmp_filters[i]) + virNWFilterFree(tmp_filters[i]); + } + } + VIR_FREE(tmp_filters); + + return ret; +} + static virNWFilterPtr nwfilterDefine(virConnectPtr conn, const char *xml) @@ -627,6 +682,7 @@ static virNWFilterDriver nwfilterDriver = { .close = nwfilterClose, /* 0.8.0 */ .numOfNWFilters = nwfilterNumNWFilters, /* 0.8.0 */ .listNWFilters = nwfilterListNWFilters, /* 0.8.0 */ + .listAllNWFilters = nwfilterListAllNWFilters, /* 0.10.2 */ .nwfilterLookupByName = nwfilterLookupByName, /* 0.8.0 */ .nwfilterLookupByUUID = nwfilterLookupByUUID, /* 0.8.0 */ .defineXML = nwfilterDefine, /* 0.8.0 */ -- 1.7.7.3

The implementation is done manually as the generator does not support wrapping lists of C pointers into Python objects. python/libvirt-override-api.xml: Document python/libvirt-override-virConnect.py: * Implementation for listAllNWFilters. python/libvirt-override.c: Implementation for the wrapper. --- python/libvirt-override-api.xml | 6 ++++ python/libvirt-override-virConnect.py | 12 ++++++++ python/libvirt-override.c | 48 +++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 0 deletions(-) diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index a3d6bbb..b8ab823 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -396,6 +396,12 @@ <arg name='conn' type='virConnectPtr' info='virConnect connection'/> <return type='str *' info='the list of network filter IDs or None in case of error'/> </function> + <function name='virConnectListAllNWFilters' file='python'> + <info>returns list of all network fitlers</info> + <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/> + <arg name='flags' type='unsigned int' info='optional flags'/> + <return type='nwfilter *' info='the list of network filters or None in case of error'/> + </function> <function name='virNWFilterLookupByUUID' file='python'> <info>Try to lookup a network filter on the given hypervisor based on its UUID.</info> <return type='virNWFilterPtr' info='a new network filter object or NULL in case of failure'/> diff --git a/python/libvirt-override-virConnect.py b/python/libvirt-override-virConnect.py index 0859c36..caca982 100644 --- a/python/libvirt-override-virConnect.py +++ b/python/libvirt-override-virConnect.py @@ -254,3 +254,15 @@ retlist.append(virNodeDevice(self, _obj=devptr)) return retlist + + def listAllNWFilters(self, flags): + """Returns a list of network filter objects""" + ret = libvirtmod.virConnectListAllNWFilters(self._o, flags) + if ret is None: + raise libvirtError("virConnectListAllNWFilters() failed", conn=self) + + retlist = list() + for filter_ptr in ret: + retlist.append(virNWFilter(self, _obj=filter_ptr)) + + return retlist diff --git a/python/libvirt-override.c b/python/libvirt-override.c index a0478cd..b344ff5 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3764,6 +3764,53 @@ libvirt_virConnectListNWFilters(PyObject *self ATTRIBUTE_UNUSED, } static PyObject * +libvirt_virConnectListAllNWFilters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *pyobj_conn; + PyObject *py_retval = NULL; + PyObject *tmp = NULL; + virConnectPtr conn; + virNWFilterPtr *filters = NULL; + int c_retval = 0; + int i; + unsigned int flags; + + if (!PyArg_ParseTuple(args, (char *)"Oi:virConnectListAllNWFilters", + &pyobj_conn, &flags)) + return NULL; + conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virConnectListAllNWFilters(conn, &filters, flags); + LIBVIRT_END_ALLOW_THREADS; + if (c_retval < 0) + return VIR_PY_NONE; + + if (!(py_retval = PyList_New(c_retval))) + goto cleanup; + + for (i = 0; i < c_retval; i++) { + if (!(tmp = libvirt_virNWFilterPtrWrap(filters[i])) || + PyList_SetItem(py_retval, i, tmp) < 0) { + Py_XDECREF(tmp); + Py_DECREF(py_retval); + py_retval = NULL; + goto cleanup; + } + /* python steals the pointer */ + filters[i] = NULL; + } + +cleanup: + for (i = 0; i < c_retval; i++) + if (filters[i]) + virNWFilterFree(filters[i]); + VIR_FREE(filters); + return py_retval; +} + +static PyObject * libvirt_virConnectListInterfaces(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *py_retval; @@ -6142,6 +6189,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virNWFilterGetUUIDString", libvirt_virNWFilterGetUUIDString, METH_VARARGS, NULL}, {(char *) "virNWFilterLookupByUUID", libvirt_virNWFilterLookupByUUID, METH_VARARGS, NULL}, {(char *) "virConnectListNWFilters", libvirt_virConnectListNWFilters, METH_VARARGS, NULL}, + {(char *) "virConnectListAllNWFilters", libvirt_virConnectListAllNWFilters, METH_VARARGS, NULL}, {(char *) "virConnectListInterfaces", libvirt_virConnectListInterfaces, METH_VARARGS, NULL}, {(char *) "virConnectListDefinedInterfaces", libvirt_virConnectListDefinedInterfaces, METH_VARARGS, NULL}, {(char *) "virConnectListAllInterfaces", libvirt_virConnectListAllInterfaces, METH_VARARGS, NULL}, -- 1.7.7.3

tools/virsh-nwfilter.c: * vshNWFilterSorter to sort network filters by name * vshNWFilterListFree to free the network filter objects list. * vshNWFilterListCollect to collect the network filter objects, trying to use new API first, fall back to older APIs if it's not supported. --- tools/virsh-nwfilter.c | 163 +++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 134 insertions(+), 29 deletions(-) diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index 5169d38..57cf2b7 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -190,6 +190,134 @@ cmdNWFilterDumpXML(vshControl *ctl, const vshCmd *cmd) return ret; } +static int +vshNWFilterSorter(const void *a, const void *b) +{ + virNWFilterPtr *fa = (virNWFilterPtr *) a; + virNWFilterPtr *fb = (virNWFilterPtr *) b; + + if (*fa && !*fb) + return -1; + + if (!*fa) + return *fb != NULL; + + return vshStrcasecmp(virNWFilterGetName(*fa), + virNWFilterGetName(*fb)); +} + +struct vshNWFilterList { + virNWFilterPtr *filters; + size_t nfilters; +}; +typedef struct vshNWFilterList *vshNWFilterListPtr; + +static void +vshNWFilterListFree(vshNWFilterListPtr list) +{ + int i; + + if (list && list->nfilters) { + for (i = 0; i < list->nfilters; i++) { + if (list->filters[i]) + virNWFilterFree(list->filters[i]); + } + VIR_FREE(list->filters); + } + VIR_FREE(list); +} + +static vshNWFilterListPtr +vshNWFilterListCollect(vshControl *ctl, + unsigned int flags) +{ + vshNWFilterListPtr list = vshMalloc(ctl, sizeof(*list)); + int i; + int ret; + virNWFilterPtr filter; + bool success = false; + size_t deleted = 0; + int nfilters = 0; + char **names = NULL; + + /* try the list with flags support (0.10.2 and later) */ + if ((ret = virConnectListAllNWFilters(ctl->conn, + &list->filters, + flags)) >= 0) { + list->nfilters = ret; + goto finished; + } + + /* check if the command is actually supported */ + if (last_error && last_error->code == VIR_ERR_NO_SUPPORT) { + vshResetLibvirtError(); + goto fallback; + } + + /* there was an error during the call */ + vshError(ctl, "%s", _("Failed to list node filters")); + goto cleanup; + + +fallback: + /* fall back to old method (0.9.13 and older) */ + vshResetLibvirtError(); + + nfilters = virConnectNumOfNWFilters(ctl->conn); + if (nfilters < 0) { + vshError(ctl, "%s", _("Failed to count network filters")); + goto cleanup; + } + + if (nfilters == 0) + return list; + + names = vshMalloc(ctl, sizeof(char *) * nfilters); + + nfilters = virConnectListNWFilters(ctl->conn, names, nfilters); + if (nfilters < 0) { + vshError(ctl, "%s", _("Failed to list network filters")); + goto cleanup; + } + + list->filters = vshMalloc(ctl, sizeof(virNWFilterPtr) * (nfilters)); + list->nfilters = 0; + + /* get the network filters */ + for (i = 0; i < nfilters ; i++) { + if (!(filter = virNWFilterLookupByName(ctl->conn, names[i]))) + continue; + list->filters[list->nfilters++] = filter; + } + + /* truncate network filters that weren't found */ + deleted = nfilters - list->nfilters; + +finished: + /* sort the list */ + if (list->filters && list->nfilters) + qsort(list->filters, list->nfilters, + sizeof(*list->filters), vshNWFilterSorter); + + /* truncate the list for not found filter objects */ + if (deleted) + VIR_SHRINK_N(list->filters, list->nfilters, deleted); + + success = true; + +cleanup: + for (i = 0; i < nfilters; i++) + VIR_FREE(names[i]); + VIR_FREE(names); + + if (!success) { + vshNWFilterListFree(list); + list = NULL; + } + + return list; +} + /* * "nwfilter-list" command */ @@ -206,50 +334,27 @@ static const vshCmdOptDef opts_nwfilter_list[] = { static bool cmdNWFilterList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { - int numfilters, i; - char **names; + int i; char uuid[VIR_UUID_STRING_BUFLEN]; + vshNWFilterListPtr list = NULL; - numfilters = virConnectNumOfNWFilters(ctl->conn); - if (numfilters < 0) { - vshError(ctl, "%s", _("Failed to list network filters")); - return false; - } - - names = vshMalloc(ctl, sizeof(char *) * numfilters); - - if ((numfilters = virConnectListNWFilters(ctl->conn, names, - numfilters)) < 0) { - vshError(ctl, "%s", _("Failed to list network filters")); - VIR_FREE(names); + if (!(list = vshNWFilterListCollect(ctl, 0))) return false; - } - - qsort(&names[0], numfilters, sizeof(char *), vshNameSorter); vshPrintExtra(ctl, "%-36s %-20s \n", _("UUID"), _("Name")); vshPrintExtra(ctl, "----------------------------------------------------------------\n"); - for (i = 0; i < numfilters; i++) { - virNWFilterPtr nwfilter = - virNWFilterLookupByName(ctl->conn, names[i]); - - /* this kind of work with networks is not atomic operation */ - if (!nwfilter) { - VIR_FREE(names[i]); - continue; - } + for (i = 0; i < list->nfilters; i++) { + virNWFilterPtr nwfilter = list->filters[i]; virNWFilterGetUUIDString(nwfilter, uuid); vshPrint(ctl, "%-36s %-20s\n", uuid, virNWFilterGetName(nwfilter)); - virNWFilterFree(nwfilter); - VIR_FREE(names[i]); } - VIR_FREE(names); + vshNWFilterListFree(list); return true; } -- 1.7.7.3

On 09/05/12 08:02, Osier Yang wrote:
tools/virsh-nwfilter.c: * vshNWFilterSorter to sort network filters by name
* vshNWFilterListFree to free the network filter objects list.
* vshNWFilterListCollect to collect the network filter objects, trying to use new API first, fall back to older APIs if it's not supported. --- tools/virsh-nwfilter.c | 163 +++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 134 insertions(+), 29 deletions(-)
diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index 5169d38..57cf2b7 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -190,6 +190,134 @@ cmdNWFilterDumpXML(vshControl *ctl, const vshCmd *cmd) return ret; }
+static int +vshNWFilterSorter(const void *a, const void *b) +{ + virNWFilterPtr *fa = (virNWFilterPtr *) a; + virNWFilterPtr *fb = (virNWFilterPtr *) b; + + if (*fa && !*fb) + return -1; + + if (!*fa) + return *fb != NULL; + + return vshStrcasecmp(virNWFilterGetName(*fa), + virNWFilterGetName(*fb));
Bad indentation.
+} + +struct vshNWFilterList { + virNWFilterPtr *filters; + size_t nfilters; +}; +typedef struct vshNWFilterList *vshNWFilterListPtr; + +static void +vshNWFilterListFree(vshNWFilterListPtr list) +{ + int i; + + if (list && list->nfilters) { + for (i = 0; i < list->nfilters; i++) { + if (list->filters[i]) + virNWFilterFree(list->filters[i]); + } + VIR_FREE(list->filters); + } + VIR_FREE(list); +} + +static vshNWFilterListPtr +vshNWFilterListCollect(vshControl *ctl, + unsigned int flags) +{ + vshNWFilterListPtr list = vshMalloc(ctl, sizeof(*list)); + int i; + int ret; + virNWFilterPtr filter; + bool success = false; + size_t deleted = 0; + int nfilters = 0; + char **names = NULL; + + /* try the list with flags support (0.10.2 and later) */ + if ((ret = virConnectListAllNWFilters(ctl->conn, + &list->filters, + flags)) >= 0) { + list->nfilters = ret; + goto finished; + } + + /* check if the command is actually supported */ + if (last_error && last_error->code == VIR_ERR_NO_SUPPORT) { + vshResetLibvirtError(); + goto fallback; + } + + /* there was an error during the call */ + vshError(ctl, "%s", _("Failed to list node filters")); + goto cleanup; + + +fallback: + /* fall back to old method (0.9.13 and older) */ + vshResetLibvirtError(); + + nfilters = virConnectNumOfNWFilters(ctl->conn); + if (nfilters < 0) { + vshError(ctl, "%s", _("Failed to count network filters")); + goto cleanup; + } + + if (nfilters == 0) + return list; + + names = vshMalloc(ctl, sizeof(char *) * nfilters); + + nfilters = virConnectListNWFilters(ctl->conn, names, nfilters); + if (nfilters < 0) { + vshError(ctl, "%s", _("Failed to list network filters")); + goto cleanup; + } + + list->filters = vshMalloc(ctl, sizeof(virNWFilterPtr) * (nfilters));
Brackets are not necessary around nfilters.
+ list->nfilters = 0; + + /* get the network filters */ + for (i = 0; i < nfilters ; i++) { + if (!(filter = virNWFilterLookupByName(ctl->conn, names[i]))) + continue; + list->filters[list->nfilters++] = filter; + } + + /* truncate network filters that weren't found */ + deleted = nfilters - list->nfilters; + +finished: + /* sort the list */ + if (list->filters && list->nfilters) + qsort(list->filters, list->nfilters, + sizeof(*list->filters), vshNWFilterSorter); + + /* truncate the list for not found filter objects */ + if (deleted) + VIR_SHRINK_N(list->filters, list->nfilters, deleted); + + success = true; + +cleanup: + for (i = 0; i < nfilters; i++) + VIR_FREE(names[i]); + VIR_FREE(names); + + if (!success) { + vshNWFilterListFree(list); + list = NULL; + } + + return list; +} + /* * "nwfilter-list" command */
Otherwise looks OK. Peter

On 2012年09月06日 22:41, Peter Krempa wrote:
On 09/05/12 08:02, Osier Yang wrote:
tools/virsh-nwfilter.c: * vshNWFilterSorter to sort network filters by name
* vshNWFilterListFree to free the network filter objects list.
* vshNWFilterListCollect to collect the network filter objects, trying to use new API first, fall back to older APIs if it's not supported. --- tools/virsh-nwfilter.c | 163 +++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 134 insertions(+), 29 deletions(-)
diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index 5169d38..57cf2b7 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -190,6 +190,134 @@ cmdNWFilterDumpXML(vshControl *ctl, const vshCmd *cmd) return ret; }
+static int +vshNWFilterSorter(const void *a, const void *b) +{ + virNWFilterPtr *fa = (virNWFilterPtr *) a; + virNWFilterPtr *fb = (virNWFilterPtr *) b; + + if (*fa && !*fb) + return -1; + + if (!*fa) + return *fb != NULL; + + return vshStrcasecmp(virNWFilterGetName(*fa), + virNWFilterGetName(*fb));
Bad indentation.
+} + +struct vshNWFilterList { + virNWFilterPtr *filters; + size_t nfilters; +}; +typedef struct vshNWFilterList *vshNWFilterListPtr; + +static void +vshNWFilterListFree(vshNWFilterListPtr list) +{ + int i; + + if (list && list->nfilters) { + for (i = 0; i < list->nfilters; i++) { + if (list->filters[i]) + virNWFilterFree(list->filters[i]); + } + VIR_FREE(list->filters); + } + VIR_FREE(list); +} + +static vshNWFilterListPtr +vshNWFilterListCollect(vshControl *ctl, + unsigned int flags) +{ + vshNWFilterListPtr list = vshMalloc(ctl, sizeof(*list)); + int i; + int ret; + virNWFilterPtr filter; + bool success = false; + size_t deleted = 0; + int nfilters = 0; + char **names = NULL; + + /* try the list with flags support (0.10.2 and later) */ + if ((ret = virConnectListAllNWFilters(ctl->conn, + &list->filters, + flags)) >= 0) { + list->nfilters = ret; + goto finished; + } + + /* check if the command is actually supported */ + if (last_error && last_error->code == VIR_ERR_NO_SUPPORT) { + vshResetLibvirtError(); + goto fallback; + } + + /* there was an error during the call */ + vshError(ctl, "%s", _("Failed to list node filters")); + goto cleanup; + + +fallback: + /* fall back to old method (0.9.13 and older) */ + vshResetLibvirtError(); + + nfilters = virConnectNumOfNWFilters(ctl->conn); + if (nfilters < 0) { + vshError(ctl, "%s", _("Failed to count network filters")); + goto cleanup; + } + + if (nfilters == 0) + return list; + + names = vshMalloc(ctl, sizeof(char *) * nfilters); + + nfilters = virConnectListNWFilters(ctl->conn, names, nfilters); + if (nfilters < 0) { + vshError(ctl, "%s", _("Failed to list network filters")); + goto cleanup; + } + + list->filters = vshMalloc(ctl, sizeof(virNWFilterPtr) * (nfilters));
Brackets are not necessary around nfilters.
+ list->nfilters = 0; + + /* get the network filters */ + for (i = 0; i < nfilters ; i++) { + if (!(filter = virNWFilterLookupByName(ctl->conn, names[i]))) + continue; + list->filters[list->nfilters++] = filter; + } + + /* truncate network filters that weren't found */ + deleted = nfilters - list->nfilters; + +finished: + /* sort the list */ + if (list->filters && list->nfilters) + qsort(list->filters, list->nfilters, + sizeof(*list->filters), vshNWFilterSorter); + + /* truncate the list for not found filter objects */ + if (deleted) + VIR_SHRINK_N(list->filters, list->nfilters, deleted); + + success = true; + +cleanup: + for (i = 0; i < nfilters; i++) + VIR_FREE(names[i]); + VIR_FREE(names); + + if (!success) { + vshNWFilterListFree(list); + list = NULL; + } + + return list; +} + /* * "nwfilter-list" command */
Otherwise looks OK.
Pushed with the nits fixed. Regards, Osier

On 09/05/12 08:02, Osier Yang wrote:
v3 - v4: * Just rebase on the top, and split the patches from v3's large set.
Osier Yang (5): list: Define new API virConnectListAllNWFilters list: Implement RPC calls for virConnectListAllNWFilters list: Implement listAllNWFilters list: Expose virConnectListAllNWFilters to Python binding list: Use virConnectListAllNWFilters in virsh
This series looks ok in overall so ACK if nobody else has any comments. (For example any filter flags. I just checked the code.) Peter
participants (3)
-
Eric Blake
-
Osier Yang
-
Peter Krempa