On Tue, 16 Jun 2009 07:31:35 +0200, Marcel Telka <Marcel.Telka at sun.com>
wrote:
> 4) When _PC_FILESIZEBITS is not supported VOP_PATHCONF() will fail and return
> EINVAL (according to the comments section in the CR), so the codepath will
> break at line 1574 and your fix won't be used to salvage the situation:
>
> 1571 error = VOP_PATHCONF(sarg->cs->vp, _PC_FILESIZEBITS,
> &val,
> 1572 sarg->cs->cr, NULL);
> 1573 if (error)
> 1574 break;
> 1575
> 1576 /*
> 1577 * If the underlying file system does not support
> 1578 * _PC_FILESIZEBITS, return a reasonable default. Note
> that
> 1579 * error code on VOP_PATHCONF will be 0, even if the
> underlying
> 1580 * file system does not support _PC_FILESIZEBITS.
> 1581 */
> 1582 if (val == -1) {
> 1583 na->maxfilesize = MAXOFF32_T;
> 1584 } else {
> 1585 if (val >= (sizeof (uint64_t) * 8))
> 1586 na->maxfilesize = UINT64_MAX;
> 1587 else
> 1588 na->maxfilesize = ((1LL << val) - 1);
> 1589 }
I disagree, you have to distinguish between the fact that _PC_FILESIZEBITS is
not implemented and VOP_PATHCONF() not being implemented or returning an error.
The error out you complain about is the latter fact, ie. VOP_PATHCONF()
is not implemented by the underlaying file system at all and thus the default
VOP is
fs_nosys() ie. returning ENOSYS. bailing out here is the correct course of
action.
Or, the other part is if the underlaying file systems routine does implement
VOP_PATHCONF() but does not provide _PC_FILESIZEBITS component and does return
an error, like pcfs in the past with EINVAL. bailing out with an error is
correct here as well.
In general, the underlaying file system should for everything in fpathconf(2)
it does not support
default to the generic fs_pathconf() - I've fixed pcfs to adhere to this via
6656370 .
finally fs_pathconf() will deal with the fact that the underlaying file system
does not support _PC_FILESIZEBITS and will return -1 and _no_ error returned.
486 case _PC_FILESIZEBITS:
487
488 /*
489 * If ever we come here it means that underlying file
system
490 * does not recognise the command and therefore this
491 * configurable limit cannot be determined. We return -1
492 * and don't change errno.
493 */
494
495 val = (ulong_t)-1; /* large file support */
496 break;
which is where Piyush's fix comes into play finally.
There are corner cases for certain non-fpathconf(2) calls where the default
fs_pathconf()
action does not make sense for the underlaying file system and it still may be
forced
to directly return an error here, like pcfs_pathconf() for _PC_CASE_BEHAVIOR,
for which the fs_pathconf()
default of _CASE_SENSITIVE is wrong (FAT by definition is case insensitive)
but on the other hand pcfs does not implement the VFSFT_CASEINSENSITIVE feature
and it is not part of the standard fpathconf(2) query table. fwiw, we have a
bug open already
to change pcfs here too, see 6621526.
UFS on the other hand does not need to special case this as it is
_CASE_SENSITIVE
so the default action eventually taken in fs_pathconf() fallback from
ufs_l_pathconf()
does fit our needs.
---
frankB
