Re: [Qemu-devel] [Qemu-block] [PATCH] block/nfs: add support for setting debug level

[Peter Lieven]

Am 26.10.2015 um 11:45 schrieb Stefan Hajnoczi:
> On Thu, Oct 22, 2015 at 08:37:19AM +0200, Peter Lieven wrote:
> > Am 22.09.2015 um 08:13 schrieb Peter Lieven:
> > > Am 25.06.2015 um 15:18 schrieb Stefan Hajnoczi:
> > > > On Tue, Jun 23, 2015 at 10:12:15AM +0200, Peter Lieven wrote:
> > > > > upcoming libnfs versions will support logging debug messages. Add
> > > > > support for it in qemu through an URL parameter.
> > > > >
> > > > > Signed-off-by: Peter Lieven <address@hidden>
> > > > > ---
> > > > >   block/nfs.c | 4 ++++
> > > > >   1 file changed, 4 insertions(+)
> > > > >
> > > > > diff --git a/block/nfs.c b/block/nfs.c
> > > > > index ca9e24e..f7388a3 100644
> > > > > --- a/block/nfs.c
> > > > > +++ b/block/nfs.c
> > > > > @@ -329,6 +329,10 @@ static int64_t nfs_client_open(NFSClient *client, const 
> > > > > char *filename,
> > > > >           } else if (!strcmp(qp->p[i].name, "readahead")) {
> > > > >               nfs_set_readahead(client->context, val);
> > > > >   #endif
> > > > > +#ifdef LIBNFS_FEATURE_DEBUG
> > > > > +        } else if (!strcmp(qp->p[i].name, "debug")) {
> > > > > +            nfs_set_debug(client->context, val);
> > > > > +#endif
> > > > >           } else {
> > > > >               error_setg(errp, "Unknown NFS parameter name: %s",
> > > > >                          qp->p[i].name);
> > > > Untrusted users may be able to set these options since they are encoded
> > > > in the URI.  I'm imagining a hosting or cloud scenario like OpenStack.
> > > >
> > > > A verbose debug level spams stderr and could consume a lot of disk
> > > > space.
> > > >
> > > > (The uid and gid options are probably okay since the NFS server cannot
> > > > trust the uid/gid coming from QEMU anyway.)
> > > >
> > > > I think we can merge this patch for QEMU 2.4 but I'd like to have a
> > > > discussion about the security risk of encoding libnfs options in the
> > > > URI.
> > > >
> > > > CCed Eric Blake in case libvirt is affected.
> > > >
> > > > Has anyone thought about this and what are the rules?
> > > As I hadn't time to work further on the best way to add options for NFS (and 
> > > other
> > > protocols), would it be feasible to allow passing debug as an URL parameter, but
> > > limit the maximum debug level to limit a possible security impact (flooding 
> > > logs)?
> > >
> > > If a higher debug level is needed it can be set via device specific options as 
> > > soon
> > > there is a common scheme for them.
> > Any objections?
> If you are sure that ERROR and WARN levels (or similar) don't flood the
> logs, then it sounds like a solution.

Thats not the case. I use debug level 2 for quite some time. Mainly to see NFS 
connection interruptions.

So I would be happy if we could allow for debug <= 2 from the cmdline.

Peter