There is a second attempted at making paths in the
device names that have multiple slashes or no slash
at all unmount-able, without breaking any APIs.
Due to the way entries are written to both the /etc/mtab
and /proc/mounts, multiple slash have to be stripped and
leading slash have to be added (when they don't exist)
on v4 mounts. With v3 mounts this normalization can
not occur since those entries are always in the same format.
Finally, this normalization only needs to happen
when the mtab and /proc/mounts are not the same file.
Steve Dickson (1):
umount.nfs: normalize path names during umounts.
utils/mount/nfsumount.c | 64 +++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 62 insertions(+), 2 deletions(-)
Steve Dickson [[email protected]] wrote:
> So path names are found during umounts, normalize
> path names by removing any extra slashes and add
> a lead slash if one does not exist.
>
> This normalization only has to occur when the mtab
> and /proc/mounts are not the same file.
>
> Signed-off-by: Steve Dickson <[email protected]>
> ---
> utils/mount/nfsumount.c | 64 +++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 62 insertions(+), 2 deletions(-)
>
> diff --git a/utils/mount/nfsumount.c b/utils/mount/nfsumount.c
> index 3538d88..0f77261 100644
> --- a/utils/mount/nfsumount.c
> +++ b/utils/mount/nfsumount.c
> @@ -140,6 +140,52 @@ static int del_mtab(const char *spec, const char *node)
> }
>
> /*
> + * To ensure the path is found during unmounts, strip
> + * off the multiple '/' or add a '/' if one does not exist.
> + */
> +static inline char *
> +normalize_path(char *spec)
> +{
> + char *colen, *ptr, *str, *dev;
> +
> + if ((colen = strchr(spec, ':')) == NULL)
> + return NULL;
> +
> + if (*(colen + 1) != '/') {
> + dev = str = malloc(strlen(spec) + 2);
> + if (dev == NULL)
> + return NULL;
> +
> + ptr = spec;
> + while (ptr <= colen)
> + *(str++) = *(ptr++);
> + *str++='/';
> + while (*ptr)
> + *(str++) = *(ptr++);
> + *str='\0';
> +
> + return dev;
> + }
> +
> + if (strstr(spec, "//") != NULL) {
> + dev = strdup(spec);
> + if (dev == NULL)
> + return NULL;
> +
> + colen = strchr(dev, ':');
> + ptr = (colen +1);
> + while (*ptr && *(ptr+1) == '/')
> + ptr++;
> + while (*ptr)
> + *(++colen) = *(ptr++);
> + *(colen+1) = '\0';
> +
> + return dev;
> + }
> + return NULL;
> +}
> +
> +/*
> * Detect NFSv4 mounts.
> *
> * Consult /proc/mounts to determine if the mount point
> @@ -154,6 +200,7 @@ static int nfs_umount_is_vers4(const struct mntentchn *mc)
> struct mntentchn *pmc;
> struct mount_options *options;
> int retval;
> + char *normpath=NULL;
>
> retval = -1;
> pmc = getprocmntdirbackward(mc->m.mnt_dir, NULL);
> @@ -171,8 +218,20 @@ static int nfs_umount_is_vers4(const struct mntentchn *mc)
> */
> while (pmc->m.mnt_fsname[nlen - 1] == '/')
> nlen--;
> - if (strncmp(pmc->m.mnt_fsname, mc->m.mnt_fsname, nlen) != 0)
> - continue;
> + /*
> + * When the mtab and /proc/mounts are not the same
> + * file, normalize the path in the mtab if needed.
> + */
> + if (mtab_is_writable())
> + normpath = normalize_path(mc->m.mnt_fsname);
> +
> + if (strncmp(pmc->m.mnt_fsname, mc->m.mnt_fsname, nlen) != 0) {
> + /* Is there a normalized path, if so compare that one too */
> + if (normpath == NULL)
> + continue;
> + if (strncmp(pmc->m.mnt_fsname, normpath, nlen) != 0)
> + continue;
You need to free normpath here before the "continue".
Also, when you normalize, why not go the extra mile of doing it all the
way as the patch I posted? I wanted to cover specs like
"host:/server/../home/./blah". This patch only does partial
normalization. The original patch also normalizes /proc/mount entry's
pathname (this avoids dealing with trailing '/' hack that exists now).
I don't understand the backward compatibility issue. Are there times when
you have a mount, and it's in /etc/mtab, but not in /proc/mounts? I don't
see how that's possible.
On 03/05/2012 02:37 PM, Chuck Lever wrote:
>
> On Mar 5, 2012, at 2:36 PM, Steve Dickson wrote:
>
>> There is a second attempted at making paths in the
>> device names that have multiple slashes or no slash
>> at all unmount-able, without breaking any APIs.
>>
>> Due to the way entries are written to both the /etc/mtab
>> and /proc/mounts, multiple slash have to be stripped and
>> leading slash have to be added (when they don't exist)
>> on v4 mounts.
>
> I thought a pathname without a leading slash was allowed for NFSv4, for mounting OSD devices? Not really sure.
Sorry I guess I didn't make myself clear... this adding and subtracting
of slashes only happen on umounts and only if /etc/mtab is not symbolically
to /proc/mounts. Has no effect on the mounts that do or do not have
slashes... Is that a bit clearer?
steved.
>
>> With v3 mounts this normalization can
>> not occur since those entries are always in the same format.
>>
>> Finally, this normalization only needs to happen
>> when the mtab and /proc/mounts are not the same file.
>>
>> Steve Dickson (1):
>> umount.nfs: normalize path names during umounts.
>>
>> utils/mount/nfsumount.c | 64 +++++++++++++++++++++++++++++++++++++++++++++-
>> 1 files changed, 62 insertions(+), 2 deletions(-)
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On 03/05/2012 09:25 PM, Malahal Naineni wrote:
> Jim Rees [[email protected]] wrote:
>> I don't understand the backward compatibility issue. Are there times when
>> you have a mount, and it's in /etc/mtab, but not in /proc/mounts? I don't
>> see how that's possible.
>
> No, but that is what NFS umount thinks due to slight differences in
> those two files. Happens only in NFSv4 world.
Exactly... When /etc/mtab and /proc/mounts are not symbolically
linked the v4 mount are written differently to both files as
well as v3 mount are also written differently that v4 mounts...
steved.
Jim Rees [[email protected]] wrote:
> I don't understand the backward compatibility issue. Are there times when
> you have a mount, and it's in /etc/mtab, but not in /proc/mounts? I don't
> see how that's possible.
No, but that is what NFS umount thinks due to slight differences in
those two files. Happens only in NFSv4 world.
On 03/05/2012 08:04 PM, Myklebust, Trond wrote:
>>> Is there any reason why we actually care about checking the crap
>>> in /etc/mtab on umount?
>>>
>> Yeah... its called backwards compatibility with older distros...
>> Believe, if I could bury mtab I would... in a New York minute!
>> I just don't see it happening...
>
> Yes, but despite all your work, you are just replacing one broken model
> by another.
>
>
> At least with the current code, they can _see_ that the model is broken
> and have an immediate incentive to move to the
> mtab-is-a-symlink-to-/proc/mounts based model. The latter is in any case
> the only one that is valid in the mount-namespace based world in which
> we've been living for the past 5 years or so...
>
I do not disagree with what you are saying... The fact the code/model
is broken is unarguably true... Relying on info in the mtab verses
/proc/mounts is completely brain dead... But those bits are on the
street and need to be fixed... So the question is how to fix
them...
Sending out a patch that symbolically links mtab to /proc/mounts
would solve this entire problem! But I just can't make major change
like that in established worlds... Who know what people use mtab
for... I certainly don't. Changing mtab to read-only is recipe for
disaster... IMHO...
Now if we don't want to take a patch like this in upstream
that's a different story... This patch is basically meaningless
when it comes to new Linux distros... I'm ok with that..
But I do think its wise for us to at least try to be somewhat
backwards compatible....
steved.
So path names are found during umounts, normalize
path names by removing any extra slashes and add
a lead slash if one does not exist.
This normalization only has to occur when the mtab
and /proc/mounts are not the same file.
Signed-off-by: Steve Dickson <[email protected]>
---
utils/mount/nfsumount.c | 64 +++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/utils/mount/nfsumount.c b/utils/mount/nfsumount.c
index 3538d88..0f77261 100644
--- a/utils/mount/nfsumount.c
+++ b/utils/mount/nfsumount.c
@@ -140,6 +140,52 @@ static int del_mtab(const char *spec, const char *node)
}
/*
+ * To ensure the path is found during unmounts, strip
+ * off the multiple '/' or add a '/' if one does not exist.
+ */
+static inline char *
+normalize_path(char *spec)
+{
+ char *colen, *ptr, *str, *dev;
+
+ if ((colen = strchr(spec, ':')) == NULL)
+ return NULL;
+
+ if (*(colen + 1) != '/') {
+ dev = str = malloc(strlen(spec) + 2);
+ if (dev == NULL)
+ return NULL;
+
+ ptr = spec;
+ while (ptr <= colen)
+ *(str++) = *(ptr++);
+ *str++='/';
+ while (*ptr)
+ *(str++) = *(ptr++);
+ *str='\0';
+
+ return dev;
+ }
+
+ if (strstr(spec, "//") != NULL) {
+ dev = strdup(spec);
+ if (dev == NULL)
+ return NULL;
+
+ colen = strchr(dev, ':');
+ ptr = (colen +1);
+ while (*ptr && *(ptr+1) == '/')
+ ptr++;
+ while (*ptr)
+ *(++colen) = *(ptr++);
+ *(colen+1) = '\0';
+
+ return dev;
+ }
+ return NULL;
+}
+
+/*
* Detect NFSv4 mounts.
*
* Consult /proc/mounts to determine if the mount point
@@ -154,6 +200,7 @@ static int nfs_umount_is_vers4(const struct mntentchn *mc)
struct mntentchn *pmc;
struct mount_options *options;
int retval;
+ char *normpath=NULL;
retval = -1;
pmc = getprocmntdirbackward(mc->m.mnt_dir, NULL);
@@ -171,8 +218,20 @@ static int nfs_umount_is_vers4(const struct mntentchn *mc)
*/
while (pmc->m.mnt_fsname[nlen - 1] == '/')
nlen--;
- if (strncmp(pmc->m.mnt_fsname, mc->m.mnt_fsname, nlen) != 0)
- continue;
+ /*
+ * When the mtab and /proc/mounts are not the same
+ * file, normalize the path in the mtab if needed.
+ */
+ if (mtab_is_writable())
+ normpath = normalize_path(mc->m.mnt_fsname);
+
+ if (strncmp(pmc->m.mnt_fsname, mc->m.mnt_fsname, nlen) != 0) {
+ /* Is there a normalized path, if so compare that one too */
+ if (normpath == NULL)
+ continue;
+ if (strncmp(pmc->m.mnt_fsname, normpath, nlen) != 0)
+ continue;
+ }
if (strcmp(pmc->m.mnt_type, "nfs4") == 0)
goto out_nfs4;
@@ -209,6 +268,7 @@ out_nfs:
retval = 0;
out:
+ free(normpath);
return retval;
}
--
1.7.1
Malahal Naineni [[email protected]] wrote:
> > while (pmc->m.mnt_fsname[nlen - 1] == '/')
> > nlen--;
> > - if (strncmp(pmc->m.mnt_fsname, mc->m.mnt_fsname, nlen) != 0)
> > - continue;
> > + /*
> > + * When the mtab and /proc/mounts are not the same
> > + * file, normalize the path in the mtab if needed.
> > + */
> > + if (mtab_is_writable())
> > + normpath = normalize_path(mc->m.mnt_fsname);
> > +
> > + if (strncmp(pmc->m.mnt_fsname, mc->m.mnt_fsname, nlen) != 0) {
> > + /* Is there a normalized path, if so compare that one too */
> > + if (normpath == NULL)
> > + continue;
> > + if (strncmp(pmc->m.mnt_fsname, normpath, nlen) != 0)
> > + continue;
>
> You need to free normpath here before the "continue".
Better yet, you don't need to normalize the path inside the loop. The
path you normalize doesn't change, so keep it outside "do while" loop.
Regards, Malahal.
T24gTW9uLCAyMDEyLTAzLTA1IGF0IDE5OjUzIC0wNTAwLCBTdGV2ZSBEaWNrc29uIHdyb3RlOg0K
PiANCj4gT24gMDMvMDUvMjAxMiAwNzozMSBQTSwgTXlrbGVidXN0LCBUcm9uZCB3cm90ZToNCj4g
PiBPbiBNb24sIDIwMTItMDMtMDUgYXQgMTk6MjcgLTA1MDAsIFN0ZXZlIERpY2tzb24gd3JvdGU6
DQo+ID4+DQo+ID4+IE9uIDAzLzA1LzIwMTIgMDQ6MjAgUE0sIE1hbGFoYWwgTmFpbmVuaSB3cm90
ZToNCj4gPj4+IFN0ZXZlIERpY2tzb24gW3N0ZXZlZEByZWRoYXQuY29tXSB3cm90ZToNCj4gPj4+
DQo+ID4+PiBBbHNvLCB3aGVuIHlvdSBub3JtYWxpemUsIHdoeSBub3QgZ28gdGhlIGV4dHJhIG1p
bGUgb2YgZG9pbmcgaXQgYWxsIHRoZQ0KPiA+Pj4gd2F5IGFzIHRoZSBwYXRjaCBJIHBvc3RlZD8g
SSB3YW50ZWQgdG8gY292ZXIgc3BlY3MgbGlrZQ0KPiA+Pj4gImhvc3Q6L3NlcnZlci8uLi9ob21l
Ly4vYmxhaCIuIFRoaXMgcGF0Y2ggb25seSBkb2VzIHBhcnRpYWwNCj4gPj4+IG5vcm1hbGl6YXRp
b24uIFRoZSBvcmlnaW5hbCBwYXRjaCBhbHNvIG5vcm1hbGl6ZXMgL3Byb2MvbW91bnQgZW50cnkn
cw0KPiA+Pj4gcGF0aG5hbWUgKHRoaXMgYXZvaWRzIGRlYWxpbmcgd2l0aCB0cmFpbGluZyAnLycg
aGFjayB0aGF0IGV4aXN0cyBub3cpLg0KPiA+PiBJIGp1c3QgZG8gbm90IHNlZSB0aGUgbmVlZCBm
b3IgdGhhdCB0eXBlIG9mIGNvbXBsZXhpdHkuLi4gTWF5YmUgSSdtDQo+ID4+IGJlaW5nIGEgYml0
IG5haXZlLCBidXQgSSBzZWUgdHdvIHByb2JsZW1zIGhlcmUuIE9uZSwgdjQgbW91bnRzIA0KPiA+
PiB3aXRoIG11bHRpcGxlIHNsYXNoZXMgYW5kIHR3byB2NCBtb3VudHMgd2l0aG91dCBhbnkgc2xh
c2hlcy4uLiANCj4gPj4NCj4gPj4gTm93IGJvdGggb2Ygb3VyIHBhdGNoZXMgZG8gYWRkcmVzcyB0
aG9zZSBpc3N1ZXMgYnV0IG1pbmUgb25seQ0KPiA+PiBhZGRyZXNzZXMgdGhvc2UgaXNzdWVzIGFu
ZCBubywgaXQgZG9lcyBub3QgZ28gdGhlICJleHRyYSBtaWxlIiANCj4gPj4gb2YgYWRkcmVzc2lu
ZyAnLi4nIGluIHBhdGggbmFtZXMsIGJ1dCBkb2VzIGl0IG5lZWQgdG8/IElzIA0KPiA+PiB0aGVy
ZSByZWFsbHkgYW4gdXNlIGNhc2Ugd2hlcmUgcGVvcGxlIGV4cG9ydCB0aGluZ3Mgd2l0aCAiLi4i
DQo+ID4+IGluIHRoZSBwYXRoPyANCj4gPj4NCj4gPj4gU29tZXRpbWVzIGdvaW5nIHRoZSBqdXN0
IGV4dHJhIG1pbGUganVzdCBicmluZ3MgbW9yZSBwYWluLi4uIA0KPiA+PiBmb3Igbm8gcmVhc29u
Li4uIGFuZCBiZWxpZXZlIG1lIEknbSBubyBydW5uZXIuLi4gIDgtKSANCj4gPiANCj4gPiBOb3Rl
IHRoYXQgdGhlIE5GU3Y0IHNlcnZlciBtYXkgaGF2ZSBzeW1saW5rcyBhbmQvb3IgcmVmZXJyYWxz
IGluIHRoZQ0KPiA+IG1vdW50IHBhdGgsIGluIHdoaWNoIGNhc2UgaXQgaXMgZ2FtZSBvdmVyIGZv
ciB0aGlzIGtpbmQgb2YgYXBwcm9hY2gNCj4gPiBhbnl3YXk6IHlvdSBjYW4ndCAnbm9ybWFsaXNl
JyB5b3VyIHdheSB0byBpbnRlcnByZXRpbmcgdGhhdC4uLg0KPiBXZWxsIEkgZ3Vlc3Mgd2Ugd2ls
bCBjcm9zcyB0aGF0IGJyaWRnZSB3aGVuIHdlIGdldCB0aGVyZS4uLiBBdA0KPiB0aGlzIHBvaW50
IEkganVzdCB3YW50IGZpeGVzIHRoZXNlIHR3byBidWdzIGFuZCBtb3ZlIG9uLi4uIA0KPiANCj4g
PiANCj4gPiBJcyB0aGVyZSBhbnkgcmVhc29uIHdoeSB3ZSBhY3R1YWxseSBjYXJlIGFib3V0IGNo
ZWNraW5nIHRoZSBjcmFwDQo+ID4gaW4gL2V0Yy9tdGFiIG9uIHVtb3VudD8NCj4gPiANCj4gWWVh
aC4uLiBpdHMgY2FsbGVkIGJhY2t3YXJkcyBjb21wYXRpYmlsaXR5IHdpdGggb2xkZXIgZGlzdHJv
cy4uLg0KPiBCZWxpZXZlLCBpZiBJIGNvdWxkIGJ1cnkgbXRhYiBJIHdvdWxkLi4uIGluIGEgTmV3
IFlvcmsgbWludXRlISANCj4gSSBqdXN0IGRvbid0IHNlZSBpdCBoYXBwZW5pbmcuLi4gDQoNClll
cywgYnV0IGRlc3BpdGUgYWxsIHlvdXIgd29yaywgeW91IGFyZSBqdXN0IHJlcGxhY2luZyBvbmUg
YnJva2VuIG1vZGVsDQpieSBhbm90aGVyLg0KDQpBdCBsZWFzdCB3aXRoIHRoZSBjdXJyZW50IGNv
ZGUsIHRoZXkgY2FuIF9zZWVfIHRoYXQgdGhlIG1vZGVsIGlzIGJyb2tlbg0KYW5kIGhhdmUgYW4g
aW1tZWRpYXRlIGluY2VudGl2ZSB0byBtb3ZlIHRvIHRoZQ0KbXRhYi1pcy1hLXN5bWxpbmstdG8t
L3Byb2MvbW91bnRzIGJhc2VkIG1vZGVsLiBUaGUgbGF0dGVyIGlzIGluIGFueSBjYXNlDQp0aGUg
b25seSBvbmUgdGhhdCBpcyB2YWxpZCBpbiB0aGUgbW91bnQtbmFtZXNwYWNlIGJhc2VkIHdvcmxk
IGluIHdoaWNoDQp3ZSd2ZSBiZWVuIGxpdmluZyBmb3IgdGhlIHBhc3QgNSB5ZWFycyBvciBzby4u
Lg0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoN
Ck5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQoNCg==
T24gTW9uLCAyMDEyLTAzLTA1IGF0IDE5OjI3IC0wNTAwLCBTdGV2ZSBEaWNrc29uIHdyb3RlOg0K
PiANCj4gT24gMDMvMDUvMjAxMiAwNDoyMCBQTSwgTWFsYWhhbCBOYWluZW5pIHdyb3RlOg0KPiA+
IFN0ZXZlIERpY2tzb24gW3N0ZXZlZEByZWRoYXQuY29tXSB3cm90ZToNCj4gPiANCj4gPiBBbHNv
LCB3aGVuIHlvdSBub3JtYWxpemUsIHdoeSBub3QgZ28gdGhlIGV4dHJhIG1pbGUgb2YgZG9pbmcg
aXQgYWxsIHRoZQ0KPiA+IHdheSBhcyB0aGUgcGF0Y2ggSSBwb3N0ZWQ/IEkgd2FudGVkIHRvIGNv
dmVyIHNwZWNzIGxpa2UNCj4gPiAiaG9zdDovc2VydmVyLy4uL2hvbWUvLi9ibGFoIi4gVGhpcyBw
YXRjaCBvbmx5IGRvZXMgcGFydGlhbA0KPiA+IG5vcm1hbGl6YXRpb24uIFRoZSBvcmlnaW5hbCBw
YXRjaCBhbHNvIG5vcm1hbGl6ZXMgL3Byb2MvbW91bnQgZW50cnkncw0KPiA+IHBhdGhuYW1lICh0
aGlzIGF2b2lkcyBkZWFsaW5nIHdpdGggdHJhaWxpbmcgJy8nIGhhY2sgdGhhdCBleGlzdHMgbm93
KS4NCj4gSSBqdXN0IGRvIG5vdCBzZWUgdGhlIG5lZWQgZm9yIHRoYXQgdHlwZSBvZiBjb21wbGV4
aXR5Li4uIE1heWJlIEknbQ0KPiBiZWluZyBhIGJpdCBuYWl2ZSwgYnV0IEkgc2VlIHR3byBwcm9i
bGVtcyBoZXJlLiBPbmUsIHY0IG1vdW50cyANCj4gd2l0aCBtdWx0aXBsZSBzbGFzaGVzIGFuZCB0
d28gdjQgbW91bnRzIHdpdGhvdXQgYW55IHNsYXNoZXMuLi4gDQo+IA0KPiBOb3cgYm90aCBvZiBv
dXIgcGF0Y2hlcyBkbyBhZGRyZXNzIHRob3NlIGlzc3VlcyBidXQgbWluZSBvbmx5DQo+IGFkZHJl
c3NlcyB0aG9zZSBpc3N1ZXMgYW5kIG5vLCBpdCBkb2VzIG5vdCBnbyB0aGUgImV4dHJhIG1pbGUi
IA0KPiBvZiBhZGRyZXNzaW5nICcuLicgaW4gcGF0aCBuYW1lcywgYnV0IGRvZXMgaXQgbmVlZCB0
bz8gSXMgDQo+IHRoZXJlIHJlYWxseSBhbiB1c2UgY2FzZSB3aGVyZSBwZW9wbGUgZXhwb3J0IHRo
aW5ncyB3aXRoICIuLiINCj4gaW4gdGhlIHBhdGg/IA0KPiANCj4gU29tZXRpbWVzIGdvaW5nIHRo
ZSBqdXN0IGV4dHJhIG1pbGUganVzdCBicmluZ3MgbW9yZSBwYWluLi4uIA0KPiBmb3Igbm8gcmVh
c29uLi4uIGFuZCBiZWxpZXZlIG1lIEknbSBubyBydW5uZXIuLi4gIDgtKSANCg0KTm90ZSB0aGF0
IHRoZSBORlN2NCBzZXJ2ZXIgbWF5IGhhdmUgc3ltbGlua3MgYW5kL29yIHJlZmVycmFscyBpbiB0
aGUNCm1vdW50IHBhdGgsIGluIHdoaWNoIGNhc2UgaXQgaXMgZ2FtZSBvdmVyIGZvciB0aGlzIGtp
bmQgb2YgYXBwcm9hY2gNCmFueXdheTogeW91IGNhbid0ICdub3JtYWxpc2UnIHlvdXIgd2F5IHRv
IGludGVycHJldGluZyB0aGF0Li4uDQoNCklzIHRoZXJlIGFueSByZWFzb24gd2h5IHdlIGFjdHVh
bGx5IGNhcmUgYWJvdXQgY2hlY2tpbmcgdGhlIGNyYXANCmluIC9ldGMvbXRhYiBvbiB1bW91bnQ/
DQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0K
TmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K
On 03/05/2012 04:30 PM, Malahal Naineni wrote:
> Malahal Naineni [[email protected]] wrote:
>>> while (pmc->m.mnt_fsname[nlen - 1] == '/')
>>> nlen--;
>>> - if (strncmp(pmc->m.mnt_fsname, mc->m.mnt_fsname, nlen) != 0)
>>> - continue;
>>> + /*
>>> + * When the mtab and /proc/mounts are not the same
>>> + * file, normalize the path in the mtab if needed.
>>> + */
>>> + if (mtab_is_writable())
>>> + normpath = normalize_path(mc->m.mnt_fsname);
>>> +
>>> + if (strncmp(pmc->m.mnt_fsname, mc->m.mnt_fsname, nlen) != 0) {
>>> + /* Is there a normalized path, if so compare that one too */
>>> + if (normpath == NULL)
>>> + continue;
>>> + if (strncmp(pmc->m.mnt_fsname, normpath, nlen) != 0)
>>> + continue;
>>
>> You need to free normpath here before the "continue".
>
> Better yet, you don't need to normalize the path inside the loop. The
> path you normalize doesn't change, so keep it outside "do while" loop.
Your are right... thank you!
steved.
On Mar 5, 2012, at 2:36 PM, Steve Dickson wrote:
> There is a second attempted at making paths in the
> device names that have multiple slashes or no slash
> at all unmount-able, without breaking any APIs.
>
> Due to the way entries are written to both the /etc/mtab
> and /proc/mounts, multiple slash have to be stripped and
> leading slash have to be added (when they don't exist)
> on v4 mounts.
I thought a pathname without a leading slash was allowed for NFSv4, for mounting OSD devices? Not really sure.
> With v3 mounts this normalization can
> not occur since those entries are always in the same format.
>
> Finally, this normalization only needs to happen
> when the mtab and /proc/mounts are not the same file.
>
> Steve Dickson (1):
> umount.nfs: normalize path names during umounts.
>
> utils/mount/nfsumount.c | 64 +++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 62 insertions(+), 2 deletions(-)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
On 03/05/2012 04:20 PM, Malahal Naineni wrote:
> Steve Dickson [[email protected]] wrote:
>> So path names are found during umounts, normalize
>> path names by removing any extra slashes and add
>> a lead slash if one does not exist.
>>
>> This normalization only has to occur when the mtab
>> and /proc/mounts are not the same file.
>>
>> Signed-off-by: Steve Dickson <[email protected]>
>> ---
>> utils/mount/nfsumount.c | 64 +++++++++++++++++++++++++++++++++++++++++++++-
>> 1 files changed, 62 insertions(+), 2 deletions(-)
>>
>> diff --git a/utils/mount/nfsumount.c b/utils/mount/nfsumount.c
>> index 3538d88..0f77261 100644
>> --- a/utils/mount/nfsumount.c
>> +++ b/utils/mount/nfsumount.c
>> @@ -140,6 +140,52 @@ static int del_mtab(const char *spec, const char *node)
>> }
>>
>> /*
>> + * To ensure the path is found during unmounts, strip
>> + * off the multiple '/' or add a '/' if one does not exist.
>> + */
>> +static inline char *
>> +normalize_path(char *spec)
>> +{
>> + char *colen, *ptr, *str, *dev;
>> +
>> + if ((colen = strchr(spec, ':')) == NULL)
>> + return NULL;
>> +
>> + if (*(colen + 1) != '/') {
>> + dev = str = malloc(strlen(spec) + 2);
>> + if (dev == NULL)
>> + return NULL;
>> +
>> + ptr = spec;
>> + while (ptr <= colen)
>> + *(str++) = *(ptr++);
>> + *str++='/';
>> + while (*ptr)
>> + *(str++) = *(ptr++);
>> + *str='\0';
>> +
>> + return dev;
>> + }
>> +
>> + if (strstr(spec, "//") != NULL) {
>> + dev = strdup(spec);
>> + if (dev == NULL)
>> + return NULL;
>> +
>> + colen = strchr(dev, ':');
>> + ptr = (colen +1);
>> + while (*ptr && *(ptr+1) == '/')
>> + ptr++;
>> + while (*ptr)
>> + *(++colen) = *(ptr++);
>> + *(colen+1) = '\0';
>> +
>> + return dev;
>> + }
>> + return NULL;
>> +}
>> +
>> +/*
>> * Detect NFSv4 mounts.
>> *
>> * Consult /proc/mounts to determine if the mount point
>> @@ -154,6 +200,7 @@ static int nfs_umount_is_vers4(const struct mntentchn *mc)
>> struct mntentchn *pmc;
>> struct mount_options *options;
>> int retval;
>> + char *normpath=NULL;
>>
>> retval = -1;
>> pmc = getprocmntdirbackward(mc->m.mnt_dir, NULL);
>> @@ -171,8 +218,20 @@ static int nfs_umount_is_vers4(const struct mntentchn *mc)
>> */
>> while (pmc->m.mnt_fsname[nlen - 1] == '/')
>> nlen--;
>> - if (strncmp(pmc->m.mnt_fsname, mc->m.mnt_fsname, nlen) != 0)
>> - continue;
>> + /*
>> + * When the mtab and /proc/mounts are not the same
>> + * file, normalize the path in the mtab if needed.
>> + */
>> + if (mtab_is_writable())
>> + normpath = normalize_path(mc->m.mnt_fsname);
>> +
>> + if (strncmp(pmc->m.mnt_fsname, mc->m.mnt_fsname, nlen) != 0) {
>> + /* Is there a normalized path, if so compare that one too */
>> + if (normpath == NULL)
>> + continue;
>> + if (strncmp(pmc->m.mnt_fsname, normpath, nlen) != 0)
>> + continue;
>
> You need to free normpath here before the "continue".
>
> Also, when you normalize, why not go the extra mile of doing it all the
> way as the patch I posted? I wanted to cover specs like
> "host:/server/../home/./blah". This patch only does partial
> normalization. The original patch also normalizes /proc/mount entry's
> pathname (this avoids dealing with trailing '/' hack that exists now).
I just do not see the need for that type of complexity... Maybe I'm
being a bit naive, but I see two problems here. One, v4 mounts
with multiple slashes and two v4 mounts without any slashes...
Now both of our patches do address those issues but mine only
addresses those issues and no, it does not go the "extra mile"
of addressing '..' in path names, but does it need to? Is
there really an use case where people export things with ".."
in the path?
Sometimes going the just extra mile just brings more pain...
for no reason... and believe me I'm no runner... 8-)
steved.
On 03/05/2012 07:31 PM, Myklebust, Trond wrote:
> On Mon, 2012-03-05 at 19:27 -0500, Steve Dickson wrote:
>>
>> On 03/05/2012 04:20 PM, Malahal Naineni wrote:
>>> Steve Dickson [[email protected]] wrote:
>>>
>>> Also, when you normalize, why not go the extra mile of doing it all the
>>> way as the patch I posted? I wanted to cover specs like
>>> "host:/server/../home/./blah". This patch only does partial
>>> normalization. The original patch also normalizes /proc/mount entry's
>>> pathname (this avoids dealing with trailing '/' hack that exists now).
>> I just do not see the need for that type of complexity... Maybe I'm
>> being a bit naive, but I see two problems here. One, v4 mounts
>> with multiple slashes and two v4 mounts without any slashes...
>>
>> Now both of our patches do address those issues but mine only
>> addresses those issues and no, it does not go the "extra mile"
>> of addressing '..' in path names, but does it need to? Is
>> there really an use case where people export things with ".."
>> in the path?
>>
>> Sometimes going the just extra mile just brings more pain...
>> for no reason... and believe me I'm no runner... 8-)
>
> Note that the NFSv4 server may have symlinks and/or referrals in the
> mount path, in which case it is game over for this kind of approach
> anyway: you can't 'normalise' your way to interpreting that...
Well I guess we will cross that bridge when we get there... At
this point I just want fixes these two bugs and move on...
>
> Is there any reason why we actually care about checking the crap
> in /etc/mtab on umount?
>
Yeah... its called backwards compatibility with older distros...
Believe, if I could bury mtab I would... in a New York minute!
I just don't see it happening...
steved.