On Fri, Dec 09, 2016 at 06:59:57AM -0500, John Ferlan wrote:
On 11/25/2016 08:12 AM, Erik Skultety wrote:
Enable libvirt users to query logging filter settings.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- daemon/admin.c | 47 +++++++++++++++++++++++++++++++++++++++++ include/libvirt/libvirt-admin.h | 4 ++++ src/admin/admin_protocol.x | 16 +++++++++++++- src/admin/admin_remote.c | 35 ++++++++++++++++++++++++++++++ src/admin_protocol-structs | 8 +++++++ src/libvirt-admin.c | 41 +++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 2 ++ src/libvirt_admin_public.syms | 1 + 8 files changed, 153 insertions(+), 1 deletion(-)
diff --git a/daemon/admin.c b/daemon/admin.c index 4fa3851..f3bc1de 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -399,6 +399,22 @@ adminConnectGetLoggingOutputs(char **outputs, unsigned int flags) return virLogGetNbOutputs(); }
+/* Returns the number of defined filters or -1 in case of an error */ +static int +adminConnectGetLoggingFilters(char **filters, unsigned int flags) +{ + char *tmp = NULL; + int ret = 0; + + virCheckFlags(0, -1); + + if ((ret = virLogGetNbFilters()) > 0 && !(tmp = virLogGetFilters())) + return -1; + + *filters = tmp; + return ret; +} + static int adminDispatchConnectGetLoggingOutputs(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, @@ -420,4 +436,35 @@ adminDispatchConnectGetLoggingOutputs(virNetServerPtr server ATTRIBUTE_UNUSED,
return 0; } + +static int +adminDispatchConnectGetLoggingFilters(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + admin_connect_get_logging_filters_args *args, + admin_connect_get_logging_filters_ret *ret) +{ + char *filters = NULL; + int nfilters = 0; + + if ((nfilters = adminConnectGetLoggingFilters(&filters, args->flags)) < 0) { + virNetMessageSaveError(rerr); + return -1; + } + + if (nfilters == 0) { + ret->filters = NULL; + } else { + char **ret_filters = NULL; + if (VIR_ALLOC(ret_filters) < 0) + return -1; + + *ret_filters = filters; + ret->filters = ret_filters;
So while this works - why the extra effort to allocate an array of 1 item to stuff the returned filter into it?
While I understand outputs has a default and thus doesn't return NULL - the adminConnectGetLoggingFilters will return :
-1 on failure 0 w/ NULL filters # w/ non-NULL filters
So I believe you could still just VIR_STEAL_PTR(ret->filters, filters) as it doesn't distinguish if 'b' is NULL or not - it just assigns it.
I almost forgot to reply to this patch. Unfortunately, VIR_STEAL_PTR is not enough and that's because of the way XDR stores pointers - string that actually can be NULL is defined as char **, so you have to allocate the 1 member array because what virNetServerProgramDispatchCall will then do is call xdr_free() which will try to free both ret->filters and *ret->filters - it's just how XDR defines it. I remember wanting to do it as you suggest (the intuitive way) but then the daemon either crashed or I got an RPC error so I looked at the XDR spec and found out that to be able to pass both NULL and string data in the same data type, double pointer is needed. Erik