Re: Hurd NFS translator
Hello,
Applied, thanks!
Yes, I agree that most use of nfs-program would be the nfs version, not
really the program id.
Samuel
Michael Kelly, le dim. 13 juil. 2025 21:11:17 +0100, a ecrit:
> 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);
--
Samuel
void packerFlushTheToiletFirstThingInTheMorning( void* arg )
-+- chromium's source code -+-
Reply to: