Re: Hurd NFS translator
On 13/07/2025 11:51, Samuel Thibault wrote:
Previously the call to chown that was present did not consider the
return value. I have done so but am now wondering if there was a
specific reason to discard that error ?
That was probably just a miss, like the rest of the nfsv3 code which is
incomplete.
That being said, in case of an error, one should probably remove the
just-created file, since the operation is supposed to have failed?
I'll add this to my 'TODO' list. Meanwhile, here are 3 further patches
which reach the target of being able to compile gnumach on NFSv3 (but
only with my patched 'ld').
v3_5.patch:
1) Implement statfs for V3. There was little overlap with the v2 code so
it got an entirely separate implementation. There is the possibility of
completing statfs.f_namelen properly using FSINFO3 and PATHCONF3 rpc
calls but I didn't know if f_namelen should be the maximum length for a
file name component or the entire file name path.
2) nfs_attempt_lookup() did not unlock mutex on unlikely RPC init failure
3) nfs_attempt_rename() deadlocks performing an exclusive rename whilst
attempting to lock an already held mutex. Also doesn't unlock mutex
after processing wcc_stat.
4) Removed inapplicable comment in netfs_report_access().
v3_6.patch:
1) Use the variables for NFS program and NFS version rather than constants.
2) Remove comment regarding getservbyname as its conclusion was valid
and now implemented.
3) lookup root node handle specified by the mount server to retrieve the
true identity.
v3_7.patch:
1) Implement --nfs-program command line option to enable v3
functionality. Note that I have altered the specification of the
argument value so that the version can be specified without the program
number rather than the other way around. I think it is much more likely
the version would be specified than the program number. I can alter this
if required.
2) A couple of return code checks.
That completes all that I have implemented at the moment. The processing
of the RPC reply to SYMLINK3 and MKNOD3 are wrong but the NFS translator
appears to overcome that problem. The symlink succeeds and it can be
opened and read successfully.
I will correct these problems with a subsequent patch quite soon but I
think the V3 code can now be considered active but usable only with the
understanding that it should be considered very 'experimental'.
--- nfs.submitted/ops.c 2025-07-13 11:07:05.000000000 +0100
+++ nfs.new/ops.c 2025-07-13 17:20:42.000000000 +0100
@@ -439,11 +439,9 @@
return err;
}
-/* Implement the netfs_attempt_statfs callback as described in
- <hurd/netfs.h>. */
-error_t
-netfs_attempt_statfs (struct iouser *cred, struct node *np,
- struct statfs *st)
+static error_t
+netfs_attempt_statfs_v2 (struct iouser *cred, struct node *np,
+ struct statfs *st)
{
int *p;
void *rpcbuf;
@@ -484,6 +482,68 @@
return err;
}
+static error_t
+netfs_attempt_statfs_v3 (struct iouser *cred, struct node *np,
+ struct statfs *st)
+{
+ int *p;
+ void *rpcbuf;
+ error_t err;
+
+ p = nfs_initialize_rpc (NFS3PROC_FSSTAT, cred, 0, &rpcbuf, np, -1);
+ if (! p)
+ return errno;
+
+ p = xdr_encode_fhandle (p, &np->nn->handle);
+
+ err = conduct_rpc (&rpcbuf, &p);
+ if (!err)
+ {
+ err = nfs_error_trans (ntohl (*p++));
+ p = process_returned_stat (np, p, 1);
+
+ if (!err)
+ {
+ /* NFS V3 does not support the concept of blocks */
+ st->f_bsize = 1;
+
+ /* uint64t FSSTAT3resok.tbytes */
+ p = xdr_decode_64bit(p, &st->f_blocks);
+ /* uint64t FSSTAT3resok.fbytes */
+ p = xdr_decode_64bit(p, &st->f_bfree);
+ /* uint64t FSSTAT3resok.abytes */
+ p = xdr_decode_64bit(p, &st->f_bavail);
+ /* uint64t FSSTAT3resok.tfiles */
+ p = xdr_decode_64bit(p, &st->f_files);
+ /* uint64t FSSTAT3resok.ffiles */
+ p = xdr_decode_64bit(p, &st->f_ffree);
+
+ /* No need to decode the reply further */
+ /* uint64t FSSTAT3resok.afiles */
+ /* uint32 FSSTAT3resok.invarsec */
+
+ st->f_type = FSTYPE_NFS;
+ st->f_fsid = getpid ();
+
+ st->f_namelen = 0;
+ }
+ }
+
+ free (rpcbuf);
+ return err;
+}
+
+/* Implement the netfs_attempt_statfs callback as described in
+ <hurd/netfs.h>. */
+error_t
+netfs_attempt_statfs (struct iouser *cred, struct node *np,
+ struct statfs *st)
+{
+ return (protocol_version == 2
+ ? netfs_attempt_statfs_v2 (cred, np, st)
+ : netfs_attempt_statfs_v3 (cred, np, st));
+}
+
/* Implement the netfs_attempt_sync callback as described in
<hurd/netfs.h>. */
error_t
@@ -737,7 +797,10 @@
p = nfs_initialize_rpc (NFSPROC_LOOKUP (protocol_version),
cred, 0, &rpcbuf, np, -1);
if (! p)
- return errno;
+ {
+ pthread_mutex_unlock (&np->lock);
+ return errno;
+ }
p = xdr_encode_fhandle (p, &np->nn->handle);
p = xdr_encode_string (p, name);
@@ -1450,6 +1513,7 @@
if (err)
return err;
+ pthread_mutex_unlock(&np->lock);
err = netfs_attempt_link (cred, todir, np, toname, 1);
netfs_nput (np);
if (err)
@@ -1500,7 +1564,10 @@
{
pthread_mutex_lock (&fromdir->lock);
p = process_wcc_stat (fromdir, p, !err);
+ pthread_mutex_unlock (&fromdir->lock);
+ pthread_mutex_lock (&todir->lock);
p = process_wcc_stat (todir, p, !err);
+ pthread_mutex_unlock (&todir->lock);
}
}
@@ -1620,9 +1687,8 @@
{
err = nfs_error_trans (ntohl (*p));
p++;
- p = process_returned_stat (np, p, 0); /* XXX Should this be
- protected by the
- if (!err) ? */
+ p = process_returned_stat (np, p, 0);
+
if (!err)
{
ret = ntohl (*p);
diff -ur nfs.submitted/mount.c nfs.new/mount.c
--- nfs.submitted/mount.c 2025-06-22 20:49:52.000000000 +0100
+++ nfs.new/mount.c 2025-07-13 19:19:04.000000000 +0100
@@ -102,6 +102,7 @@
void *rpcbuf;
int port;
error_t err;
+ struct fhandle mount_fhandle;
struct node *np;
short pmapport;
@@ -110,13 +111,7 @@
{
struct servent *s;
- /* XXX This will always fail! pmap_service_name will always be "sunrpc"
- What should pmap_service_name really be? By definition the second
- argument is either "tcp" or "udp" Thus, is this backwards
- (as service_name suggests)? If so, should it read:
- s = getservbyname (pmap_service_name, "udp");
- or is there something I am missing here? */
- s = getservbyname ("sunrpc", pmap_service_name);
+ s = getservbyname (pmap_service_name, "udp");
if (s)
pmapport = s->s_port;
else
@@ -212,10 +207,9 @@
goto error_with_rpcbuf;
}
- /* Create the node for root */
- xdr_decode_fhandle (p, &np);
+ mount_fhandle.size = NFS2_FHSIZE;
+ memcpy(&mount_fhandle.data, p, mount_fhandle.size);
free (rpcbuf);
- pthread_mutex_unlock (&np->lock);
if (nfs_port_override)
port = nfs_port;
@@ -236,8 +230,8 @@
error (0, errno, "rpc");
goto error_with_rpcbuf;
}
- *(p++) = htonl (NFS_PROGRAM);
- *(p++) = htonl (NFS_VERSION);
+ *(p++) = htonl (nfs_program);
+ *(p++) = htonl (nfs_version);
*(p++) = htonl (IPPROTO_UDP);
*(p++) = htonl (0);
err = conduct_rpc (&rpcbuf, &p);
@@ -267,7 +261,47 @@
mounted_hostname = host;
mounted_nfs_port = port;
- return np;
+ /* The handle returned by the mount server is always NFS2_FHSIZE.
+ The handle on NFSv3 can be larger. An NFS server lookup for
+ the root node succeeds with the handle from the mount server
+ but a longer handle is returned as the true identity. This is
+ the one that must be maintained by the root node.
+ So refetch it here... */
+ p = initialize_rpc(nfs_program,
+ nfs_version,
+ NFSPROC_LOOKUP (protocol_version),
+ 0, &rpcbuf,
+ 0, 0, -1);
+ if (! p)
+ {
+ error (0, errno, "rpc");
+ goto error_with_rpcbuf;
+ }
+
+ p = xdr_encode_fhandle(p, &mount_fhandle);
+ p = xdr_encode_string (p, ".");
+
+ err = conduct_rpc (&rpcbuf, &p);
+
+ if (!err) {
+ err = nfs_error_trans (ntohl (*p));
+ p++;
+ }
+ else
+ {
+ error (0, errno, "rpc");
+ goto error_with_rpcbuf;
+ }
+
+ if (!err)
+ {
+ /* Create the node for root */
+ xdr_decode_fhandle (p, &np);
+ pthread_mutex_unlock (&np->lock);
+ free(rpcbuf);
+
+ return np;
+ }
error_with_rpcbuf:
free (rpcbuf);
diff -ur nfs.submitted/nfs.c nfs.new/nfs.c
--- nfs.submitted/nfs.c 2025-07-13 19:12:51.000000000 +0100
+++ nfs.new/nfs.c 2025-07-13 19:13:24.000000000 +0100
@@ -608,7 +608,7 @@
else
uid = gid = second_gid = -1;
- return initialize_rpc (NFS_PROGRAM, NFS_VERSION, rpc_proc, len, bufp,
+ return initialize_rpc (nfs_program, nfs_version, rpc_proc, len, bufp,
uid, gid, second_gid);
}
diff -ru nfs.submitted/main.c nfs.new/main.c
--- nfs.submitted/main.c 2025-06-22 20:49:52.000000000 +0100
+++ nfs.new/main.c 2025-07-13 14:57:23.000000000 +0100
@@ -197,7 +197,7 @@
"Port for nfs operations"},
{"default-nfs-port", OPT_NFS_PORT_D,"PORT", 0,
"Port for nfs operations, if none can be found automatically"},
- {"nfs-program", OPT_NFS_PROG, "ID[.VERS]"},
+ {"nfs-program", OPT_NFS_PROG, "[ID.]VERS"},
{"pmap-port", OPT_PMAP_PORT, "SVC|PORT"},
@@ -338,6 +338,30 @@
nfs_port = atoi (arg);
break;
+ case OPT_NFS_PROG:
+ {
+ const char* version = strrchr (arg, '.');
+ const char* program = NULL;
+
+ if (version != NULL)
+ {
+ program = arg;
+ version++;
+ }
+ else
+ version = arg;
+
+ nfs_version = atoi (version);
+ if (program)
+ nfs_program = atoi (program);
+
+ if (nfs_version < 2 || nfs_version > 3)
+ argp_error (state, "Invalid NFS version: %d", nfs_version);
+
+ protocol_version = nfs_version;
+ }
+ break;
+
case ARGP_KEY_ARG:
if (state->arg_num == 0)
remote_fs = arg;
@@ -378,12 +402,17 @@
struct sockaddr_in addr;
int ret;
- argp_parse (&argp, argc, argv, 0, 0, 0);
-
+ err = argp_parse (&argp, argc, argv, 0, 0, 0);
+ if (err)
+ error (1, err, "Invalid command line");
+
task_get_bootstrap_port (mach_task_self (), &bootstrap);
netfs_init ();
- main_udp_socket = socket (PF_INET, SOCK_DGRAM, 0);
+ if ((main_udp_socket = socket (PF_INET, SOCK_DGRAM, 0)) == -1) {
+ error(1, errno, "Socket failed");
+ }
+
addr.sin_family = AF_INET;
addr.sin_addr.s_addr = INADDR_ANY;
addr.sin_port = htons (IPPORT_RESERVED);
Reply to: