Linux NFS development
 help / color / mirror / Atom feed
From: Steve Dickson <SteveD@redhat.com>
To: "David Härdeman" <david@hardeman.nu>, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 00/19] gssd improvements
Date: Tue, 09 Dec 2014 11:39:59 -0500	[thread overview]
Message-ID: <548725DF.9000202@RedHat.com> (raw)
In-Reply-To: <20141209053828.24756.89941.stgit@zeus.muc.hardeman.nu>

Hello,

On 12/09/2014 12:40 AM, David Härdeman wrote:
> The following series converts gssd to use libevent and inotify instead
> of a handrolled event loop and dnotify. Lots of cleanups in the process
> (e.g. removing a lot of arbitrary limitations and fixed size buffers).
> 
> All in all a nice reduction in code size (what can I say, I was bored).
I just have to asked... Does this patch set solve a problem? Fix a Bug?
I know you said you were bored :-) but what was your motivation?

The reason I ask is this patch set just scream out to me were "fixing 
something that is not broken". Plus rewrites like this eliminate years 
of testing and stability, so we can't take it lightly. gssd is now
an important part of all nfs client mounts... 

That said, I did read through the set and there is definitely some
good/needed cleanup as well some superfluous changes which is fine..
Its obvious you do have a clue and you spent some time on them.. 

So by no means I am against these patches. I guess I need a reason 
to apply them... ;-) What do they fix? Are these patches leading use 
down to a better place? Is there a noticeable performance gain?  

Finally, why the "change dnotify to inotify" a good thing? 

> 
> I've even managed to mount NFS shares with the patched server :)
Was that mount at least a secure mount? ;-)  Seriously was that all
the testing that done?

Again, thank you for your time!

steved.
> 
> ---
> 
> David Härdeman (19):
>       nfs-utils: cleanup daemonization code
>       nfs-utils: gssd - merge gssd_main_loop.c and gssd.c
>       nfs-utils: gssd - simplify some option handling
>       nfs-utils: gssd - remove arbitrary GSSD_MAX_CCACHE_SEARCH limitation
>       nfs-utils: gssd - simplify topdirs path
>       nfs-utils: gssd - move over pipfs scanning code
>       nfs-utils: gssd - simplify client dir scanning code
>       nfs-utils: gssd - use libevent
>       nfs-utils: gssd - remove "close me" code
>       nfs-utils: gssd - make the client lists per-topdir
>       nfs-utils: gssd - keep the rpc_pipefs dir open
>       nfs-utils: gssd - use more relative paths
>       nfs-utils: gssd - simplify topdir scanning
>       nfs-utils: gssd - simplify client scanning
>       nfs-utils: gssd - cleanup read_service_info
>       nfs-utils: gssd - change dnotify to inotify
>       nfs-utils: gssd - further shorten some pathnames
>       nfs-utils: gssd - improve inotify
>       nfs-utils: gssd - simplify handle_gssd_upcall
> 
> 
>  support/include/nfslib.h    |    5 
>  support/nfs/mydaemon.c      |   92 +++--
>  utils/gssd/Makefile.am      |   24 +
>  utils/gssd/gss_util.h       |    2 
>  utils/gssd/gssd.c           |  785 +++++++++++++++++++++++++++++++++++++++++--
>  utils/gssd/gssd.h           |   46 +--
>  utils/gssd/gssd_main_loop.c |  263 --------------
>  utils/gssd/gssd_proc.c      |  654 ++----------------------------------
>  utils/gssd/svcgssd.c        |    8 
>  utils/idmapd/idmapd.c       |    6 
>  utils/statd/statd.c         |   66 +---
>  11 files changed, 878 insertions(+), 1073 deletions(-)
>  delete mode 100644 utils/gssd/gssd_main_loop.c
> 
> --
> David Härdeman
> 

  parent reply	other threads:[~2014-12-09 16:40 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-09  5:40 [PATCH 00/19] gssd improvements David Härdeman
2014-12-09  5:40 ` [PATCH 01/19] nfs-utils: cleanup daemonization code David Härdeman
2014-12-09  5:40 ` [PATCH 02/19] nfs-utils: gssd - merge gssd_main_loop.c and gssd.c David Härdeman
2014-12-09  5:40 ` [PATCH 03/19] nfs-utils: gssd - simplify some option handling David Härdeman
2014-12-09  5:41 ` [PATCH 04/19] nfs-utils: gssd - remove arbitrary GSSD_MAX_CCACHE_SEARCH limitation David Härdeman
2014-12-09  5:41 ` [PATCH 05/19] nfs-utils: gssd - simplify topdirs path David Härdeman
2014-12-09  5:41 ` [PATCH 06/19] nfs-utils: gssd - move over pipfs scanning code David Härdeman
2014-12-09  5:41 ` [PATCH 07/19] nfs-utils: gssd - simplify client dir " David Härdeman
2014-12-09  5:41 ` [PATCH 08/19] nfs-utils: gssd - use libevent David Härdeman
2014-12-09  5:41 ` [PATCH 09/19] nfs-utils: gssd - remove "close me" code David Härdeman
2014-12-09  5:41 ` [PATCH 10/19] nfs-utils: gssd - make the client lists per-topdir David Härdeman
2014-12-09  5:41 ` [PATCH 11/19] nfs-utils: gssd - keep the rpc_pipefs dir open David Härdeman
2014-12-09  5:41 ` [PATCH 12/19] nfs-utils: gssd - use more relative paths David Härdeman
2014-12-09  5:41 ` [PATCH 13/19] nfs-utils: gssd - simplify topdir scanning David Härdeman
2014-12-09  5:41 ` [PATCH 14/19] nfs-utils: gssd - simplify client scanning David Härdeman
2014-12-09  5:41 ` [PATCH 15/19] nfs-utils: gssd - cleanup read_service_info David Härdeman
2014-12-09  5:42 ` [PATCH 16/19] nfs-utils: gssd - change dnotify to inotify David Härdeman
2014-12-09  5:42 ` [PATCH 17/19] nfs-utils: gssd - further shorten some pathnames David Härdeman
2014-12-09  5:42 ` [PATCH 18/19] nfs-utils: gssd - improve inotify David Härdeman
2014-12-09  5:42 ` [PATCH 19/19] nfs-utils: gssd - simplify handle_gssd_upcall David Härdeman
2014-12-09 13:09 ` [PATCH 00/19] gssd improvements Jeff Layton
2014-12-09 13:52   ` David Härdeman
2014-12-09 14:58     ` Jeff Layton
2014-12-09 15:07       ` Simo Sorce
2014-12-09 19:55       ` David Härdeman
2014-12-10 11:52         ` Jeff Layton
2014-12-10 14:08           ` David Härdeman
2014-12-10 14:17             ` Jeff Layton
2014-12-10 14:31               ` David Härdeman
2014-12-10 14:34                 ` Jeff Layton
2014-12-10 16:03                   ` David Howells
2014-12-10 19:03                     ` Jeff Layton
2014-12-10 20:55                       ` David Härdeman
2014-12-10 23:44                       ` Ian Kent
2014-12-10 23:21                     ` Benjamin Coddington
2014-12-11  0:12                       ` Ian Kent
2014-12-11  1:54                         ` Benjamin Coddington
2014-12-11  3:21                           ` Ian Kent
2014-12-11 11:45                             ` Jeff Layton
2014-12-11 12:55                               ` Ian Kent
2014-12-11 13:46                                 ` Jeff Layton
2014-12-11 22:31                                   ` Ian Kent
2014-12-11 19:32                               ` J. Bruce Fields
2014-12-11 19:50                                 ` Jeff Layton
2014-12-11 19:55                                   ` J. Bruce Fields
2014-12-11 20:11                                     ` Jeff Layton
2014-12-11 20:38                                       ` J. Bruce Fields
2014-12-11 22:20                                         ` Ian Kent
2014-12-09 16:39 ` Steve Dickson [this message]
2014-12-09 20:22   ` David Härdeman
2014-12-09 21:13     ` Steve Dickson
2014-12-10 14:20       ` David Härdeman
2014-12-10 20:35 ` J. Bruce Fields
2014-12-10 20:49   ` David Härdeman
2014-12-10 21:07     ` J. Bruce Fields
2015-01-28 21:29 ` Steve Dickson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=548725DF.9000202@RedHat.com \
    --to=steved@redhat.com \
    --cc=david@hardeman.nu \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox