DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 08DA02081C
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\))
Subject: Re: [PATCH v1 3/3] Finish port of junction support to nfs-utils
From:   Chuck Lever <chuck.lever@oracle.com>
In-Reply-To: <6443fe07-373c-2ef2-c491-20bdd083e620@RedHat.com>
Date:   Wed, 28 Nov 2018 16:05:00 -0500
Cc:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <D30C0131-84B2-4A95-ADEC-E688FFF2FE64@oracle.com>
References: <20181031200218.7507.15214.stgit@klimt.1015granger.net>
 <20181031200229.7507.26189.stgit@klimt.1015granger.net>
 <EA0135DC-ADCF-49C1-A5C3-03499F62C37E@oracle.com>
 <6443fe07-373c-2ef2-c491-20bdd083e620@RedHat.com>
To:     Steve Dickson <SteveD@RedHat.com>
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk


> On Nov 28, 2018, at 4:00 PM, Steve Dickson <SteveD@RedHat.com> wrote:
>=20
>=20
>=20
> On 11/27/18 11:45 AM, Chuck Lever wrote:
>> Ping!
> My apologies for taking so long to get to this...=20
> I took an extended Thanksgiving then completely=20
> for got about it... Thanks for the ping... but :-)=20
>=20
> When I enable junctions via  --enable-junction I get the
> following compile error with or w/out these patches
>=20
> gcc -DHAVE_CONFIG_H -I. -I../../support/include  -I. =
-I../../support/include -D_GNU_SOURCE -pipe  -Wall  -Wextra  =
-Werror=3Dstrict-prototypes  -Werror=3Dmissing-prototypes  =
-Werror=3Dmissing-declarations  -Werror=3Dformat=3D2  -Werror=3Dundef  =
-Werror=3Dmissing-include-dirs  -Werror=3Dstrict-aliasing=3D2  =
-Werror=3Dinit-self  -Werror=3Dimplicit-function-declaration  =
-Werror=3Dreturn-type  -Werror=3Dswitch  -Werror=3Doverflow  =
-Werror=3Dparentheses  -Werror=3Daggregate-return  -Werror=3Dunused-result=
  -fno-strict-aliasing  -Werror=3Dformat-overflow=3D2 =
-Werror=3Dint-conversion -Werror=3Dincompatible-pointer-types =
-Werror=3Dmisleading-indentation -g -O2 -MT remove.o -MD -MP -MF =
.deps/remove.Tpo -c -o remove.o remove.c
> In file included from lookup.c:31:
> ../../support/include/rpcsvc/nfs_prot.h:9:10: fatal error: rpc/rpc.h: =
No such file or directory
> #include <rpc/rpc.h>
>          ^~~~~~~~~~~
> I know how to fix it... Add the following to CFLAGS and LDFLAGS
> `pkg-config --cflags libtirpc`
> `pkg-config --libs libtirpc`
>=20
> First I'm wonder how you got things to compile w/out making these =
changes

Well I built and tested this on RHEL 7. If you're testing on
late-model Fedora, you're probably getting a slightly different
build configuration. Just a guess.


> and secondly I'm wondering where we should make these changes...=20
>=20
> Maybe we should hardcode the -I/usr/include/tirpc and -ltirpc
> flags in aclocal/libtirpc.m4?

libtirpc.m4 seems like the right place, but hard-coding seems
like a sledgehammer. Maybe have a look at the changes I made
to libxml2.m4 and do something similar?


> steved.
>=20
>>=20
>>=20
>>> On Oct 31, 2018, at 4:02 PM, Chuck Lever <chuck.lever@oracle.com> =
wrote:
>>>=20
>>> CONFIG_JUNCTION was defined for use in Makefiles, but that does not
>>> provide a -DCONFIG_JUNCTION on the compiler command line. Add logic
>>> to configure.ac to get the compiler flags right.
>>>=20
>>> Now that mountd junction support is getting built, a few last minute
>>> porting bugs popped out. Fix those up.
>>>=20
>>> Reported-by: Yongcheng Yang <yoyang@redhat.com>
>>> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=3D1543126
>>> Fixes: 79978ed34973 ("mountd: Solder in support for NFS basic ... ")
>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>> ---
>>> configure.ac         |    8 +++++++-
>>> utils/mountd/cache.c |   18 +++++++++---------
>>> 2 files changed, 16 insertions(+), 10 deletions(-)
>>>=20
>>> diff --git a/configure.ac b/configure.ac
>>> index 16eae06..b458891 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -185,7 +185,13 @@ AC_ARG_ENABLE(junction,
>>> 			[enable support for NFS junctions =
@<:@default=3Dno@:>@])],
>>> 	enable_junction=3D$enableval,
>>> 	enable_junction=3Dno)
>>> -AM_CONDITIONAL(CONFIG_JUNCTION, [test "$enable_junction" =3D "yes" =
])
>>> +	if test "$enable_junction" =3D yes; then
>>> +		AC_DEFINE(HAVE_JUNCTION_SUPPORT, 1,
>>> +                          [Define this if you want junction support =
compiled in])
>>> +	else
>>> +		enable_junction=3D
>>> +	fi
>>> +	AM_CONDITIONAL(CONFIG_JUNCTION, [test "$enable_junction" =3D =
"yes" ])
>>>=20
>>> AC_ARG_ENABLE(tirpc,
>>> 	[AC_HELP_STRING([--disable-tirpc],
>>> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
>>> index 6f42512..7e8d403 100644
>>> --- a/utils/mountd/cache.c
>>> +++ b/utils/mountd/cache.c
>>> @@ -976,8 +976,9 @@ lookup_export(char *dom, char *path, struct =
addrinfo *ai)
>>> 	return found;
>>> }
>>>=20
>>> -#ifdef CONFIG_JUNCTION
>>> +#ifdef HAVE_JUNCTION_SUPPORT
>>>=20
>>> +#include <libxml/parser.h>
>>> #include "junction.h"
>>>=20
>>> struct nfs_fsloc_set {
>>> @@ -1084,8 +1085,7 @@ static bool locations_to_fslocdata(struct =
nfs_fsloc_set *locations,
>>> 	*ttl =3D 0;
>>>=20
>>> 	for (;;) {
>>> -		enum jp_status status;
>>> -		int len;
>>> +		int len, status;
>>>=20
>>> 		status =3D get_next_location(locations, &server,
>>> 							&rootpath, ttl);
>>> @@ -1219,7 +1219,7 @@ nfs_get_basic_junction(const char *junct_path, =
struct nfs_fsloc_set **locset)
>>> 		return EINVAL;
>>> 	}
>>>=20
>>> -	locset->ns_current =3D locset->ns_list;
>>> +	new->ns_current =3D new->ns_list;
>>> 	new->ns_ttl =3D 300;
>>> 	*locset =3D new;
>>> 	return 0;
>>> @@ -1242,7 +1242,7 @@ static struct exportent *lookup_junction(char =
*dom, const char *pathname,
>>> 	status =3D nfs_get_basic_junction(pathname, &locations);
>>> 	switch (status) {
>>> 		xlog(L_WARNING, "Dangling junction %s: %s",
>>> -			pathname, strerro(status));
>>> +			pathname, strerror(status));
>>> 		goto out;
>>> 	}
>>>=20
>>> @@ -1252,8 +1252,8 @@ static struct exportent *lookup_junction(char =
*dom, const char *pathname,
>>>=20
>>> 	exp =3D locations_to_export(locations, pathname, parent);
>>>=20
>>> -	nfs_free_locations(locset->ns_list);
>>> -	free(locset);
>>> +	nfs_free_locations(locations->ns_list);
>>> +	free(locations);
>>>=20
>>> out:
>>> 	xmlCleanupParser();
>>> @@ -1273,7 +1273,7 @@ static void lookup_nonexport(int f, char *buf, =
int buflen, char *dom, char *path
>>> 	free(eep);
>>> }
>>>=20
>>> -#else	/* !CONFIG_JUNCTION */
>>> +#else	/* !HAVE_JUNCTION_SUPPORT */
>>>=20
>>> static void lookup_nonexport(int f, char *buf, int buflen, char =
*dom, char *path,
>>> 		struct addrinfo *UNUSED(ai))
>>> @@ -1281,7 +1281,7 @@ static void lookup_nonexport(int f, char *buf, =
int buflen, char *dom, char *path
>>> 	dump_to_cache(f, buf, buflen, dom, path, NULL, 0);
>>> }
>>>=20
>>> -#endif	/* !CONFIG_JUNCTION */
>>> +#endif	/* !HAVE_JUNCTION_SUPPORT */
>>>=20
>>> static void nfsd_export(int f)
>>> {
>>>=20
>>=20
>> --
>> Chuck Lever
>>=20
>>=20
>>=20

--
Chuck Lever