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@xxxxxxxxxx>
> > ---
> > 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
