Re: [PATCH] Migrating storeio from trivfs to netfs.

Hello,

Mikhail Karpov, le mer. 05 août 2026 23:25:15 +0700, a ecrit:
> On Fri, 31 Jul 2026 02:13:26 Samuel Thibault wrote:
> > Uh? So if the disk image has not partition table at all, libparted says
> > that it has one partition? That's ugly :/ We'd really need to find a
> > way to ask libparted whether there is a partition table or not. Perhaps
> > through comparing the PedDiskType name field against "loop"?
> 
> While studying the libparted code, I discovered that dev->type is always set
> to PED_DEVICE_UNKNOWN in libparted/arch/gnu.c:

No, I didn't mean PedDeviceType, but really PedDiskType, in the PedDisk
structure:

  PedDisk *disk = ped_disk_new (device);

Then disk->type->name tells you the name of the partition type, which
can be "msdos", "gpt", and probably "loop" when there is no actual
partition table (last detector as last dummy resort).

> dev->type = PED_DEVICE_UNKNOWN;    /* It's deprecated anyway */
> 
> So first we need to understand how to determine the type based on the
> information the store can provide.

As mentioned, it's probably not worth bothering with implementing the
device type.

> > I don't see the relation with store_parsed?
> 
> I mean, we could check store_parsed for 'part:' and, if it detects it, avoid
> calling create_partitions, since we're guaranteed to be working with a
> partition, not the entire disk.

Ok but if the disk itself has no partition table, we have to properly
expose it.

And actually, some partition types can be nested.

> > I was rather thinking about all the code about permission checking and
> > locking that you have in your netfs_S_io_read/write before calling
> > dev_read/write.
> 
> We can pass peropen to pager_create instead of dev, and then check
> permissions and locking po->np->lock before calling dev_read/write.

? I don't understand, what is the point of this?

Really, I'm really *only* talking about this code:


netfs_S_io_read (struct protid *cred, data_t *data,
                 mach_msg_type_number_t *datalen, off_t offset,
                 vm_size_t amount)
{
  debug ("netfs_S_io_read:\n");

  if (!cred)
    {
      debug ("!cred\n");
      debug ("netfs_S_io_read return: EOPNOTSUPP\n");
      return EOPNOTSUPP;
    }

  if (!(cred->po->openstat & O_READ))
    {
      debug ("!(cred->po->openstat & O_READ)\n");
      debug ("netfs_S_io_read return: EBADF");
      return EBADF;
    }

  struct node *node = cred->po->np;
  pthread_mutex_lock (&node->lock);

  size_t data_size = *datalen;

  error_t err;
  if (offset < 0)
    {
      err = dev_read (node->nn->dev, cred->po->filepointer, amount,
                      (void **) data, &data_size);
      if (!err)
        cred->po->filepointer += data_size;
    }
  else
    err = dev_read (node->nn->dev, offset, amount, (void **) data, &data_size);

  pthread_mutex_unlock (&node->lock);
  *datalen = data_size;

  debug ("netfs_S_io_read return: %d\n", err);
  return err;
}

Everything but the call to dev_read are *exact* duplicates of libnetfs' provided netfs_S_io_read:

kern_return_t
netfs_S_io_read (struct protid *user,
		 data_t *data,
		 mach_msg_type_number_t *datalen,
		 off_t offset,
		 vm_size_t amount)
{
  error_t err;
  off_t start;
  struct node *node;
  int alloced = 0;
  size_t data_size = *datalen;

  if (!user)
    return EOPNOTSUPP;

  node = user->po->np;
  pthread_mutex_lock (&user->po->np->lock);

  if ((user->po->openstat & O_READ) == 0)
    {
      pthread_mutex_unlock (&node->lock);
      return EBADF;
    }

  if (amount > data_size)
    {
      void *new_data = mmap (0, amount, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
      if (new_data == MAP_FAILED)
	{
	  pthread_mutex_unlock (&node->lock);
	  return errno;
	}

      alloced = 1;
      *data = new_data;
    }
  data_size = amount;

  start = (offset == -1 ? user->po->filepointer : offset);

  if (start < 0)
    err = EINVAL;
  else if (S_ISLNK (node->nn_stat.st_mode))
    /* Read from a symlink.  */
    {
      off_t size = node->nn_stat.st_size;

      if (start + amount > size)
	amount = size - start;
      if (amount > size)
	amount = size;

      if (start >= size)
	{
	  data_size = 0;
	  err = 0;
	}
      else if (amount < size || start > 0)
	{
	  char *whole_link = alloca (size);
	  err = netfs_attempt_readlink (user->user, node, whole_link);
	  if (! err)
	    {
	      memcpy (*data, whole_link + start, amount);
	      data_size = amount;
	    }
	}
      else
	{
	  err = netfs_attempt_readlink (user->user, node, *data);
	  data_size = amount;
	}
    }
  else
    /* Read from a normal file.  */
    err = netfs_attempt_read (user->user, node, start, &data_size, *data);

  if (offset == -1 && !err)
    user->po->filepointer += data_size;

  pthread_mutex_unlock (&node->lock);

  if (err && alloced)
    munmap (*data, amount);

  if (!err && alloced && (round_page (data_size) < round_page (amount)))
    munmap (*data + round_page (data_size),
	    round_page (amount) - round_page (data_size));

  *datalen = data_size;
  return err;
}

The only spurious part is the S_ISLNK case, but we don't need to care.

*All* I'm saying is that instead of defining netfs_S_io_read, you can
probably simply define netfs_attempt_read, something like:

error_t
netfs_attempt_read (struct iouser * cred, struct node * node,
                   off_t offset, size_t * len, void *data)
{
    return dev_read(node->nn->dev, offset, *len, &data, len);
}


Way less duplicated code to maintain.


And similarly for netfs_S_io_write.

And netfs_S_io_map can probably be implemented by just providing
netfs_get_filemap.

And netfs_S_io_readable can probably be implemented by making
netfs_validate_stat actually fill np->nn_stat so libnetfs'
implementation of netfs_S_io_readable will just work already.

Samuel