Re: [Nbd] [PATCH 2/2] nbd-server: Make command-line -r apply to all exports

[Alex Bligh <alex@...872...>]

> On 16 Dec 2016, at 02:09, Eric Blake <eblake@...696...> wrote:
> 
> Now that we prefer the config file for everything, many command
> line options don't make much sense any more.  But one common
> tweak is a decision on whether to fire up nbd-server with all
> exports as read-only.  Repurpose the -r command line option to
> imply that all exports are now readonly, rather than just the
> ones mentioned as readonly in the config, which makes it easier
> to test a server setup without the worry of accidental writes
> while also avoiding the need to tweak the config file just to
> turn lots of readonly markers on and then back off again.

I think this is a good idea (as is the 1/1 minor doc patch).

> +++ b/man/nbd-server.1.in.sgml
> @@ -159,9 +159,10 @@ manpage.1: manpage.sgml
>       <varlistentry>
> 	<term><option>-r</option></term>
> 	<listitem>
> -	  <para>Export the file read-only. If a client tries to write
> +	  <para>Export all files as read-only. If a client tries to write

Perhaps "make all exports read only"?

> 	    to a read-only exported file, it will receive an error, but
> -	    the connection will stay up.</para>
> +	    the connection will stay up. This option overrides the
> +	    configuration file per-export readonly settings.</para>
> 	</listitem>
>       </varlistentry>
>       <varlistentry>
> diff --git a/man/nbd-server.5.in.sgml b/man/nbd-server.5.in.sgml
> index e838bcf..03ded44 100644
> --- a/man/nbd-server.5.in.sgml
> +++ b/man/nbd-server.5.in.sgml
> @@ -684,7 +684,7 @@ manpage.1: manpage.sgml
> 	    Use of this option in conjunction with
> 	    <option>copyonwrite</option> is possible, but silly.
> 	  </para>
> -	  <para>Corresponds to the <option>-r</option> option on the
> +	  <para>Implied if the <option>-r</option> option is used on the
> 	  command line.</para>
> 	</listitem>
>       </varlistentry>
> diff --git a/nbd-server.c b/nbd-server.c
> index 698e86e..cb341f6 100644
> --- a/nbd-server.c
> +++ b/nbd-server.c
> @@ -164,6 +164,7 @@ int dontfork = 0;
> #define F_OLDSTYLE 1	  /**< Allow oldstyle (port-based) exports */
> #define F_LIST 2	  /**< Allow clients to list the exports on a server */
> #define F_NO_ZEROES 4	  /**< Do not send zeros to client */
> +#define F_ALL_READONLY 8  /**< Force all exports to be readonly */
> // also accepts F_FORCEDTLS (which is 16384)
> GHashTable *children;
> char pidfname[256]; /**< name of our PID file */
> @@ -440,7 +441,7 @@ static inline void spliceit(int fd_in, loff_t *off_in, int fd_out,
> void usage() {
> 	printf("This is nbd-server version " VERSION "\n");
> 	printf("Usage: [[ip:|ip6@]port file_to_export [size][kKmM]] [-l authorize_file] [-r] [-m] [-c] [-C configuration file] [-p PID file name] [-o section name] [-M max connections] [-d] [-V]\n"
> -	       "\t-r|--read-only\t\tread only\n"
> +	       "\t-r|--read-only\t\tmake all exports be read only\n"

Again "make all exports read only" (i.e. drop 'be')?

> 	       "\t-m|--multi-file\t\tmultiple file\n"
> 	       "\t-c|--copy-on-write\tcopy on write\n"
> 	       "\t-C|--config-file\tspecify an alternate configuration file\n"
> @@ -577,6 +578,7 @@ SERVER* cmdline(int argc, char *argv[], struct generic_conf *genconf) {
> 			}
> 			break;
> 		case 'r':
> +			genconf->flags |= F_ALL_READONLY;
> 			serve->flags |= F_READONLY;
> 			break;
> 		case 'm':
> @@ -945,6 +947,9 @@ GArray* parse_cfile(gchar* f, struct generic_conf *const genconf, bool expect_ge
> 		/* Don't append values for the [generic] group */
> 		if(i>0 || !expect_generic) {
> 			s.servename = groups[i];
> +			if (genconftmp.flags & F_ALL_READONLY) {
> +				s.flags |= F_READONLY;
> +			}
> 
> 			g_array_append_val(retval, s);
> 		}
> -- 
> 2.9.3
> 
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most 
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> Nbd-general mailing list
> Nbd-general@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nbd-general
> 

-- 
Alex Bligh