Hello,
Thanks for the patch revision, sorry I was on vacation for two weeks.
We're getting closer!
Mikhail Karpov, le dim. 09 août 2026 18:35:34 +0700, a ecrit:
> @@ -37,7 +37,7 @@ lib-subdirs = libshouldbeinlibc libihash libiohelp libports
> \
> # Hurd programs
> prog-subdirs = auth proc exec term \
> ext2fs isofs tmpfs fatfs \
> - storeio pflocal pfinet defpager mach-defpager \
> + pflocal pfinet defpager mach-defpager \
> login daemons boot console \
> hostmux usermux ftpfs trans \
> console-client utils sutils libfshelp-tests \
> @@ -70,6 +70,10 @@ ifeq ($(HAVE_LIBACPICA),yes)
> prog-subdirs += acpi
> endif
>
> +ifneq ($(PARTED_LIBS),)
> +prog-subdirs += storeio
> +endif
> +
Ah, no, we do not want to have to depend on parted for such a low-level
thing as storeio. Better automatically disable the part of the code that
adds partitions, it shouldn't be very hard, it'd essentially mean making
create_partitions just return ENOTDIR, and #ifdef-out the parted-using
functions.
> diff --git a/libnetfs/init-init.c b/libnetfs/init-init.c
> index 19ed0d3..f64c7bb 100644
> --- a/libnetfs/init-init.c
> +++ b/libnetfs/init-init.c
> @@ -38,9 +38,9 @@ void
> netfs_init (void)
> {
> error_t err;
> - err = maptime_map (0, 0, &netfs_mtime);
> + err = maptime_map (1, 0, &netfs_mtime);
> if (err)
> - err = maptime_map (1, 0, &netfs_mtime);
> + err = maptime_map (0, 0, &netfs_mtime);
> if (err)
> error (2, err, "mapping time");
>
Better make this a separate patch.
> diff --git a/storeio/dev.c b/storeio/dev.c
> index c87400c..97b36cd 100644
> --- a/storeio/dev.c
> +++ b/storeio/dev.c
> @@ -22,7 +22,6 @@
> #include <assert-backtrace.h>
> #include <string.h>
> #include <hurd/pager.h>
> -#include <hurd/store.h>
dev.c is still calling store_create etc. so should include store.h
> @@ -140,48 +139,31 @@ dev_buf_rw (struct dev *dev, size_t buf_offs, size_t
> *io_offs, size_t *len,
> return 0;
> }
> }
> -
> -/* Called with DEV->lock held. Try to open the store underlying DEV. */
> +
> error_t
> -dev_open (struct dev *dev)
> +dev_open_from_store (struct dev *dev, struct store *store)
Keep the commend about DEV->lock being held, and add a comment so the
reader understands the difference with dev_open.
> diff --git a/storeio/dev.h b/storeio/dev.h
> index eda7a93..2252f60 100644
> --- a/storeio/dev.h
> +++ b/storeio/dev.h
> @@ -84,14 +63,48 @@ struct dev
> pthread_mutex_t pager_lock;
> };
>
> +struct netnode
> +{
> + struct dev *dev;
> + struct opens *opens;
"opens" does not seem to be used any more?
> + char *name;
> + struct node **entries;
> + size_t entries_size;
> +};
Please comment what entries_size is exactly.
> diff --git a/storeio/pager.c b/storeio/pager.c
> index 11bf469..7df1a79 100644
> --- a/storeio/pager.c
> +++ b/storeio/pager.c
> @@ -27,6 +27,7 @@
> #include <error.h>
> #include <sys/mman.h>
> #include <stdio.h>
> +#include <string.h>
>
> #include "dev.h"
>
> @@ -51,6 +52,11 @@ pager_read_page (struct user_pager_info *upi,
> /* Read a partial page if necessary to avoid reading off the end. */
> want = store->size - page;
>
> + void *new_buf = mmap (0, want, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
> + if (new_buf == MAP_FAILED)
> + return EIO;
> +
> + *((void **) buf) = new_buf;
> err = dev_read (dev, page, want, (void **)buf, &read);
Mmm, why always allocating? It's better to allocate lazily in case the
read can be directly be fed by raw_read i.e. store_read, and avoid
a copy. That means keeping ensure_buf() etc., did you have a reason
to remove it? You can make pager_read_page set buf to NULL, and let
dev_read cope with that case, and otherwise try to use the provided
buffer (netfs_attempt_read case).
> diff --git a/storeio/storeio.c b/storeio/storeio.c
> index 4e8a962..3e79e12 100644
> --- a/storeio/storeio.c
> +++ b/storeio/storeio.c
[...]
> +static inline char *
> +create_node_name (const size_t num)
> {
> - struct dev *const device = fsys->hook;
> + char buffer[20];
> + snprintf (buffer, sizeof (buffer), "%zu", num);
> +
> + return strdup (buffer);
> +}
Better use asprintf.
Samuel
