diff mbox series

[v2,1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers

Message ID 9c046648bfd9c8260ec7bd37e0a93f7821e0842f.1644515977.git.bcodding@redhat.com (mailing list archive)
State New, archived
Headers show
Series nfsuuid and udev examples | expand

Commit Message

Benjamin Coddington Feb. 10, 2022, 6:01 p.m. UTC
The nfsuuid program will either create a new UUID from a random source or
derive it from /etc/machine-id, else it returns a UUID that has already
been written to /etc/nfsuuid or the file specified.  This small,
lightweight tool is suitable for execution by systemd-udev in rules to
populate the nfs4 client uniquifier.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 .gitignore                |   1 +
 aclocal/libuuid.m4        |  17 ++++
 configure.ac              |   4 +
 tools/Makefile.am         |   1 +
 tools/nfsuuid/Makefile.am |   8 ++
 tools/nfsuuid/nfsuuid.c   | 203 ++++++++++++++++++++++++++++++++++++++
 tools/nfsuuid/nfsuuid.man |  33 +++++++
 7 files changed, 267 insertions(+)
 create mode 100644 aclocal/libuuid.m4
 create mode 100644 tools/nfsuuid/Makefile.am
 create mode 100644 tools/nfsuuid/nfsuuid.c
 create mode 100644 tools/nfsuuid/nfsuuid.man

Comments

Chuck Lever Feb. 10, 2022, 6:15 p.m. UTC | #1
> On Feb 10, 2022, at 1:01 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
> 
> The nfsuuid program will either create a new UUID from a random source or
> derive it from /etc/machine-id, else it returns a UUID that has already
> been written to /etc/nfsuuid or the file specified.  This small,
> lightweight tool is suitable for execution by systemd-udev in rules to
> populate the nfs4 client uniquifier.

I like everything but the name. Without context, even if
I read NFS protocol specifications, I have no idea what
"nfsuuid" does.

Possible alternatives:

nfshostuniquifier
nfsuniquehost
nfshostuuid


> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> ---
> .gitignore                |   1 +
> aclocal/libuuid.m4        |  17 ++++
> configure.ac              |   4 +
> tools/Makefile.am         |   1 +
> tools/nfsuuid/Makefile.am |   8 ++
> tools/nfsuuid/nfsuuid.c   | 203 ++++++++++++++++++++++++++++++++++++++
> tools/nfsuuid/nfsuuid.man |  33 +++++++
> 7 files changed, 267 insertions(+)
> create mode 100644 aclocal/libuuid.m4
> create mode 100644 tools/nfsuuid/Makefile.am
> create mode 100644 tools/nfsuuid/nfsuuid.c
> create mode 100644 tools/nfsuuid/nfsuuid.man
> 
> diff --git a/.gitignore b/.gitignore
> index c89d1cd2583d..4d63ee93b2dc 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -61,6 +61,7 @@ utils/statd/statd
> tools/locktest/testlk
> tools/getiversion/getiversion
> tools/nfsconf/nfsconf
> +tools/nfsuuid/nfsuuid
> support/export/mount.h
> support/export/mount_clnt.c
> support/export/mount_xdr.c
> diff --git a/aclocal/libuuid.m4 b/aclocal/libuuid.m4
> new file mode 100644
> index 000000000000..f64085010d1d
> --- /dev/null
> +++ b/aclocal/libuuid.m4
> @@ -0,0 +1,17 @@
> +AC_DEFUN([AC_LIBUUID], [
> +        LIBUUID=
> +
> +        AC_CHECK_LIB([uuid], [uuid_generate_random], [], [AC_MSG_FAILURE(
> +                [Missing libuuid uuid_generate_random, needed to build nfs4id])])
> +
> +        AC_CHECK_LIB([uuid], [uuid_generate_sha1], [], [AC_MSG_FAILURE(
> +                [Missing libuuid uuid_generate_sha1, needed to build nfs4id])])
> +
> +        AC_CHECK_LIB([uuid], [uuid_unparse], [], [AC_MSG_FAILURE(
> +                [Missing libuuid uuid_unparse, needed to build nfs4id])])
> +
> +        AC_CHECK_HEADER([uuid/uuid.h], [], [AC_MSG_FAILURE(
> +                [Missing uuid/uuid.h, needed to build nfs4id])])
> +
> +        AC_SUBST([LIBUUID], ["-luuid"])
> +])
> diff --git a/configure.ac b/configure.ac
> index 50e9b321dcf3..1342c471f142 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -355,6 +355,9 @@ if test "$enable_nfsv4" = yes; then
>   dnl check for the keyutils libraries and headers
>   AC_KEYUTILS
> 
> +  dnl check for the libuuid library and headers
> +  AC_LIBUUID
> +
>   dnl Check for sqlite3
>   AC_SQLITE3_VERS
> 
> @@ -740,6 +743,7 @@ AC_CONFIG_FILES([
> 	tools/nfsdclnts/Makefile
> 	tools/nfsconf/Makefile
> 	tools/nfsdclddb/Makefile
> +	tools/nfsuuid/Makefile
> 	utils/Makefile
> 	utils/blkmapd/Makefile
> 	utils/nfsdcld/Makefile
> diff --git a/tools/Makefile.am b/tools/Makefile.am
> index 9b4b0803db39..a12b0f34f4e7 100644
> --- a/tools/Makefile.am
> +++ b/tools/Makefile.am
> @@ -7,6 +7,7 @@ OPTDIRS += rpcgen
> endif
> 
> OPTDIRS += nfsconf
> +OPTDIRS += nfsuuid
> 
> if CONFIG_NFSDCLD
> OPTDIRS += nfsdclddb
> diff --git a/tools/nfsuuid/Makefile.am b/tools/nfsuuid/Makefile.am
> new file mode 100644
> index 000000000000..7b3a54c30d50
> --- /dev/null
> +++ b/tools/nfsuuid/Makefile.am
> @@ -0,0 +1,8 @@
> +## Process this file with automake to produce Makefile.in
> +
> +man8_MANS	= nfsuuid.man
> +
> +bin_PROGRAMS = nfsuuid
> +
> +nfsuuid_SOURCES = nfsuuid.c
> +nfsuuid_LDADD = $(LIBUUID)
> diff --git a/tools/nfsuuid/nfsuuid.c b/tools/nfsuuid/nfsuuid.c
> new file mode 100644
> index 000000000000..bbdec59f1afe
> --- /dev/null
> +++ b/tools/nfsuuid/nfsuuid.c
> @@ -0,0 +1,203 @@
> +/*
> + * nfsuuid.c -- create and persist uniquifiers for nfs4 clients
> + *
> + * Copyright (C) 2022  Red Hat, Benjamin Coddington <bcodding@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor,
> + * Boston, MA 02110-1301, USA.
> + */
> +
> +#include <stdio.h>
> +#include <stdarg.h>
> +#include <getopt.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <uuid/uuid.h>
> +
> +#define DEFAULT_ID_FILE "/etc/nfsuuid"
> +
> +UUID_DEFINE(nfs4_clientid_uuid_template,
> +	0xa2, 0x25, 0x68, 0xb2, 0x7a, 0x5f, 0x49, 0x90,
> +	0x8f, 0x98, 0xc5, 0xf0, 0x67, 0x78, 0xcc, 0xf1);
> +
> +static char *prog;
> +static char *source = NULL;
> +static char *id_file = DEFAULT_ID_FILE;
> +static char nfs_unique_id[64];
> +static int replace = 0;
> +
> +static void usage(void)
> +{
> +	fprintf(stderr, "usage: %s [-r|--replace] [-f <file> |--file <file> ] [machine]\n", prog);
> +}
> +
> +static void fatal(const char *fmt, ...)
> +{
> +	int err = errno;
> +	va_list args;
> +	char fatal_msg[256] = "fatal: ";
> +
> +	va_start(args, fmt);
> +	vsnprintf(&fatal_msg[7], 255, fmt, args);
> +	if (err)
> +		fprintf(stderr, "%s: %s\n", fatal_msg, strerror(err));
> +	else
> +		fprintf(stderr, "%s\n", fatal_msg);
> +	exit(-1);
> +}
> +
> +static int read_nfs_unique_id(void)
> +{
> +	int fd;
> +	ssize_t len;
> +
> +	if (replace) {
> +		errno = ENOENT;
> +		return -1;
> +	}
> +
> +	fd = open(id_file, O_RDONLY);
> +	if (fd < 0)
> +		return fd;
> +	len = read(fd, nfs_unique_id, 64);
> +	close(fd);
> +	return len;
> +}
> +
> +static void write_nfs_unique_id(void)
> +{
> +	int fd;
> +
> +	fd = open(id_file, O_RDWR|O_TRUNC|O_CREAT, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH);
> +	if (fd < 0)
> +		fatal("could not write id to %s", id_file);
> +	write(fd, nfs_unique_id, 37);
> +}
> +
> +static void print_nfs_unique_id(void)
> +{
> +	fprintf(stdout, "%s", nfs_unique_id);
> +}
> +
> +static void check_or_make_id(void)
> +{
> +	int ret;
> +	uuid_t uuid;
> +
> +	ret = read_nfs_unique_id();
> +	if (ret < 1) {
> +		if (errno != ENOENT )
> +			fatal("reading file %s", id_file);
> +		uuid_generate_random(uuid);
> +		uuid_unparse(uuid, nfs_unique_id);
> +		nfs_unique_id[36] = '\n';
> +		nfs_unique_id[37] = '\0';
> +		write_nfs_unique_id();
> +	}
> +	print_nfs_unique_id();
> +}
> +
> +static void check_or_make_id_from_machine(void)
> +{
> +	int fd, ret;
> +	char machineid[32];
> +	uuid_t uuid;
> +
> +	ret = read_nfs_unique_id();
> +	if (ret < 1) {
> +		if (errno != ENOENT )
> +			fatal("reading file %s", id_file);
> +
> +		fd = open("/etc/machine-id", O_RDONLY);
> +		if (fd < 0)
> +			fatal("unable to read /etc/machine-id");
> +
> +		read(fd, machineid, 32);
> +		close(fd);
> +
> +		uuid_generate_sha1(uuid, nfs4_clientid_uuid_template, machineid, 32);
> +		uuid_unparse(uuid, nfs_unique_id);
> +		nfs_unique_id[36] = '\n';
> +		nfs_unique_id[37] = '\0';
> +		write_nfs_unique_id();
> +	}
> +	print_nfs_unique_id();
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	prog = argv[0];
> +
> +	while (1) {
> +		int opt, prev_ind;
> +		int option_index = 0;
> +		static struct option long_options[] = {
> +			{"replace",	no_argument,		0, 'r' },
> +			{"file",	required_argument,	0, 'f' },
> +			{ 0, 0, 0, 0 }
> +		};
> +
> +		errno = 0;
> +		prev_ind = optind;
> +		opt = getopt_long(argc, argv, ":rf:", long_options, &option_index);
> +		if (opt == -1)
> +			break;
> +
> +		/* Let's detect missing options in the middle of an option list */
> +		if (optind == prev_ind + 2 && *optarg == '-') {
> +			opt = ':';
> +			--optind;
> +		}
> +
> +		switch (opt) {
> +		case 'r':
> +			replace = 1;
> +			break;
> +		case 'f':
> +			id_file = optarg;
> +			break;
> +		case ':':
> +			usage();
> +			fatal("option \"%s\" requires an argument", argv[prev_ind]);
> +			break;
> +		case '?':
> +			usage();
> +			fatal("unexpected arg \"%s\"", argv[optind - 1]);
> +			break;
> +		}
> +	}
> +
> +	argc -= optind;
> +
> +	if (argc > 1) {
> +		usage();
> +		fatal("Too many arguments");
> +	}
> +
> +	if (argc)
> +		source = argv[optind++];
> +
> +	if (!source)
> +		check_or_make_id();
> +	else if (strcmp(source, "machine") == 0)
> +		check_or_make_id_from_machine();
> +	else {
> +		usage();
> +		fatal("unrecognized source %s\n", source);
> +	}
> +}
> diff --git a/tools/nfsuuid/nfsuuid.man b/tools/nfsuuid/nfsuuid.man
> new file mode 100644
> index 000000000000..856d2f383e0f
> --- /dev/null
> +++ b/tools/nfsuuid/nfsuuid.man
> @@ -0,0 +1,33 @@
> +.\"
> +.\" nfsuuid(8)
> +.\"
> +.TH nfsuuid 8 "10 Feb 2022"
> +.SH NAME
> +nfsuuid \- Generate or return nfs client id uniquifiers
> +.SH SYNOPSIS
> +.B nfsuuid [ -r | --replace ] [ -f | --file <file> ] [<source>]
> +
> +.SH DESCRIPTION
> +The
> +.B nfsuuid
> +command provides a simple utility to help NFS Version 4 clients use unique
> +and persistent client id values.  The command checks for the existence of a
> +file /etc/nfsuuid and returns the first 64 chars read from that file.  If
> +the file is not found, a UUID is generated from the specified source and
> +written to the file and returned.
> +.SH OPTIONS
> +.TP
> +.BR \-r,\ \-\-replace
> +Overwrite the <file> with a UUID generated from <source>.
> +.TP
> +.BR \-f,\ \-\-file
> +Use the specified file to lookup or store any generated UUID.  If <file> is
> +not specified, the default file used is /etc/nfsuuid.
> +.SH Sources
> +If <source> is not specified, nfsuuid will generate a new random UUID.
> +
> +If <source> is "machine", nfsuuid will generate a deterministic UUID value
> +derived from a sha1 hash of the contents of /etc/machine-id and a static
> +key.
> +.SH SEE ALSO
> +.BR machine-id (5)
> -- 
> 2.31.1
> 

--
Chuck Lever
Steve Dickson Feb. 11, 2022, 1:44 p.m. UTC | #2
On 2/10/22 1:15 PM, Chuck Lever III wrote:
> 
> 
>> On Feb 10, 2022, at 1:01 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
>>
>> The nfsuuid program will either create a new UUID from a random source or
>> derive it from /etc/machine-id, else it returns a UUID that has already
>> been written to /etc/nfsuuid or the file specified.  This small,
>> lightweight tool is suitable for execution by systemd-udev in rules to
>> populate the nfs4 client uniquifier.
> 
> I like everything but the name. Without context, even if
> I read NFS protocol specifications, I have no idea what
> "nfsuuid" does.
man nfsuuid :-)
> 
> Possible alternatives:
> 
> nfshostuniquifier
> nfsuniquehost
> nfshostuuid
I'm good with the name... short sweet and easy to type.
The name a command does not have to explain what it does. IMHO.

I think what is more concerning is addressing
Neil's concern as to why we even using udev [1].

steved.

[1] 
https://lore.kernel.org/all/0AB20C82-6200-46E0-A76C-62345DAF8A3A@redhat.com/T/#mdb6af27181ed912ed2220208dea9ca8711937864
> 
> 
>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
>> ---
>> .gitignore                |   1 +
>> aclocal/libuuid.m4        |  17 ++++
>> configure.ac              |   4 +
>> tools/Makefile.am         |   1 +
>> tools/nfsuuid/Makefile.am |   8 ++
>> tools/nfsuuid/nfsuuid.c   | 203 ++++++++++++++++++++++++++++++++++++++
>> tools/nfsuuid/nfsuuid.man |  33 +++++++
>> 7 files changed, 267 insertions(+)
>> create mode 100644 aclocal/libuuid.m4
>> create mode 100644 tools/nfsuuid/Makefile.am
>> create mode 100644 tools/nfsuuid/nfsuuid.c
>> create mode 100644 tools/nfsuuid/nfsuuid.man
>>
>> diff --git a/.gitignore b/.gitignore
>> index c89d1cd2583d..4d63ee93b2dc 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -61,6 +61,7 @@ utils/statd/statd
>> tools/locktest/testlk
>> tools/getiversion/getiversion
>> tools/nfsconf/nfsconf
>> +tools/nfsuuid/nfsuuid
>> support/export/mount.h
>> support/export/mount_clnt.c
>> support/export/mount_xdr.c
>> diff --git a/aclocal/libuuid.m4 b/aclocal/libuuid.m4
>> new file mode 100644
>> index 000000000000..f64085010d1d
>> --- /dev/null
>> +++ b/aclocal/libuuid.m4
>> @@ -0,0 +1,17 @@
>> +AC_DEFUN([AC_LIBUUID], [
>> +        LIBUUID=
>> +
>> +        AC_CHECK_LIB([uuid], [uuid_generate_random], [], [AC_MSG_FAILURE(
>> +                [Missing libuuid uuid_generate_random, needed to build nfs4id])])
>> +
>> +        AC_CHECK_LIB([uuid], [uuid_generate_sha1], [], [AC_MSG_FAILURE(
>> +                [Missing libuuid uuid_generate_sha1, needed to build nfs4id])])
>> +
>> +        AC_CHECK_LIB([uuid], [uuid_unparse], [], [AC_MSG_FAILURE(
>> +                [Missing libuuid uuid_unparse, needed to build nfs4id])])
>> +
>> +        AC_CHECK_HEADER([uuid/uuid.h], [], [AC_MSG_FAILURE(
>> +                [Missing uuid/uuid.h, needed to build nfs4id])])
>> +
>> +        AC_SUBST([LIBUUID], ["-luuid"])
>> +])
>> diff --git a/configure.ac b/configure.ac
>> index 50e9b321dcf3..1342c471f142 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -355,6 +355,9 @@ if test "$enable_nfsv4" = yes; then
>>    dnl check for the keyutils libraries and headers
>>    AC_KEYUTILS
>>
>> +  dnl check for the libuuid library and headers
>> +  AC_LIBUUID
>> +
>>    dnl Check for sqlite3
>>    AC_SQLITE3_VERS
>>
>> @@ -740,6 +743,7 @@ AC_CONFIG_FILES([
>> 	tools/nfsdclnts/Makefile
>> 	tools/nfsconf/Makefile
>> 	tools/nfsdclddb/Makefile
>> +	tools/nfsuuid/Makefile
>> 	utils/Makefile
>> 	utils/blkmapd/Makefile
>> 	utils/nfsdcld/Makefile
>> diff --git a/tools/Makefile.am b/tools/Makefile.am
>> index 9b4b0803db39..a12b0f34f4e7 100644
>> --- a/tools/Makefile.am
>> +++ b/tools/Makefile.am
>> @@ -7,6 +7,7 @@ OPTDIRS += rpcgen
>> endif
>>
>> OPTDIRS += nfsconf
>> +OPTDIRS += nfsuuid
>>
>> if CONFIG_NFSDCLD
>> OPTDIRS += nfsdclddb
>> diff --git a/tools/nfsuuid/Makefile.am b/tools/nfsuuid/Makefile.am
>> new file mode 100644
>> index 000000000000..7b3a54c30d50
>> --- /dev/null
>> +++ b/tools/nfsuuid/Makefile.am
>> @@ -0,0 +1,8 @@
>> +## Process this file with automake to produce Makefile.in
>> +
>> +man8_MANS	= nfsuuid.man
>> +
>> +bin_PROGRAMS = nfsuuid
>> +
>> +nfsuuid_SOURCES = nfsuuid.c
>> +nfsuuid_LDADD = $(LIBUUID)
>> diff --git a/tools/nfsuuid/nfsuuid.c b/tools/nfsuuid/nfsuuid.c
>> new file mode 100644
>> index 000000000000..bbdec59f1afe
>> --- /dev/null
>> +++ b/tools/nfsuuid/nfsuuid.c
>> @@ -0,0 +1,203 @@
>> +/*
>> + * nfsuuid.c -- create and persist uniquifiers for nfs4 clients
>> + *
>> + * Copyright (C) 2022  Red Hat, Benjamin Coddington <bcodding@redhat.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version 2
>> + * of the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor,
>> + * Boston, MA 02110-1301, USA.
>> + */
>> +
>> +#include <stdio.h>
>> +#include <stdarg.h>
>> +#include <getopt.h>
>> +#include <string.h>
>> +#include <errno.h>
>> +#include <stdlib.h>
>> +#include <fcntl.h>
>> +#include <unistd.h>
>> +#include <uuid/uuid.h>
>> +
>> +#define DEFAULT_ID_FILE "/etc/nfsuuid"
>> +
>> +UUID_DEFINE(nfs4_clientid_uuid_template,
>> +	0xa2, 0x25, 0x68, 0xb2, 0x7a, 0x5f, 0x49, 0x90,
>> +	0x8f, 0x98, 0xc5, 0xf0, 0x67, 0x78, 0xcc, 0xf1);
>> +
>> +static char *prog;
>> +static char *source = NULL;
>> +static char *id_file = DEFAULT_ID_FILE;
>> +static char nfs_unique_id[64];
>> +static int replace = 0;
>> +
>> +static void usage(void)
>> +{
>> +	fprintf(stderr, "usage: %s [-r|--replace] [-f <file> |--file <file> ] [machine]\n", prog);
>> +}
>> +
>> +static void fatal(const char *fmt, ...)
>> +{
>> +	int err = errno;
>> +	va_list args;
>> +	char fatal_msg[256] = "fatal: ";
>> +
>> +	va_start(args, fmt);
>> +	vsnprintf(&fatal_msg[7], 255, fmt, args);
>> +	if (err)
>> +		fprintf(stderr, "%s: %s\n", fatal_msg, strerror(err));
>> +	else
>> +		fprintf(stderr, "%s\n", fatal_msg);
>> +	exit(-1);
>> +}
>> +
>> +static int read_nfs_unique_id(void)
>> +{
>> +	int fd;
>> +	ssize_t len;
>> +
>> +	if (replace) {
>> +		errno = ENOENT;
>> +		return -1;
>> +	}
>> +
>> +	fd = open(id_file, O_RDONLY);
>> +	if (fd < 0)
>> +		return fd;
>> +	len = read(fd, nfs_unique_id, 64);
>> +	close(fd);
>> +	return len;
>> +}
>> +
>> +static void write_nfs_unique_id(void)
>> +{
>> +	int fd;
>> +
>> +	fd = open(id_file, O_RDWR|O_TRUNC|O_CREAT, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH);
>> +	if (fd < 0)
>> +		fatal("could not write id to %s", id_file);
>> +	write(fd, nfs_unique_id, 37);
>> +}
>> +
>> +static void print_nfs_unique_id(void)
>> +{
>> +	fprintf(stdout, "%s", nfs_unique_id);
>> +}
>> +
>> +static void check_or_make_id(void)
>> +{
>> +	int ret;
>> +	uuid_t uuid;
>> +
>> +	ret = read_nfs_unique_id();
>> +	if (ret < 1) {
>> +		if (errno != ENOENT )
>> +			fatal("reading file %s", id_file);
>> +		uuid_generate_random(uuid);
>> +		uuid_unparse(uuid, nfs_unique_id);
>> +		nfs_unique_id[36] = '\n';
>> +		nfs_unique_id[37] = '\0';
>> +		write_nfs_unique_id();
>> +	}
>> +	print_nfs_unique_id();
>> +}
>> +
>> +static void check_or_make_id_from_machine(void)
>> +{
>> +	int fd, ret;
>> +	char machineid[32];
>> +	uuid_t uuid;
>> +
>> +	ret = read_nfs_unique_id();
>> +	if (ret < 1) {
>> +		if (errno != ENOENT )
>> +			fatal("reading file %s", id_file);
>> +
>> +		fd = open("/etc/machine-id", O_RDONLY);
>> +		if (fd < 0)
>> +			fatal("unable to read /etc/machine-id");
>> +
>> +		read(fd, machineid, 32);
>> +		close(fd);
>> +
>> +		uuid_generate_sha1(uuid, nfs4_clientid_uuid_template, machineid, 32);
>> +		uuid_unparse(uuid, nfs_unique_id);
>> +		nfs_unique_id[36] = '\n';
>> +		nfs_unique_id[37] = '\0';
>> +		write_nfs_unique_id();
>> +	}
>> +	print_nfs_unique_id();
>> +}
>> +
>> +int main(int argc, char **argv)
>> +{
>> +	prog = argv[0];
>> +
>> +	while (1) {
>> +		int opt, prev_ind;
>> +		int option_index = 0;
>> +		static struct option long_options[] = {
>> +			{"replace",	no_argument,		0, 'r' },
>> +			{"file",	required_argument,	0, 'f' },
>> +			{ 0, 0, 0, 0 }
>> +		};
>> +
>> +		errno = 0;
>> +		prev_ind = optind;
>> +		opt = getopt_long(argc, argv, ":rf:", long_options, &option_index);
>> +		if (opt == -1)
>> +			break;
>> +
>> +		/* Let's detect missing options in the middle of an option list */
>> +		if (optind == prev_ind + 2 && *optarg == '-') {
>> +			opt = ':';
>> +			--optind;
>> +		}
>> +
>> +		switch (opt) {
>> +		case 'r':
>> +			replace = 1;
>> +			break;
>> +		case 'f':
>> +			id_file = optarg;
>> +			break;
>> +		case ':':
>> +			usage();
>> +			fatal("option \"%s\" requires an argument", argv[prev_ind]);
>> +			break;
>> +		case '?':
>> +			usage();
>> +			fatal("unexpected arg \"%s\"", argv[optind - 1]);
>> +			break;
>> +		}
>> +	}
>> +
>> +	argc -= optind;
>> +
>> +	if (argc > 1) {
>> +		usage();
>> +		fatal("Too many arguments");
>> +	}
>> +
>> +	if (argc)
>> +		source = argv[optind++];
>> +
>> +	if (!source)
>> +		check_or_make_id();
>> +	else if (strcmp(source, "machine") == 0)
>> +		check_or_make_id_from_machine();
>> +	else {
>> +		usage();
>> +		fatal("unrecognized source %s\n", source);
>> +	}
>> +}
>> diff --git a/tools/nfsuuid/nfsuuid.man b/tools/nfsuuid/nfsuuid.man
>> new file mode 100644
>> index 000000000000..856d2f383e0f
>> --- /dev/null
>> +++ b/tools/nfsuuid/nfsuuid.man
>> @@ -0,0 +1,33 @@
>> +.\"
>> +.\" nfsuuid(8)
>> +.\"
>> +.TH nfsuuid 8 "10 Feb 2022"
>> +.SH NAME
>> +nfsuuid \- Generate or return nfs client id uniquifiers
>> +.SH SYNOPSIS
>> +.B nfsuuid [ -r | --replace ] [ -f | --file <file> ] [<source>]
>> +
>> +.SH DESCRIPTION
>> +The
>> +.B nfsuuid
>> +command provides a simple utility to help NFS Version 4 clients use unique
>> +and persistent client id values.  The command checks for the existence of a
>> +file /etc/nfsuuid and returns the first 64 chars read from that file.  If
>> +the file is not found, a UUID is generated from the specified source and
>> +written to the file and returned.
>> +.SH OPTIONS
>> +.TP
>> +.BR \-r,\ \-\-replace
>> +Overwrite the <file> with a UUID generated from <source>.
>> +.TP
>> +.BR \-f,\ \-\-file
>> +Use the specified file to lookup or store any generated UUID.  If <file> is
>> +not specified, the default file used is /etc/nfsuuid.
>> +.SH Sources
>> +If <source> is not specified, nfsuuid will generate a new random UUID.
>> +
>> +If <source> is "machine", nfsuuid will generate a deterministic UUID value
>> +derived from a sha1 hash of the contents of /etc/machine-id and a static
>> +key.
>> +.SH SEE ALSO
>> +.BR machine-id (5)
>> -- 
>> 2.31.1
>>
> 
> --
> Chuck Lever
> 
> 
>
Chuck Lever Feb. 11, 2022, 3:48 p.m. UTC | #3
> On Feb 11, 2022, at 8:44 AM, Steve Dickson <SteveD@RedHat.com> wrote:
> 
> On 2/10/22 1:15 PM, Chuck Lever III wrote:
>>> On Feb 10, 2022, at 1:01 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
>>> 
>>> The nfsuuid program will either create a new UUID from a random source or
>>> derive it from /etc/machine-id, else it returns a UUID that has already
>>> been written to /etc/nfsuuid or the file specified.  This small,
>>> lightweight tool is suitable for execution by systemd-udev in rules to
>>> populate the nfs4 client uniquifier.
>> I like everything but the name. Without context, even if
>> I read NFS protocol specifications, I have no idea what
>> "nfsuuid" does.
> man nfsuuid :-)

Any time an administrator has to stop to read documentation
is an unnecessary interruption in their flow of attention.
It wastes their time.


>> Possible alternatives:
>> nfshostuniquifier
>> nfsuniquehost
>> nfshostuuid
> I'm good with the name... short sweet and easy to type.

I'll happily keep it short so it is readable and does not
unnecessarily inflate the length of a line in a bash script.
But almost no-one will ever type this command. Especially
if they don't have to find its man page ;-p

My last serious offers: nfsmachineid nfshostid

I strongly prefer not having uuid in the name. A UUID is
essentially a data type, it does not explain the purpose of
the content.


--
Chuck Lever
Benjamin Coddington Feb. 11, 2022, 6:28 p.m. UTC | #4
On 11 Feb 2022, at 10:48, Chuck Lever III wrote:

>> On Feb 11, 2022, at 8:44 AM, Steve Dickson <SteveD@RedHat.com> wrote:
>>
>> On 2/10/22 1:15 PM, Chuck Lever III wrote:
>>>> On Feb 10, 2022, at 1:01 PM, Benjamin Coddington 
>>>> <bcodding@redhat.com> wrote:
>>>>
>>>> The nfsuuid program will either create a new UUID from a random 
>>>> source or
>>>> derive it from /etc/machine-id, else it returns a UUID that has 
>>>> already
>>>> been written to /etc/nfsuuid or the file specified.  This small,
>>>> lightweight tool is suitable for execution by systemd-udev in rules 
>>>> to
>>>> populate the nfs4 client uniquifier.
>>> I like everything but the name. Without context, even if
>>> I read NFS protocol specifications, I have no idea what
>>> "nfsuuid" does.
>> man nfsuuid :-)
>
> Any time an administrator has to stop to read documentation
> is an unnecessary interruption in their flow of attention.
> It wastes their time.
>
>
>>> Possible alternatives:
>>> nfshostuniquifier
>>> nfsuniquehost
>>> nfshostuuid
>> I'm good with the name... short sweet and easy to type.
>
> I'll happily keep it short so it is readable and does not
> unnecessarily inflate the length of a line in a bash script.
> But almost no-one will ever type this command. Especially
> if they don't have to find its man page ;-p
>
> My last serious offers: nfsmachineid nfshostid
>
> I strongly prefer not having uuid in the name. A UUID is
> essentially a data type, it does not explain the purpose of
> the content.

How do you feel about these suggestions being misleading since the 
output is
not the nfs client's id?  Should we put that information in the man 
page?
Do sysadmins need to know that the output of this command is (if it is 
even
used at all) merely possibility of being a part of the client's id, the
other parts come from other places in the system: the hostname and 
possibly
the ip address?  That's my worry.  If we name it nfsmachineid or 
nfshostid,
do you feel like we ought to have it do the much more complicated job of
accurately outputting the actual client id?

Right now nfsuuid outputs uuids (or whatever was previously stored, up 
to 64
chars).  It generates uuids, like uuidgen.  Without something external 
to
itself (a udev rule, for example), it doesn't have any relationship to 
an
NFS client's id.  It could plausably be used in the future for other 
parts
of NFS to generate persitent uuids.  The only reason we don't just use
`uuidgen` is that we want to wrap it with some persistence.  A would a 
name
like stickyuuid be better?

Ben
Chuck Lever Feb. 11, 2022, 7:04 p.m. UTC | #5
> On Feb 11, 2022, at 1:28 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
> 
> On 11 Feb 2022, at 10:48, Chuck Lever III wrote:
> 
>>> On Feb 11, 2022, at 8:44 AM, Steve Dickson <SteveD@RedHat.com> wrote:
>>> 
>>> On 2/10/22 1:15 PM, Chuck Lever III wrote:
>>>>> On Feb 10, 2022, at 1:01 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
>>>>> 
>>>>> The nfsuuid program will either create a new UUID from a random source or
>>>>> derive it from /etc/machine-id, else it returns a UUID that has already
>>>>> been written to /etc/nfsuuid or the file specified.  This small,
>>>>> lightweight tool is suitable for execution by systemd-udev in rules to
>>>>> populate the nfs4 client uniquifier.
>>>> I like everything but the name. Without context, even if
>>>> I read NFS protocol specifications, I have no idea what
>>>> "nfsuuid" does.
>>> man nfsuuid :-)
>> 
>> Any time an administrator has to stop to read documentation
>> is an unnecessary interruption in their flow of attention.
>> It wastes their time.
>> 
>> 
>>>> Possible alternatives:
>>>> nfshostuniquifier
>>>> nfsuniquehost
>>>> nfshostuuid
>>> I'm good with the name... short sweet and easy to type.
>> 
>> I'll happily keep it short so it is readable and does not
>> unnecessarily inflate the length of a line in a bash script.
>> But almost no-one will ever type this command. Especially
>> if they don't have to find its man page ;-p
>> 
>> My last serious offers: nfsmachineid nfshostid
>> 
>> I strongly prefer not having uuid in the name. A UUID is
>> essentially a data type, it does not explain the purpose of
>> the content.
> 
> How do you feel about these suggestions being misleading since the output is
> not the nfs client's id?  Should we put that information in the man page?
> Do sysadmins need to know that the output of this command is (if it is even
> used at all) merely possibility of being a part of the client's id, the
> other parts come from other places in the system: the hostname and possibly
> the ip address?  That's my worry.

Yes. We're not using the output of the tool for anything
else. At the very least the man page should explain the
proposed client ID usage as an EXAMPLE with an explanation.

But honestly, I haven't seen any use case that requires
this exact functionality. Can you explain why you believe
there needs to be extra generality? (that's been one of
the main reasons I object to "nfsuuid" -- what else can
this tool be used for?)


> If we name it nfsmachineid or nfshostid,
> do you feel like we ought to have it do the much more complicated job of
> accurately outputting the actual client id?

It could do that, but the part that the kernel struggles
with is the uniquifier part. That is the part that _has_
to be done in user space (because that's the part that
requires persistence). The rest of the nfs_client_id4
argument can be formed in the kernel.


> Right now nfsuuid outputs uuids (or whatever was previously stored, up to 64
> chars).

Right. It can output anything, not just a UUID. It will
output whatever is in the special file. If the content
of that file is a random string, what will nfsuuid output?

If someone runs nfsuuid expecting a UUID and gets the
random crap that was previously stored in the persistence
file, wouldn't that be surprising?

Precisely because it has the ability to output whatever
is in the persistence file, and that file does not have
to contain a UUID, that makes this tool not "nfsuuid".


> It generates uuids, like uuidgen.  Without something external to
> itself (a udev rule, for example), it doesn't have any relationship to an
> NFS client's id.  It could plausably be used in the future for other parts
> of NFS to generate persitent uuids.  The only reason we don't just use
> `uuidgen` is that we want to wrap it with some persistence.  A would a name
> like stickyuuid be better?

No: a UUID is a data type. Would you call the tool
nfsunsignedint if we stored the ino of the net namespace
instead?

Do you have any other specific use cases for an nfsuuid
tool today? If you do, then you have a platform for more
generality. If not, then there really isn't any other
purpose for this tool. It is a single-tasker, and we
shouldn't treat it otherwise.


--
Chuck Lever
Benjamin Coddington Feb. 11, 2022, 7:30 p.m. UTC | #6
On 11 Feb 2022, at 14:04, Chuck Lever III wrote:
> Yes. We're not using the output of the tool for anything
> else. At the very least the man page should explain the
> proposed client ID usage as an EXAMPLE with an explanation.
>
> But honestly, I haven't seen any use case that requires
> this exact functionality. Can you explain why you believe
> there needs to be extra generality? (that's been one of
> the main reasons I object to "nfsuuid" -- what else can
> this tool be used for?)

Not yet, no.

>> If we name it nfsmachineid or nfshostid,
>> do you feel like we ought to have it do the much more complicated job 
>> of
>> accurately outputting the actual client id?
>
> It could do that, but the part that the kernel struggles
> with is the uniquifier part. That is the part that _has_
> to be done in user space (because that's the part that
> requires persistence). The rest of the nfs_client_id4
> argument can be formed in the kernel.

Sure we could, but what I was trying to point out is that your names are
just as miserable if you apply your exacting logic to them, and we're 
going
to be hard-pressed to meet the bar unless we name it something 
completely
meaningless.

>> Right now nfsuuid outputs uuids (or whatever was previously stored, 
>> up to 64
>> chars).
>
> Right. It can output anything, not just a UUID. It will
> output whatever is in the special file. If the content
> of that file is a random string, what will nfsuuid output?

64 chars of random string.

> If someone runs nfsuuid expecting a UUID and gets the
> random crap that was previously stored in the persistence
> file, wouldn't that be surprising?

Nothing on stdout surprises me.  After years of sysadmining I'm cold as 
ice.

> Precisely because it has the ability to output whatever
> is in the persistence file, and that file does not have
> to contain a UUID, that makes this tool not "nfsuuid".

Alright.

>> It generates uuids, like uuidgen.  Without something external to
>> itself (a udev rule, for example), it doesn't have any relationship 
>> to an
>> NFS client's id.  It could plausably be used in the future for other 
>> parts
>> of NFS to generate persitent uuids.  The only reason we don't just 
>> use
>> `uuidgen` is that we want to wrap it with some persistence.  A would 
>> a name
>> like stickyuuid be better?
>
> No: a UUID is a data type. Would you call the tool
> nfsunsignedint if we stored the ino of the net namespace
> instead?

I really might, sadly, just to get the job done.

> Do you have any other specific use cases for an nfsuuid
> tool today? If you do, then you have a platform for more
> generality. If not, then there really isn't any other
> purpose for this tool. It is a single-tasker, and we
> shouldn't treat it otherwise.

All the arguments for exacting tolerances on how it should be named 
apply
equally well to anything that implies its output will be used for nfs 
client
ids, or host ids.

So, I've forgotten your other suggestions that don't have anything to do
with NFS or clients or machine IDs.  I'll make more suggestions:

persistychars
mostlyuuids
theuniquifier
randomonce
distroschoice

I like these.  These names are more fun.  :)

In good will,
Ben
Chuck Lever Feb. 11, 2022, 8 p.m. UTC | #7
> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
> 
> All the arguments for exacting tolerances on how it should be named apply
> equally well to anything that implies its output will be used for nfs client
> ids, or host ids.

I completely disagree with this assessment. I object strongly
to the name nfsuuid, and you seem to be inflexible. This is
not a hill I want to die on, however I reserve the right to
say "I told you so" when it turns out to be a poor choice.


--
Chuck Lever
Benjamin Coddington Feb. 11, 2022, 8:16 p.m. UTC | #8
On 11 Feb 2022, at 15:00, Chuck Lever III wrote:

>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington 
>> <bcodding@redhat.com> wrote:
>>
>> All the arguments for exacting tolerances on how it should be named 
>> apply
>> equally well to anything that implies its output will be used for nfs 
>> client
>> ids, or host ids.
>
> I completely disagree with this assessment.

But how, and in what way?  The tool just generates uuids, and spits them
out, or it spits out whatever's in the file you specify, up to 64 chars. 
  If
we can't have uuid in the name, how can we have NFS or machine-id or
host-id?

> I object strongly to the name nfsuuid, and you seem to be inflexible. 
> This
> is not a hill I want to die on, however I reserve the right to say "I 
> told
> you so" when it turns out to be a poor choice.

How does agreeing with you multiple times in my last response and making
further suggestions for names seem inflexible to you?  This is the worst
part of working over email - I think you're misreading my good humor in 
the
face of a drudging discussion as sarcasm or ill will.

Let's find the best name.. I'm still going here.  I do really like
`persistychars`, but I expect that's too fun for everyone.  The name at
least gets closer to your bar of describing exactly what the tool does.

I'm really tired of arguing over the name, and I'm trying my best to 
keep
some good humor about it, and find something that meets everyone's 
needs.

Ben
Chuck Lever Feb. 11, 2022, 8:51 p.m. UTC | #9
> On Feb 11, 2022, at 3:16 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
> 
> On 11 Feb 2022, at 15:00, Chuck Lever III wrote:
> 
>>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
>>> 
>>> All the arguments for exacting tolerances on how it should be named apply
>>> equally well to anything that implies its output will be used for nfs client
>>> ids, or host ids.
>> 
>> I completely disagree with this assessment.
> 
> But how, and in what way?  The tool just generates uuids, and spits them
> out, or it spits out whatever's in the file you specify, up to 64 chars.  If
> we can't have uuid in the name, how can we have NFS or machine-id or
> host-id?

We don't have a tool called "string" to get and set the DNS name of
the local host. It's called "hostname".

The purpose of the proposed tool is to persist a unique string to be
used as part of an NFS client ID. I would like to name the tool based
on that purpose, not based on the way the content of the persistent
file happens to be arranged some of the time.

When the tool generates the string, it just happens to be a UUID. It
doesn't have to be. The tool could generate a digest of the boot time
or the current time. In fact, one of those is usually part of certain
types of a UUID anyway. The fact that it is a UUID is totally not
relevant. We happen to use a UUID because it has certain global
uniqueness properties. (By the way, perhaps the man page could mention
that global uniqueness is important for this identifier. Anything with
similar guaranteed global uniqueness could be used).

You keep admitting that the tool can output something that isn't a
UUID. Doesn't that make my argument for me: that the tool doesn't
generate a UUID, it manages a persistent host identifier. Just like
"hostname." Therefore "nfshostid". I really don't see how nfshostid
is just as miserable as nfsuuid -- hence, I completely disagree
that "all arguments ... apply equally well".


In fairness, I'm trying to understand why you want to stick with
"nfsuuid". You originally said you wanted a generic tool. OK, but
now you say you don't have other uses for the tool after all. You
said you don't want it to be associated with an NFS client ID. That
part I still don't grok. Can you help me understand?


>> I object strongly to the name nfsuuid, and you seem to be inflexible. This
>> is not a hill I want to die on, however I reserve the right to say "I told
>> you so" when it turns out to be a poor choice.
> 
> How does agreeing with you multiple times in my last response and making
> further suggestions for names seem inflexible to you?  This is the worst
> part of working over email - I think you're misreading my good humor in the
> face of a drudging discussion as sarcasm or ill will.

Nope, not at all. It wasn't apparent that you agreed, as much
of your reply seemed to be disagreeing with my reply. So maybe
I am overreacting. Though my reply can also be read with humor,
even though it is a bit dry.


--
Chuck Lever
Benjamin Coddington Feb. 11, 2022, 9:06 p.m. UTC | #10
On 11 Feb 2022, at 15:51, Chuck Lever III wrote:

>> On Feb 11, 2022, at 3:16 PM, Benjamin Coddington 
>> <bcodding@redhat.com> wrote:
>>
>> On 11 Feb 2022, at 15:00, Chuck Lever III wrote:
>>
>>>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington 
>>>> <bcodding@redhat.com> wrote:
>>>>
>>>> All the arguments for exacting tolerances on how it should be named 
>>>> apply
>>>> equally well to anything that implies its output will be used for 
>>>> nfs client
>>>> ids, or host ids.
>>>
>>> I completely disagree with this assessment.
>>
>> But how, and in what way?  The tool just generates uuids, and spits 
>> them
>> out, or it spits out whatever's in the file you specify, up to 64 
>> chars.  If
>> we can't have uuid in the name, how can we have NFS or machine-id or
>> host-id?
>
> We don't have a tool called "string" to get and set the DNS name of
> the local host. It's called "hostname".
>
> The purpose of the proposed tool is to persist a unique string to be
> used as part of an NFS client ID. I would like to name the tool based
> on that purpose, not based on the way the content of the persistent
> file happens to be arranged some of the time.
>
> When the tool generates the string, it just happens to be a UUID. It
> doesn't have to be. The tool could generate a digest of the boot time
> or the current time. In fact, one of those is usually part of certain
> types of a UUID anyway. The fact that it is a UUID is totally not
> relevant. We happen to use a UUID because it has certain global
> uniqueness properties. (By the way, perhaps the man page could mention
> that global uniqueness is important for this identifier. Anything with
> similar guaranteed global uniqueness could be used).
>
> You keep admitting that the tool can output something that isn't a
> UUID. Doesn't that make my argument for me: that the tool doesn't
> generate a UUID, it manages a persistent host identifier. Just like
> "hostname." Therefore "nfshostid". I really don't see how nfshostid
> is just as miserable as nfsuuid -- hence, I completely disagree
> that "all arguments ... apply equally well".

Yes - your arguement is a good one.   I wasn't clear enough admitting 
you
were right two emails ago, sorry about that.

However, I still feel the same argument applied to "nfshostid" 
disqualifies
it as well.  It doesn't output the nfshostid.  That, if it even contains 
the
part outputted, is more than what's written out.

In my experience with linux tools, nfshostid sounds like something I can 
use
to set or retrieve the identifier for an NFS host, and this little tool 
does
not do that.

> In fairness, I'm trying to understand why you want to stick with
> "nfsuuid". You originally said you wanted a generic tool. OK, but
> now you say you don't have other uses for the tool after all. You
> said you don't want it to be associated with an NFS client ID. That
> part I still don't grok. Can you help me understand?

I don't want to stick with nfsuuid, I'm just trying to apply everyone's
reasoning to the current list of suggested names and it appears to me
that we've disqualified them all.

>>> I object strongly to the name nfsuuid, and you seem to be 
>>> inflexible. This
>>> is not a hill I want to die on, however I reserve the right to say 
>>> "I told
>>> you so" when it turns out to be a poor choice.
>>
>> How does agreeing with you multiple times in my last response and 
>> making
>> further suggestions for names seem inflexible to you?  This is the 
>> worst
>> part of working over email - I think you're misreading my good humor 
>> in the
>> face of a drudging discussion as sarcasm or ill will.
>
> Nope, not at all. It wasn't apparent that you agreed, as much
> of your reply seemed to be disagreeing with my reply. So maybe
> I am overreacting. Though my reply can also be read with humor,
> even though it is a bit dry.

Sorry about that -- I really just want to move forward.  I can send it 
again
as nfshostid.  I think if we are clear in the man page that it isn't
necessarily part or any of the identifier used by NFS, rather its just
trying to manage the persistent unique portion, it works for me.

Unless we can go with persistychars, because that makes me grin.

Ben
NeilBrown Feb. 14, 2022, 12:04 a.m. UTC | #11
On Sat, 12 Feb 2022, Benjamin Coddington wrote:
> On 11 Feb 2022, at 15:51, Chuck Lever III wrote:
> 
> >> On Feb 11, 2022, at 3:16 PM, Benjamin Coddington 
> >> <bcodding@redhat.com> wrote:
> >>
> >> On 11 Feb 2022, at 15:00, Chuck Lever III wrote:
> >>
> >>>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington 
> >>>> <bcodding@redhat.com> wrote:
> >>>>
> >>>> All the arguments for exacting tolerances on how it should be named 
> >>>> apply
> >>>> equally well to anything that implies its output will be used for 
> >>>> nfs client
> >>>> ids, or host ids.
> >>>
> >>> I completely disagree with this assessment.
> >>
> >> But how, and in what way?  The tool just generates uuids, and spits 
> >> them
> >> out, or it spits out whatever's in the file you specify, up to 64 
> >> chars.  If
> >> we can't have uuid in the name, how can we have NFS or machine-id or
> >> host-id?
> >
> > We don't have a tool called "string" to get and set the DNS name of
> > the local host. It's called "hostname".
> >
> > The purpose of the proposed tool is to persist a unique string to be
> > used as part of an NFS client ID. I would like to name the tool based
> > on that purpose, not based on the way the content of the persistent
> > file happens to be arranged some of the time.
> >
> > When the tool generates the string, it just happens to be a UUID. It
> > doesn't have to be. The tool could generate a digest of the boot time
> > or the current time. In fact, one of those is usually part of certain
> > types of a UUID anyway. The fact that it is a UUID is totally not
> > relevant. We happen to use a UUID because it has certain global
> > uniqueness properties. (By the way, perhaps the man page could mention
> > that global uniqueness is important for this identifier. Anything with
> > similar guaranteed global uniqueness could be used).
> >
> > You keep admitting that the tool can output something that isn't a
> > UUID. Doesn't that make my argument for me: that the tool doesn't
> > generate a UUID, it manages a persistent host identifier. Just like
> > "hostname." Therefore "nfshostid". I really don't see how nfshostid
> > is just as miserable as nfsuuid -- hence, I completely disagree
> > that "all arguments ... apply equally well".
> 
> Yes - your arguement is a good one.   I wasn't clear enough admitting 
> you
> were right two emails ago, sorry about that.
> 
> However, I still feel the same argument applied to "nfshostid" 
> disqualifies
> it as well.  It doesn't output the nfshostid.  That, if it even contains 
> the
> part outputted, is more than what's written out.
> 
> In my experience with linux tools, nfshostid sounds like something I can 
> use
> to set or retrieve the identifier for an NFS host, and this little tool 
> does
> not do that.
> 

I agree.  This tool primarily does 1 thing - it sets a string which will
be the uniquifier using the the client_owner4.  So I think the word
"set" should appear in the name.  I also think the name should start "nfs".
I don't much care whether it is
  nfssetid
  nfs-set-uuid
  nfssetowner
  nfssetuniquifier
  nfssetidentity
  nfsidset
though perhaps I'd prefer
  nfs=set=id

If not given any args, it should probably print a usage message rather
than perform a default action, to reduce the number of holes in feet.

.... Naming  - THE hard problem of computer engineering ....

NeilBrown
Benjamin Coddington Feb. 14, 2022, 11:15 a.m. UTC | #12
On 13 Feb 2022, at 19:04, NeilBrown wrote:

> On Sat, 12 Feb 2022, Benjamin Coddington wrote:
>> On 11 Feb 2022, at 15:51, Chuck Lever III wrote:
>>
>>>> On Feb 11, 2022, at 3:16 PM, Benjamin Coddington
>>>> <bcodding@redhat.com> wrote:
>>>>
>>>> On 11 Feb 2022, at 15:00, Chuck Lever III wrote:
>>>>
>>>>>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington
>>>>>> <bcodding@redhat.com> wrote:
>>>>>>
>>>>>> All the arguments for exacting tolerances on how it should be named
>>>>>> apply
>>>>>> equally well to anything that implies its output will be used for
>>>>>> nfs client
>>>>>> ids, or host ids.
>>>>>
>>>>> I completely disagree with this assessment.
>>>>
>>>> But how, and in what way?  The tool just generates uuids, and spits
>>>> them
>>>> out, or it spits out whatever's in the file you specify, up to 64
>>>> chars.  If
>>>> we can't have uuid in the name, how can we have NFS or machine-id or
>>>> host-id?
>>>
>>> We don't have a tool called "string" to get and set the DNS name of
>>> the local host. It's called "hostname".
>>>
>>> The purpose of the proposed tool is to persist a unique string to be
>>> used as part of an NFS client ID. I would like to name the tool based
>>> on that purpose, not based on the way the content of the persistent
>>> file happens to be arranged some of the time.
>>>
>>> When the tool generates the string, it just happens to be a UUID. It
>>> doesn't have to be. The tool could generate a digest of the boot time
>>> or the current time. In fact, one of those is usually part of certain
>>> types of a UUID anyway. The fact that it is a UUID is totally not
>>> relevant. We happen to use a UUID because it has certain global
>>> uniqueness properties. (By the way, perhaps the man page could mention
>>> that global uniqueness is important for this identifier. Anything with
>>> similar guaranteed global uniqueness could be used).
>>>
>>> You keep admitting that the tool can output something that isn't a
>>> UUID. Doesn't that make my argument for me: that the tool doesn't
>>> generate a UUID, it manages a persistent host identifier. Just like
>>> "hostname." Therefore "nfshostid". I really don't see how nfshostid
>>> is just as miserable as nfsuuid -- hence, I completely disagree
>>> that "all arguments ... apply equally well".
>>
>> Yes - your arguement is a good one.   I wasn't clear enough admitting
>> you
>> were right two emails ago, sorry about that.
>>
>> However, I still feel the same argument applied to "nfshostid"
>> disqualifies
>> it as well.  It doesn't output the nfshostid.  That, if it even contains
>> the
>> part outputted, is more than what's written out.
>>
>> In my experience with linux tools, nfshostid sounds like something I can
>> use
>> to set or retrieve the identifier for an NFS host, and this little tool
>> does
>> not do that.
>>
>
> I agree.  This tool primarily does 1 thing - it sets a string which will
> be the uniquifier using the the client_owner4.  So I think the word
> "set" should appear in the name.  I also think the name should start "nfs".
> I don't much care whether it is
>   nfssetid
>   nfs-set-uuid
>   nfssetowner
>   nfssetuniquifier
>   nfssetidentity
>   nfsidset
> though perhaps I'd prefer
>   nfs=set=id
>
> If not given any args, it should probably print a usage message rather
> than perform a default action, to reduce the number of holes in feet.
>
> .... Naming  - THE hard problem of computer engineering ....

No, it does NOT set the uniquifier string.  It returns it on stdout.  If
you run it without args it just prints the string.  Its completely harmless.

Ben
Chuck Lever Feb. 14, 2022, 3:39 p.m. UTC | #13
> On Feb 14, 2022, at 6:15 AM, Benjamin Coddington <bcodding@redhat.com> wrote:
> 
> On 13 Feb 2022, at 19:04, NeilBrown wrote:
> 
>> On Sat, 12 Feb 2022, Benjamin Coddington wrote:
>>> On 11 Feb 2022, at 15:51, Chuck Lever III wrote:
>>> 
>>>>> On Feb 11, 2022, at 3:16 PM, Benjamin Coddington
>>>>> <bcodding@redhat.com> wrote:
>>>>> 
>>>>> On 11 Feb 2022, at 15:00, Chuck Lever III wrote:
>>>>> 
>>>>>>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington
>>>>>>> <bcodding@redhat.com> wrote:
>>>>>>> 
>>>>>>> All the arguments for exacting tolerances on how it should be named
>>>>>>> apply
>>>>>>> equally well to anything that implies its output will be used for
>>>>>>> nfs client
>>>>>>> ids, or host ids.
>>>>>> 
>>>>>> I completely disagree with this assessment.
>>>>> 
>>>>> But how, and in what way?  The tool just generates uuids, and spits
>>>>> them
>>>>> out, or it spits out whatever's in the file you specify, up to 64
>>>>> chars.  If
>>>>> we can't have uuid in the name, how can we have NFS or machine-id or
>>>>> host-id?
>>>> 
>>>> We don't have a tool called "string" to get and set the DNS name of
>>>> the local host. It's called "hostname".
>>>> 
>>>> The purpose of the proposed tool is to persist a unique string to be
>>>> used as part of an NFS client ID. I would like to name the tool based
>>>> on that purpose, not based on the way the content of the persistent
>>>> file happens to be arranged some of the time.
>>>> 
>>>> When the tool generates the string, it just happens to be a UUID. It
>>>> doesn't have to be. The tool could generate a digest of the boot time
>>>> or the current time. In fact, one of those is usually part of certain
>>>> types of a UUID anyway. The fact that it is a UUID is totally not
>>>> relevant. We happen to use a UUID because it has certain global
>>>> uniqueness properties. (By the way, perhaps the man page could mention
>>>> that global uniqueness is important for this identifier. Anything with
>>>> similar guaranteed global uniqueness could be used).
>>>> 
>>>> You keep admitting that the tool can output something that isn't a
>>>> UUID. Doesn't that make my argument for me: that the tool doesn't
>>>> generate a UUID, it manages a persistent host identifier. Just like
>>>> "hostname." Therefore "nfshostid". I really don't see how nfshostid
>>>> is just as miserable as nfsuuid -- hence, I completely disagree
>>>> that "all arguments ... apply equally well".
>>> 
>>> Yes - your arguement is a good one.   I wasn't clear enough admitting
>>> you
>>> were right two emails ago, sorry about that.
>>> 
>>> However, I still feel the same argument applied to "nfshostid"
>>> disqualifies
>>> it as well.  It doesn't output the nfshostid.  That, if it even contains
>>> the
>>> part outputted, is more than what's written out.
>>> 
>>> In my experience with linux tools, nfshostid sounds like something I can
>>> use
>>> to set or retrieve the identifier for an NFS host, and this little tool
>>> does
>>> not do that.
>>> 
>> 
>> I agree.  This tool primarily does 1 thing - it sets a string which will
>> be the uniquifier using the the client_owner4.  So I think the word
>> "set" should appear in the name.  I also think the name should start "nfs".
>> I don't much care whether it is
>>  nfssetid
>>  nfs-set-uuid
>>  nfssetowner
>>  nfssetuniquifier
>>  nfssetidentity
>>  nfsidset
>> though perhaps I'd prefer
>>  nfs=set=id
>> 
>> If not given any args, it should probably print a usage message rather
>> than perform a default action, to reduce the number of holes in feet.
>> 
>> .... Naming  - THE hard problem of computer engineering ....
> 
> No, it does NOT set the uniquifier string.  It returns it on stdout.  If
> you run it without args it just prints the string.  Its completely harmless.

OK, my understanding was that if you run it as root, and the
string isn't already set, it does indeed set a value in the
persistence file.

That should be harmless, though. Once it is set, it is always
the same afterwards, and that's fine. Someone who just types
in the command to see what it does can't damage their
metatarsals; the worst that happens is the persistence file
is never used again.

I agree that's not very "set"-like.

 nfsgetuniquifier
 nfsguestuniquifier
 nfsnsuniquifier
 ns-uniquifier

Use with copious amounts of tab completion.

--
Chuck Lever
NeilBrown Feb. 14, 2022, 10:40 p.m. UTC | #14
On Mon, 14 Feb 2022, Benjamin Coddington wrote:
> On 13 Feb 2022, at 19:04, NeilBrown wrote:
> 
> > On Sat, 12 Feb 2022, Benjamin Coddington wrote:
> >> On 11 Feb 2022, at 15:51, Chuck Lever III wrote:
> >>
> >>>> On Feb 11, 2022, at 3:16 PM, Benjamin Coddington
> >>>> <bcodding@redhat.com> wrote:
> >>>>
> >>>> On 11 Feb 2022, at 15:00, Chuck Lever III wrote:
> >>>>
> >>>>>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington
> >>>>>> <bcodding@redhat.com> wrote:
> >>>>>>
> >>>>>> All the arguments for exacting tolerances on how it should be named
> >>>>>> apply
> >>>>>> equally well to anything that implies its output will be used for
> >>>>>> nfs client
> >>>>>> ids, or host ids.
> >>>>>
> >>>>> I completely disagree with this assessment.
> >>>>
> >>>> But how, and in what way?  The tool just generates uuids, and spits
> >>>> them
> >>>> out, or it spits out whatever's in the file you specify, up to 64
> >>>> chars.  If
> >>>> we can't have uuid in the name, how can we have NFS or machine-id or
> >>>> host-id?
> >>>
> >>> We don't have a tool called "string" to get and set the DNS name of
> >>> the local host. It's called "hostname".
> >>>
> >>> The purpose of the proposed tool is to persist a unique string to be
> >>> used as part of an NFS client ID. I would like to name the tool based
> >>> on that purpose, not based on the way the content of the persistent
> >>> file happens to be arranged some of the time.
> >>>
> >>> When the tool generates the string, it just happens to be a UUID. It
> >>> doesn't have to be. The tool could generate a digest of the boot time
> >>> or the current time. In fact, one of those is usually part of certain
> >>> types of a UUID anyway. The fact that it is a UUID is totally not
> >>> relevant. We happen to use a UUID because it has certain global
> >>> uniqueness properties. (By the way, perhaps the man page could mention
> >>> that global uniqueness is important for this identifier. Anything with
> >>> similar guaranteed global uniqueness could be used).
> >>>
> >>> You keep admitting that the tool can output something that isn't a
> >>> UUID. Doesn't that make my argument for me: that the tool doesn't
> >>> generate a UUID, it manages a persistent host identifier. Just like
> >>> "hostname." Therefore "nfshostid". I really don't see how nfshostid
> >>> is just as miserable as nfsuuid -- hence, I completely disagree
> >>> that "all arguments ... apply equally well".
> >>
> >> Yes - your arguement is a good one.   I wasn't clear enough admitting
> >> you
> >> were right two emails ago, sorry about that.
> >>
> >> However, I still feel the same argument applied to "nfshostid"
> >> disqualifies
> >> it as well.  It doesn't output the nfshostid.  That, if it even contains
> >> the
> >> part outputted, is more than what's written out.
> >>
> >> In my experience with linux tools, nfshostid sounds like something I can
> >> use
> >> to set or retrieve the identifier for an NFS host, and this little tool
> >> does
> >> not do that.
> >>
> >
> > I agree.  This tool primarily does 1 thing - it sets a string which will
> > be the uniquifier using the the client_owner4.  So I think the word
> > "set" should appear in the name.  I also think the name should start "nfs".
> > I don't much care whether it is
> >   nfssetid
> >   nfs-set-uuid
> >   nfssetowner
> >   nfssetuniquifier
> >   nfssetidentity
> >   nfsidset
> > though perhaps I'd prefer
> >   nfs=set=id
> >
> > If not given any args, it should probably print a usage message rather
> > than perform a default action, to reduce the number of holes in feet.
> >
> > .... Naming  - THE hard problem of computer engineering ....
> 
> No, it does NOT set the uniquifier string.  It returns it on stdout.  If
> you run it without args it just prints the string.  Its completely harmless.

Ahhh... I missed that.
Hence the "%c" magic in the udev rules, which takes the output and
stores it in the attribute.
This is a defensible approach if the tool will always be used with udev
... but you know what I think of using udev here.

Nevertheless, the main purpose when running this program is to set the
identity, even though the current implementation offloads some of the
work to udev.
So I think it should do exactly that and be named accordingly.

NeilBrown
Benjamin Coddington Feb. 16, 2022, 7:01 p.m. UTC | #15
On 14 Feb 2022, at 10:39, Chuck Lever III wrote:

>> On Feb 14, 2022, at 6:15 AM, Benjamin Coddington 
>> <bcodding@redhat.com> wrote:
>>
>> On 13 Feb 2022, at 19:04, NeilBrown wrote:
>>
>>> On Sat, 12 Feb 2022, Benjamin Coddington wrote:
>>>> On 11 Feb 2022, at 15:51, Chuck Lever III wrote:
>>>>
>>>>>> On Feb 11, 2022, at 3:16 PM, Benjamin Coddington
>>>>>> <bcodding@redhat.com> wrote:
>>>>>>
>>>>>> On 11 Feb 2022, at 15:00, Chuck Lever III wrote:
>>>>>>
>>>>>>>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington
>>>>>>>> <bcodding@redhat.com> wrote:
>>>>>>>>
>>>>>>>> All the arguments for exacting tolerances on how it should be 
>>>>>>>> named
>>>>>>>> apply
>>>>>>>> equally well to anything that implies its output will be used 
>>>>>>>> for
>>>>>>>> nfs client
>>>>>>>> ids, or host ids.
>>>>>>>
>>>>>>> I completely disagree with this assessment.
>>>>>>
>>>>>> But how, and in what way?  The tool just generates uuids, and 
>>>>>> spits
>>>>>> them
>>>>>> out, or it spits out whatever's in the file you specify, up to 64
>>>>>> chars.  If
>>>>>> we can't have uuid in the name, how can we have NFS or machine-id 
>>>>>> or
>>>>>> host-id?
>>>>>
>>>>> We don't have a tool called "string" to get and set the DNS name 
>>>>> of
>>>>> the local host. It's called "hostname".
>>>>>
>>>>> The purpose of the proposed tool is to persist a unique string to 
>>>>> be
>>>>> used as part of an NFS client ID. I would like to name the tool 
>>>>> based
>>>>> on that purpose, not based on the way the content of the 
>>>>> persistent
>>>>> file happens to be arranged some of the time.
>>>>>
>>>>> When the tool generates the string, it just happens to be a UUID. 
>>>>> It
>>>>> doesn't have to be. The tool could generate a digest of the boot 
>>>>> time
>>>>> or the current time. In fact, one of those is usually part of 
>>>>> certain
>>>>> types of a UUID anyway. The fact that it is a UUID is totally not
>>>>> relevant. We happen to use a UUID because it has certain global
>>>>> uniqueness properties. (By the way, perhaps the man page could 
>>>>> mention
>>>>> that global uniqueness is important for this identifier. Anything 
>>>>> with
>>>>> similar guaranteed global uniqueness could be used).
>>>>>
>>>>> You keep admitting that the tool can output something that isn't a
>>>>> UUID. Doesn't that make my argument for me: that the tool doesn't
>>>>> generate a UUID, it manages a persistent host identifier. Just 
>>>>> like
>>>>> "hostname." Therefore "nfshostid". I really don't see how 
>>>>> nfshostid
>>>>> is just as miserable as nfsuuid -- hence, I completely disagree
>>>>> that "all arguments ... apply equally well".
>>>>
>>>> Yes - your arguement is a good one.   I wasn't clear enough 
>>>> admitting
>>>> you
>>>> were right two emails ago, sorry about that.
>>>>
>>>> However, I still feel the same argument applied to "nfshostid"
>>>> disqualifies
>>>> it as well.  It doesn't output the nfshostid.  That, if it even 
>>>> contains
>>>> the
>>>> part outputted, is more than what's written out.
>>>>
>>>> In my experience with linux tools, nfshostid sounds like something 
>>>> I can
>>>> use
>>>> to set or retrieve the identifier for an NFS host, and this little 
>>>> tool
>>>> does
>>>> not do that.
>>>>
>>>
>>> I agree.  This tool primarily does 1 thing - it sets a string which 
>>> will
>>> be the uniquifier using the the client_owner4.  So I think the word
>>> "set" should appear in the name.  I also think the name should start 
>>> "nfs".
>>> I don't much care whether it is
>>>  nfssetid
>>>  nfs-set-uuid
>>>  nfssetowner
>>>  nfssetuniquifier
>>>  nfssetidentity
>>>  nfsidset
>>> though perhaps I'd prefer
>>>  nfs=set=id
>>>
>>> If not given any args, it should probably print a usage message 
>>> rather
>>> than perform a default action, to reduce the number of holes in 
>>> feet.
>>>
>>> .... Naming  - THE hard problem of computer engineering ....
>>
>> No, it does NOT set the uniquifier string.  It returns it on stdout.  
>> If
>> you run it without args it just prints the string.  Its completely 
>> harmless.
>
> OK, my understanding was that if you run it as root, and the
> string isn't already set, it does indeed set a value in the
> persistence file.
>
> That should be harmless, though. Once it is set, it is always
> the same afterwards, and that's fine. Someone who just types
> in the command to see what it does can't damage their
> metatarsals; the worst that happens is the persistence file
> is never used again.
>
> I agree that's not very "set"-like.
>
>  nfsgetuniquifier
>  nfsguestuniquifier
>  nfsnsuniquifier
>  ns-uniquifier
>
> Use with copious amounts of tab completion.

Coming back to this.. so it seems at least part of our disagreement 
about
the name had to do with a misunderstanding of what the tool did.

Just to finally clear the air about it: the tool does not write to 
sysfs, it
doesn't try to modify the client in any way.  I'm going to leave it up 
to
the distros.

Considering this, I think your only remaining objection to "nfsuuid" is 
that it
might return data other than a uuid if someone points it at a file that
contains data other than a uuid.

I can fix that.  And its probably not a bad idea either.  The nfsuuid 
tool
can ensure that the persisted data is a uuid.

Maybe I also need to change the man page or description of the patch to 
be
clearer about what the tool does.  Any suggestions there?

Ben
Chuck Lever Feb. 16, 2022, 7:35 p.m. UTC | #16
> On Feb 16, 2022, at 2:01 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
> 
> On 14 Feb 2022, at 10:39, Chuck Lever III wrote:
> 
>>> On Feb 14, 2022, at 6:15 AM, Benjamin Coddington <bcodding@redhat.com> wrote:
>>> 
>>> On 13 Feb 2022, at 19:04, NeilBrown wrote:
>>> 
>>>> On Sat, 12 Feb 2022, Benjamin Coddington wrote:
>>>>> On 11 Feb 2022, at 15:51, Chuck Lever III wrote:
>>>>> 
>>>>>>> On Feb 11, 2022, at 3:16 PM, Benjamin Coddington
>>>>>>> <bcodding@redhat.com> wrote:
>>>>>>> 
>>>>>>> On 11 Feb 2022, at 15:00, Chuck Lever III wrote:
>>>>>>> 
>>>>>>>>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington
>>>>>>>>> <bcodding@redhat.com> wrote:
>>>>>>>>> 
>>>>>>>>> All the arguments for exacting tolerances on how it should be named
>>>>>>>>> apply
>>>>>>>>> equally well to anything that implies its output will be used for
>>>>>>>>> nfs client
>>>>>>>>> ids, or host ids.
>>>>>>>> 
>>>>>>>> I completely disagree with this assessment.
>>>>>>> 
>>>>>>> But how, and in what way?  The tool just generates uuids, and spits
>>>>>>> them
>>>>>>> out, or it spits out whatever's in the file you specify, up to 64
>>>>>>> chars.  If
>>>>>>> we can't have uuid in the name, how can we have NFS or machine-id or
>>>>>>> host-id?
>>>>>> 
>>>>>> We don't have a tool called "string" to get and set the DNS name of
>>>>>> the local host. It's called "hostname".
>>>>>> 
>>>>>> The purpose of the proposed tool is to persist a unique string to be
>>>>>> used as part of an NFS client ID. I would like to name the tool based
>>>>>> on that purpose, not based on the way the content of the persistent
>>>>>> file happens to be arranged some of the time.
>>>>>> 
>>>>>> When the tool generates the string, it just happens to be a UUID. It
>>>>>> doesn't have to be. The tool could generate a digest of the boot time
>>>>>> or the current time. In fact, one of those is usually part of certain
>>>>>> types of a UUID anyway. The fact that it is a UUID is totally not
>>>>>> relevant. We happen to use a UUID because it has certain global
>>>>>> uniqueness properties. (By the way, perhaps the man page could mention
>>>>>> that global uniqueness is important for this identifier. Anything with
>>>>>> similar guaranteed global uniqueness could be used).
>>>>>> 
>>>>>> You keep admitting that the tool can output something that isn't a
>>>>>> UUID. Doesn't that make my argument for me: that the tool doesn't
>>>>>> generate a UUID, it manages a persistent host identifier. Just like
>>>>>> "hostname." Therefore "nfshostid". I really don't see how nfshostid
>>>>>> is just as miserable as nfsuuid -- hence, I completely disagree
>>>>>> that "all arguments ... apply equally well".
>>>>> 
>>>>> Yes - your arguement is a good one.   I wasn't clear enough admitting
>>>>> you
>>>>> were right two emails ago, sorry about that.
>>>>> 
>>>>> However, I still feel the same argument applied to "nfshostid"
>>>>> disqualifies
>>>>> it as well.  It doesn't output the nfshostid.  That, if it even contains
>>>>> the
>>>>> part outputted, is more than what's written out.
>>>>> 
>>>>> In my experience with linux tools, nfshostid sounds like something I can
>>>>> use
>>>>> to set or retrieve the identifier for an NFS host, and this little tool
>>>>> does
>>>>> not do that.
>>>>> 
>>>> 
>>>> I agree.  This tool primarily does 1 thing - it sets a string which will
>>>> be the uniquifier using the the client_owner4.  So I think the word
>>>> "set" should appear in the name.  I also think the name should start "nfs".
>>>> I don't much care whether it is
>>>> nfssetid
>>>> nfs-set-uuid
>>>> nfssetowner
>>>> nfssetuniquifier
>>>> nfssetidentity
>>>> nfsidset
>>>> though perhaps I'd prefer
>>>> nfs=set=id
>>>> 
>>>> If not given any args, it should probably print a usage message rather
>>>> than perform a default action, to reduce the number of holes in feet.
>>>> 
>>>> .... Naming  - THE hard problem of computer engineering ....
>>> 
>>> No, it does NOT set the uniquifier string.  It returns it on stdout.  If
>>> you run it without args it just prints the string.  Its completely harmless.
>> 
>> OK, my understanding was that if you run it as root, and the
>> string isn't already set, it does indeed set a value in the
>> persistence file.
>> 
>> That should be harmless, though. Once it is set, it is always
>> the same afterwards, and that's fine. Someone who just types
>> in the command to see what it does can't damage their
>> metatarsals; the worst that happens is the persistence file
>> is never used again.
>> 
>> I agree that's not very "set"-like.
>> 
>> nfsgetuniquifier
>> nfsguestuniquifier
>> nfsnsuniquifier
>> ns-uniquifier
>> 
>> Use with copious amounts of tab completion.
> 
> Coming back to this.. so it seems at least part of our disagreement about
> the name had to do with a misunderstanding of what the tool did.

I understand what your implementation does. I don't
understand why it works that way.

The disagreement is that I feel like you're trying to
abstract the operation of this little tool in ways that
no-one asked for. It's purpose is very narrow and
completely related to the NFSv4 client ID.


> Just to finally clear the air about it: the tool does not write to sysfs, it
> doesn't try to modify the client in any way.  I'm going to leave it up to
> the distros.

Seems to me that the distros care only about where the
persistence file is located. I can't see a problem with
Neil's suggestion that the tool also write into sysfs.
The location of the sysfs API is invariant; there should
be no need to configure it or expose it.

Can you give a reason why the tool should /not/ write
into sysfs?


> Considering this, I think your only remaining objection to "nfsuuid" is that it
> might return data other than a uuid if someone points it at a file that
> contains data other than a uuid.
> 
> I can fix that.  And its probably not a bad idea either.  The nfsuuid tool
> can ensure that the persisted data is a uuid.

Why is that necessary? I agree that any random string will
do, as long as it is the same after client reboots and is
globally unique. It can be a BLAKE2 hash or anything else.
Is there a hard requirement that the uniquifier is in the
form of an RFC 4122 UUID? (if there is, the requirement
should be explained in the man page).

I have no problem with using a UUID. I just don't believe
there's a hard requirement that it /must/ be a UUID.


> Maybe I also need to change the man page or description of the patch to be
> clearer about what the tool does.  Any suggestions there?

I've made some in previous responses. The rules about
reboot persistence and global uniqueness are paramount.

So I initially agreed with Trond's statement that the
client ID uniquifier is not specific to a particular
mount request, so doesn't belong in mount.nfs.

All still true. But...

If mount.nfs handles it instead, you don't need a
separate tool (naming problem solved), there's no lag
between when the uniquifier is set and the first
SETCLIENTID (race condition solved), no udev rule is
needed (no additional implementation complexity), and
no man page and no new module parameters (no
additional administrative configuration complexity).

What's not to like?


--
Chuck Lever
Benjamin Coddington Feb. 16, 2022, 8:44 p.m. UTC | #17
On 16 Feb 2022, at 14:35, Chuck Lever III wrote:

> On Feb 16, 2022, at 2:01 PM, Benjamin Coddington <bcodding@redhat.com> 
> wrote:
>> Coming back to this.. so it seems at least part of our disagreement 
>> about
>> the name had to do with a misunderstanding of what the tool did.
>
> I understand what your implementation does. I don't
> understand why it works that way.
>
> The disagreement is that I feel like you're trying to
> abstract the operation of this little tool in ways that
> no-one asked for. It's purpose is very narrow and
> completely related to the NFSv4 client ID.
>
>
>> Just to finally clear the air about it: the tool does not write to 
>> sysfs, it
>> doesn't try to modify the client in any way.  I'm going to leave it 
>> up to
>> the distros.
>
> Seems to me that the distros care only about where the
> persistence file is located. I can't see a problem with
> Neil's suggestion that the tool also write into sysfs.
> The location of the sysfs API is invariant; there should
> be no need to configure it or expose it.
>
> Can you give a reason why the tool should /not/ write
> into sysfs?

So that there is a division of work.  Systemd-udevd handles the event 
and
sets the attribute, not the tool.  Systemd-udevd is already set up to 
have
the appropriate selinux contexts and rights to make these changes.
Systemd-udevd's logging can clearly show the action and result instead
of it being hidden away inside this tool.  Systemd-udevd doesn't expect
programs to make the changes, its designed around the idea that programs
provide information and it makes the changes.

You can see how many issues we got into by imagining what would happen 
if
users ran this tool.  If the tool doesn't modify the client, that's one 
less
worry we have upstream, its the distro's problem.

If the tool writes to sysfs, then someone executing it manually can 
change
the client's id with a live mount.  That's bad.  That's less likely to
happen if its all tucked away in a udev rule.  I know the 
counter-arguement
"you can write to sysfs yourself", but I still think its better this 
way.

There's increased flexibility and utility - if an expert sysadmin wants 
to
distinguish groups of clients, they can customize their udev rule to add
custom strings to the uniquifier using the many additional points of 
data
available in udev rules.

>> Considering this, I think your only remaining objection to "nfsuuid" 
>> is that it
>> might return data other than a uuid if someone points it at a file 
>> that
>> contains data other than a uuid.
>>
>> I can fix that.  And its probably not a bad idea either.  The nfsuuid 
>> tool
>> can ensure that the persisted data is a uuid.
>
> Why is that necessary? I agree that any random string will
> do, as long as it is the same after client reboots and is
> globally unique. It can be a BLAKE2 hash or anything else.
> Is there a hard requirement that the uniquifier is in the
> form of an RFC 4122 UUID? (if there is, the requirement
> should be explained in the man page).
>
> I have no problem with using a UUID. I just don't believe
> there's a hard requirement that it /must/ be a UUID.

There's no hard requirement, as you know.  Its just an extremely useful
datatype for this purpose.  I'm sure we could make a `nfsblake2`, but I
can't imagine why.

>> Maybe I also need to change the man page or description of the patch 
>> to be
>> clearer about what the tool does.  Any suggestions there?
>
> I've made some in previous responses. The rules about
> reboot persistence and global uniqueness are paramount.

Ok, I can re-read the threads and find them and include them.

> So I initially agreed with Trond's statement that the
> client ID uniquifier is not specific to a particular
> mount request, so doesn't belong in mount.nfs.
>
> All still true. But...
>
> If mount.nfs handles it instead, you don't need a
> separate tool (naming problem solved), there's no lag
> between when the uniquifier is set and the first
> SETCLIENTID (race condition solved), no udev rule is
> needed (no additional implementation complexity), and
> no man page and no new module parameters (no
> additional administrative configuration complexity).

Yep.  The race is a problem.

> What's not to like?

Not much.  How can we figure out what we want to do as a community 
before
doing it all the ways?

Ben
NeilBrown Feb. 16, 2022, 11:16 p.m. UTC | #18
On Thu, 17 Feb 2022, Benjamin Coddington wrote:
> How can we figure out what we want to do as a community before doing
> it all the ways?

The evidence suggests that we can't.  Maybe we shouldn't.

Or maybe we can take a completely different approach.  As I like to say:
"When in doubt, document."  (OK, I just made that up...)

I propose the following text to be added to nfs(5) or maybe put in a
separate man page - nfs-container(5).


NFS IN A CONTAINER
  When NFS is used to mount filesystems in a container, and specifically
  in a separate network namespace, these mounts are treated as quite
  separate from any mounts in a different container or not in a
  container (i.e. in a different network namespace).

  In the NFSv4.x protocol, each client must have a unique identifier.
  This is used by the server to determine when a client has restarted,
  so any state from a previous instance can be discarded.  So any two
  concurrent clients that might access the same server MUST have
  different identifiers, and any two consecutive instances of the same
  client SHOULD have the same identifier.

  Linux constructs the identifier (referred to as co_ownerid in the NFS
  specifications) from various pieces of information, three of which can
  be controlled by the sysadmin:

  - the hostname.  This can be different in different containers if they
    have different "UTS" namespaces.  If the container system ensures
    each container sees a unique host name, then this is
    sufficient for a correctly functioning NFS identifier.
    The host name is copied when the first NFS filesystem is mounted in
    a given network namespace.  Any subsequent change in the apparent
    hostname will not change the NFSv4 identifier.

  - the value of the nfs.nfs4_unique_id module parameter.  This is the
    same for all containers on a given host so is not useful to
    differentiate between containers.

  - the value stored in /sys/fs/nfs/client/net/identifier.  This virtual
    file is local to the network namespace that it is accessed in and so
    can provided uniqueness between containers when the hostname is
    uniform among containers.

    This value is empty on namespace creation. though there is a pending
    kernel patch which initialises it to a random value.  This would mean
    that containers which currently rely simply on a unique hostname to
    create unique NFS identifiers, will now *not* have a stable
    identifier from one instance to the next.  This may be a regression,
    so the patch should probably be reconsidered.

    If the value is to be set, that should be done before the first
    mount (much as the hostname is copied before the first mount).
    If the container system has access to some sort of per-container
    identity, then a command like
       uuidgen --sha1 --namespace @url -N "nfs:$CONTAINER_ID" \
                 > /sys/fs/nfs/client/net/identifier 

    might be suitable.  If the containre system provides no stable name,
    but does have stable storage, then something like

      [ -s /etc/nfsv4-uuid ] || uuidgen > /etc/nfsv4-uuid && 
          cat /etc/nfsv4-uuid > /sys/fs/nfs/client/net/identifier 

    would suffice.

    If a container has neither a stable name nor stable (local) storage,
    then it is not possible to provide a stable identifer, so providing
    a random one to ensure uniqueness would be best

        uuidgen > /sys/fs/nfs/client/net/identifier


Comments, revisions, etc most welcome.

Thanks
NeilBrown
Chuck Lever Feb. 17, 2022, 2:43 p.m. UTC | #19
> On Feb 16, 2022, at 3:44 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
> 
> On 16 Feb 2022, at 14:35, Chuck Lever III wrote:
> 
>> On Feb 16, 2022, at 2:01 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
>>> Coming back to this.. so it seems at least part of our disagreement about
>>> the name had to do with a misunderstanding of what the tool did.
>> 
>> I understand what your implementation does. I don't
>> understand why it works that way.
>> 
>> The disagreement is that I feel like you're trying to
>> abstract the operation of this little tool in ways that
>> no-one asked for. It's purpose is very narrow and
>> completely related to the NFSv4 client ID.
>> 
>> 
>>> Just to finally clear the air about it: the tool does not write to sysfs, it
>>> doesn't try to modify the client in any way.  I'm going to leave it up to
>>> the distros.
>> 
>> Seems to me that the distros care only about where the
>> persistence file is located. I can't see a problem with
>> Neil's suggestion that the tool also write into sysfs.
>> The location of the sysfs API is invariant; there should
>> be no need to configure it or expose it.
>> 
>> Can you give a reason why the tool should /not/ write
>> into sysfs?
> 
> So that there is a division of work.  Systemd-udevd handles the event and
> sets the attribute, not the tool.  Systemd-udevd is already set up to have
> the appropriate selinux contexts and rights to make these changes.
> Systemd-udevd's logging can clearly show the action and result instead
> of it being hidden away inside this tool.  Systemd-udevd doesn't expect
> programs to make the changes, its designed around the idea that programs
> provide information and it makes the changes.
> 
> You can see how many issues we got into by imagining what would happen if
> users ran this tool.  If the tool doesn't modify the client, that's one less
> worry we have upstream, its the distro's problem.
> 
> If the tool writes to sysfs, then someone executing it manually can change
> the client's id with a live mount.  That's bad.  That's less likely to
> happen if its all tucked away in a udev rule.  I know the counter-arguement
> "you can write to sysfs yourself", but I still think its better this way.
> 
> There's increased flexibility and utility - if an expert sysadmin wants to
> distinguish groups of clients, they can customize their udev rule to add
> custom strings to the uniquifier using the many additional points of data
> available in udev rules.

I was not aware of the SELinux requirement and the particular
interactions the tool would have with systemd. Thanks for
explaining!

I think a "set-like" tool would work fine. We have plenty of
those. But I now understand why you designed the tool this way,
so I don't mind either way. The tool's man page, if we proceed
in that direction, should summarize the systemd expectations
(along with a SEE ALSO citation).


--
Chuck Lever
diff mbox series

Patch

diff --git a/.gitignore b/.gitignore
index c89d1cd2583d..4d63ee93b2dc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -61,6 +61,7 @@  utils/statd/statd
 tools/locktest/testlk
 tools/getiversion/getiversion
 tools/nfsconf/nfsconf
+tools/nfsuuid/nfsuuid
 support/export/mount.h
 support/export/mount_clnt.c
 support/export/mount_xdr.c
diff --git a/aclocal/libuuid.m4 b/aclocal/libuuid.m4
new file mode 100644
index 000000000000..f64085010d1d
--- /dev/null
+++ b/aclocal/libuuid.m4
@@ -0,0 +1,17 @@ 
+AC_DEFUN([AC_LIBUUID], [
+        LIBUUID=
+
+        AC_CHECK_LIB([uuid], [uuid_generate_random], [], [AC_MSG_FAILURE(
+                [Missing libuuid uuid_generate_random, needed to build nfs4id])])
+
+        AC_CHECK_LIB([uuid], [uuid_generate_sha1], [], [AC_MSG_FAILURE(
+                [Missing libuuid uuid_generate_sha1, needed to build nfs4id])])
+
+        AC_CHECK_LIB([uuid], [uuid_unparse], [], [AC_MSG_FAILURE(
+                [Missing libuuid uuid_unparse, needed to build nfs4id])])
+
+        AC_CHECK_HEADER([uuid/uuid.h], [], [AC_MSG_FAILURE(
+                [Missing uuid/uuid.h, needed to build nfs4id])])
+
+        AC_SUBST([LIBUUID], ["-luuid"])
+])
diff --git a/configure.ac b/configure.ac
index 50e9b321dcf3..1342c471f142 100644
--- a/configure.ac
+++ b/configure.ac
@@ -355,6 +355,9 @@  if test "$enable_nfsv4" = yes; then
   dnl check for the keyutils libraries and headers
   AC_KEYUTILS
 
+  dnl check for the libuuid library and headers
+  AC_LIBUUID
+
   dnl Check for sqlite3
   AC_SQLITE3_VERS
 
@@ -740,6 +743,7 @@  AC_CONFIG_FILES([
 	tools/nfsdclnts/Makefile
 	tools/nfsconf/Makefile
 	tools/nfsdclddb/Makefile
+	tools/nfsuuid/Makefile
 	utils/Makefile
 	utils/blkmapd/Makefile
 	utils/nfsdcld/Makefile
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 9b4b0803db39..a12b0f34f4e7 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -7,6 +7,7 @@  OPTDIRS += rpcgen
 endif
 
 OPTDIRS += nfsconf
+OPTDIRS += nfsuuid
 
 if CONFIG_NFSDCLD
 OPTDIRS += nfsdclddb
diff --git a/tools/nfsuuid/Makefile.am b/tools/nfsuuid/Makefile.am
new file mode 100644
index 000000000000..7b3a54c30d50
--- /dev/null
+++ b/tools/nfsuuid/Makefile.am
@@ -0,0 +1,8 @@ 
+## Process this file with automake to produce Makefile.in
+
+man8_MANS	= nfsuuid.man
+
+bin_PROGRAMS = nfsuuid
+
+nfsuuid_SOURCES = nfsuuid.c
+nfsuuid_LDADD = $(LIBUUID)
diff --git a/tools/nfsuuid/nfsuuid.c b/tools/nfsuuid/nfsuuid.c
new file mode 100644
index 000000000000..bbdec59f1afe
--- /dev/null
+++ b/tools/nfsuuid/nfsuuid.c
@@ -0,0 +1,203 @@ 
+/*
+ * nfsuuid.c -- create and persist uniquifiers for nfs4 clients
+ *
+ * Copyright (C) 2022  Red Hat, Benjamin Coddington <bcodding@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ */
+
+#include <stdio.h>
+#include <stdarg.h>
+#include <getopt.h>
+#include <string.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <uuid/uuid.h>
+
+#define DEFAULT_ID_FILE "/etc/nfsuuid"
+
+UUID_DEFINE(nfs4_clientid_uuid_template,
+	0xa2, 0x25, 0x68, 0xb2, 0x7a, 0x5f, 0x49, 0x90,
+	0x8f, 0x98, 0xc5, 0xf0, 0x67, 0x78, 0xcc, 0xf1);
+
+static char *prog;
+static char *source = NULL;
+static char *id_file = DEFAULT_ID_FILE;
+static char nfs_unique_id[64];
+static int replace = 0;
+
+static void usage(void)
+{
+	fprintf(stderr, "usage: %s [-r|--replace] [-f <file> |--file <file> ] [machine]\n", prog);
+}
+
+static void fatal(const char *fmt, ...)
+{
+	int err = errno;
+	va_list args;
+	char fatal_msg[256] = "fatal: ";
+
+	va_start(args, fmt);
+	vsnprintf(&fatal_msg[7], 255, fmt, args);
+	if (err)
+		fprintf(stderr, "%s: %s\n", fatal_msg, strerror(err));
+	else
+		fprintf(stderr, "%s\n", fatal_msg);
+	exit(-1);
+}
+
+static int read_nfs_unique_id(void)
+{
+	int fd;
+	ssize_t len;
+
+	if (replace) {
+		errno = ENOENT;
+		return -1;
+	}
+
+	fd = open(id_file, O_RDONLY);
+	if (fd < 0)
+		return fd;
+	len = read(fd, nfs_unique_id, 64);
+	close(fd);
+	return len;
+}
+
+static void write_nfs_unique_id(void)
+{
+	int fd;
+
+	fd = open(id_file, O_RDWR|O_TRUNC|O_CREAT, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH);
+	if (fd < 0)
+		fatal("could not write id to %s", id_file);
+	write(fd, nfs_unique_id, 37);
+}
+
+static void print_nfs_unique_id(void)
+{
+	fprintf(stdout, "%s", nfs_unique_id);
+}
+
+static void check_or_make_id(void)
+{
+	int ret;
+	uuid_t uuid;
+
+	ret = read_nfs_unique_id();
+	if (ret < 1) {
+		if (errno != ENOENT )
+			fatal("reading file %s", id_file);
+		uuid_generate_random(uuid);
+		uuid_unparse(uuid, nfs_unique_id);
+		nfs_unique_id[36] = '\n';
+		nfs_unique_id[37] = '\0';
+		write_nfs_unique_id();
+	}
+	print_nfs_unique_id();
+}
+
+static void check_or_make_id_from_machine(void)
+{
+	int fd, ret;
+	char machineid[32];
+	uuid_t uuid;
+
+	ret = read_nfs_unique_id();
+	if (ret < 1) {
+		if (errno != ENOENT )
+			fatal("reading file %s", id_file);
+
+		fd = open("/etc/machine-id", O_RDONLY);
+		if (fd < 0)
+			fatal("unable to read /etc/machine-id");
+
+		read(fd, machineid, 32);
+		close(fd);
+
+		uuid_generate_sha1(uuid, nfs4_clientid_uuid_template, machineid, 32);
+		uuid_unparse(uuid, nfs_unique_id);
+		nfs_unique_id[36] = '\n';
+		nfs_unique_id[37] = '\0';
+		write_nfs_unique_id();
+	}
+	print_nfs_unique_id();
+}
+
+int main(int argc, char **argv)
+{
+	prog = argv[0];
+
+	while (1) {
+		int opt, prev_ind;
+		int option_index = 0;
+		static struct option long_options[] = {
+			{"replace",	no_argument,		0, 'r' },
+			{"file",	required_argument,	0, 'f' },
+			{ 0, 0, 0, 0 }
+		};
+
+		errno = 0;
+		prev_ind = optind;
+		opt = getopt_long(argc, argv, ":rf:", long_options, &option_index);
+		if (opt == -1)
+			break;
+
+		/* Let's detect missing options in the middle of an option list */
+		if (optind == prev_ind + 2 && *optarg == '-') {
+			opt = ':';
+			--optind;
+		}
+
+		switch (opt) {
+		case 'r':
+			replace = 1;
+			break;
+		case 'f':
+			id_file = optarg;
+			break;
+		case ':':
+			usage();
+			fatal("option \"%s\" requires an argument", argv[prev_ind]);
+			break;
+		case '?':
+			usage();
+			fatal("unexpected arg \"%s\"", argv[optind - 1]);
+			break;
+		}
+	}
+
+	argc -= optind;
+
+	if (argc > 1) {
+		usage();
+		fatal("Too many arguments");
+	}
+
+	if (argc)
+		source = argv[optind++];
+
+	if (!source)
+		check_or_make_id();
+	else if (strcmp(source, "machine") == 0)
+		check_or_make_id_from_machine();
+	else {
+		usage();
+		fatal("unrecognized source %s\n", source);
+	}
+}
diff --git a/tools/nfsuuid/nfsuuid.man b/tools/nfsuuid/nfsuuid.man
new file mode 100644
index 000000000000..856d2f383e0f
--- /dev/null
+++ b/tools/nfsuuid/nfsuuid.man
@@ -0,0 +1,33 @@ 
+.\"
+.\" nfsuuid(8)
+.\"
+.TH nfsuuid 8 "10 Feb 2022"
+.SH NAME
+nfsuuid \- Generate or return nfs client id uniquifiers
+.SH SYNOPSIS
+.B nfsuuid [ -r | --replace ] [ -f | --file <file> ] [<source>]
+
+.SH DESCRIPTION
+The
+.B nfsuuid
+command provides a simple utility to help NFS Version 4 clients use unique
+and persistent client id values.  The command checks for the existence of a
+file /etc/nfsuuid and returns the first 64 chars read from that file.  If
+the file is not found, a UUID is generated from the specified source and
+written to the file and returned.
+.SH OPTIONS
+.TP
+.BR \-r,\ \-\-replace
+Overwrite the <file> with a UUID generated from <source>.
+.TP
+.BR \-f,\ \-\-file
+Use the specified file to lookup or store any generated UUID.  If <file> is
+not specified, the default file used is /etc/nfsuuid.
+.SH Sources
+If <source> is not specified, nfsuuid will generate a new random UUID.
+
+If <source> is "machine", nfsuuid will generate a deterministic UUID value
+derived from a sha1 hash of the contents of /etc/machine-id and a static
+key.
+.SH SEE ALSO
+.BR machine-id (5)