diskfs readonly confusion? (was: Re: Bug#60807: fgetpos() fails on read only filesystems

[Marcus Brinkmann <Marcus.Brinkmann@ruhr-uni-bochum.de>]

On Wed, Apr 26, 2000 at 06:56:45PM +0100, Chris Lingard wrote:
> Not got much spare time, right now.  Stepping though the procedures fgetpos,
> fgets, __stdio_check_offset, __stdio_seek; the failure is in __io_seek.  Have not
> had time to check which procedure in __io_seek is setting OutP-> RetCode, maybe
> next week.

Thanks very much. I think I have localised the bug more closely.

> ****    This is where we get failure for read only file systems
> 
> Breakpoint 12, __io_seek (io_object=91, offset=0, whence=1, newp=0x1019aa8)
>     at /Source/glibc-2.1.3/i386-gnu/obj/hurd/RPC_io_seek.c:168
> 168             if (OutP->RetCode != KERN_SUCCESS)

Ok. RPC_ files are created by MIG. MIG is checking the return code of the
remote procedure call io_seek, as describd in hurd/io.defs and implemented
for file systems in libdiskfs/io-seek.c in the Hurd tree.

So, libdiskfs/io-seek.c is returning EROFS, and this has nothing to do with
glibc but with the Hurd implementing the io_seek call for disk filesystems.

Let's take a look at libdiskfs/io-seek.c:

------------------------------------------------------------------

#include "priv.h"
#include "io_S.h"
#include <unistd.h>

#define diskfs_readonly 0
#define diskfs_synchronous 0

/* Implement io_seek as described in <hurd/io.defs>. */
kern_return_t
diskfs_S_io_seek (struct protid *cred,
                  off_t offset,
                  int whence,
                  off_t *newoffset)
{

  CHANGE_NODE_FIELD (cred,
                   ({
                     iohelp_get_conch (&np->conch);
                     switch (whence)
                       {

                       /* ... some harmless stuff ... */

                       } 
                     *newoffset = cred->po->filepointer;
                   }));
}

-----------------------------------------------------------------

We see that this uses a macro CHANGE_NODE_FIELD, which is implemented in
libdiskfs/priv.h:

------------------------------------------------------------------

/* This macro locks the node associated with PROTID, and then
   evaluates the expression OPERATION; then it syncs the inode
   (without waiting) and unlocks everything, and then returns
   the value `err' (which can be set by OPERATION if desired). */
#define CHANGE_NODE_FIELD(PROTID, OPERATION)                                \
({                                                                          \
  error_t err = 0;                                                          \
  struct node *np;                                                          \
                                                                            \
  if (!(PROTID))                                                            \
    return EOPNOTSUPP;                                                      \
                                                                            \
  if (diskfs_check_readonly ())                                             \
    return EROFS;                                                           \
                                                                            \
  np = (PROTID)->po->np;                                                    \
                                                                            \
  mutex_lock (&np->lock);                                                   \
  (OPERATION);                                                              \
  if (diskfs_synchronous)                                                   \
    diskfs_node_update (np, 1);                                             \
  mutex_unlock (&np->lock);                                                 \
  return err;                                                               \
})

-------------------------------------------------------------------

Note the suspicious  "if (diskfs_check_readonly())".
This call is implemented in libdiskfs/readonly.c, and a bit weird:

-------------------------------------------------------------------
#include <fcntl.h>
#include <error.h>

#include "priv.h"

int _diskfs_diskdirty;
int diskfs_readonly = 0;
int diskfs_hard_readonly = 0;

int
diskfs_check_readonly ()
{
  error_t err;

  if (diskfs_readonly)
    return 1;
  else
    {
      if (!_diskfs_diskdirty)
        {
          err = diskfs_set_hypermetadata (1, 0);
          if (err)
            {
              error (0, 0,
                     "%s: MEDIA NOT WRITABLE; switching to READ-ONLY",
                     diskfs_disk_name ?: "-");
              diskfs_hard_readonly = diskfs_readonly = 1;
              return 1;
            }
          _diskfs_diskdirty = 1;
        }
      return 0;
    }
}
----------------------------------------------------------------------

So, what actually happens here? Frankly, I don't know. This should be
analysed by a debugger attached to the running filesystem server (isofs for
example), with a breakpoint on io_seek or diskfs_check_readonly.

Note that the first file, io_seek.c, defines diskfs_readonly as a macro to 0.
This is something I don't understand yet: How the static variable in
readonly.c cooperates with such macro definitions (what gets precedence
etc). Anyway, it seems that the check for readonly prevents io-seek from
working.

It doesn't seem as if the macro in io_seek.c gets used at all, does it?

I hope some of the core hackers can say something about it.

Thanks,
Marcus

-- 
`Rhubarb is no Egyptian god.' Debian http://www.debian.org Check Key server 
Marcus Brinkmann              GNU    http://www.gnu.org    for public PGP Key 
Marcus.Brinkmann@ruhr-uni-bochum.de,     marcus@gnu.org    PGP Key ID 36E7CD09
http://homepage.ruhr-uni-bochum.de/Marcus.Brinkmann/       brinkmd@debian.org