Re: [PATCH] libstore: add sync function for file, mvol, remap and stripe

2026-01-27 Thread Samuel Thibault 
Hello,

Milos Nikic, le mar. 27 janv. 2026 10:43:20 -0800, a ecrit:
> Updated to use c99 named init...

Better have it separate, to avoid mixing unrelated things. I have
commited them separately.

Samuel

> And that also makes NULL vs 0 moot, which is
> nice, i think its much cleaner.
> 
> let me know!
> 
> Thanks,
> Milos
> 
> On Mon, Jan 26, 2026 at 11:47 PM Samuel Thibault <[1][email protected]>
> wrote:
> 
> Milos Nikic, le lun. 26 janv. 2026 23:13:52 -0800, a ecrit:
> > Should we go to 1999 version of C and modernize the struct 
> initialization
> in
> > these few files a bit?
> 
> Yes, that'd be nice.
> 
> > its very easy to make a mistake in the positional struct init, at least
> for me,
> > especially with so many NULLs (0s) next to each other.
> >
> > Also in 3 out of 4 files in that patch 0 is used, and in 1 file explicit
> NULL
> > is used to denote that function is not present, should we align them all
> to 0?
> 
> I'd rather say NULL.
> 
> Samuel
> 
> 
> References:
> 
> [1] mailto:[email protected]

> From a4638008c0d2a853dd0408a1a02e9cfac1f16495 Mon Sep 17 00:00:00 2001
> From: Milos Nikic 
> Date: Tue, 27 Jan 2026 10:30:18 -0800
> Subject: [PATCH] libstore: add sync function for file, mvol, remap and stripe.
> 
> Along side the store_sync this adds sync to other often used stores.
> Also change struct init from positional to named for store_class in
> these files.
> ---
>  libstore/file.c   | 22 +++---
>  libstore/mvol.c   | 31 ---
>  libstore/remap.c  | 24 +++-
>  libstore/stripe.c | 33 ++---
>  4 files changed, 96 insertions(+), 14 deletions(-)
> 
> diff --git a/libstore/file.c b/libstore/file.c
> index 80d6812a..55f97d0c 100644
> --- a/libstore/file.c
> +++ b/libstore/file.c
> @@ -76,6 +76,12 @@ file_write (struct store *store,
>return io_write (store->port, (void *) buf, len, addr * bsize, amount);
>  }
>  
> +static error_t
> +file_store_sync (struct store *store)
> +{
> +  return file_sync (store->port, 1, 0);
> +}
> +
>  static error_t
>  file_store_set_size (struct store *store, size_t newsize)
>  {
> @@ -223,9 +229,19 @@ file_map (const struct store *store, vm_prot_t prot, 
> mach_port_t *memobj)
>  const struct store_class
>  store_file_class =
>  {
> -  STORAGE_HURD_FILE, "file", file_read, file_write, file_store_set_size,
> -  store_std_leaf_allocate_encoding, store_std_leaf_encode, file_decode,
> -  file_set_flags, file_clear_flags, 0, 0, 0, file_open, 0, file_map
> +  .id = STORAGE_HURD_FILE,
> +  .name = "file",
> +  .read = file_read,
> +  .write = file_write,
> +  .set_size = file_store_set_size,
> +  .allocate_encoding = store_std_leaf_allocate_encoding,
> +  .encode = store_std_leaf_encode,
> +  .decode = file_decode,
> +  .set_flags = file_set_flags,
> +  .clear_flags = file_clear_flags,
> +  .open = file_open,
> +  .map = file_map,
> +  .sync = file_store_sync,
>  };
>  STORE_STD_CLASS (file);
>  
> diff --git a/libstore/mvol.c b/libstore/mvol.c
> index ee1526be..737ae640 100644
> --- a/libstore/mvol.c
> +++ b/libstore/mvol.c
> @@ -70,6 +70,25 @@ mvol_write (struct store *store,
>return err;
>  }
>  
> +static error_t
> +mvol_sync (struct store *store)
> +{
> +  size_t i;
> +  error_t err = 0;
> +
> +  /* We iterate through all known volumes to ensure
> +even inactive volumes are flushed to disk. */
> +  for (i = 0; i < store->num_children; i++)
> +{
> +  error_t e = store_sync (store->children[i]);
> +  /* If any child fails, we record the error but continue
> + syncing the others to maximize data safety. */
> +  if (e)
> + err = e;
> +}
> +  return err;
> +}
> +
>  static error_t
>  mvol_set_size (struct store *store, size_t newsize)
>  {
> @@ -87,9 +106,15 @@ mvol_remap (struct store *source,
>  const struct store_class
>  store_mvol_class =
>  {
> -  -1, "mvol", mvol_read, mvol_write, mvol_set_size,
> -  0, 0, 0,
> -  store_set_child_flags, store_clear_child_flags, 0, 0, mvol_remap
> +  .id = -1,
> +  .name = "mvol",
> +  .read = mvol_read,
> +  .write = mvol_write,
> +  .set_size = mvol_set_size,
> +  .set_flags = store_set_child_flags,
> +  .clear_flags = store_clear_child_flags,
> +  .remap = mvol_remap,
> +  .sync = mvol_sync,
>  };
>  STORE_STD_CLASS (mvol);
>  
> diff --git a/libstore/remap.c b/libstore/remap.c
> index de21812e..bbe78509 1006

Re: [PATCH] libstore: add sync function for file, mvol, remap and stripe

2026-01-27 Thread Milos Nikic 
OK here it is.
Updated to use c99 named init...And that also makes NULL vs 0 moot, which
is nice, i think its much cleaner.

let me know!

Thanks,
Milos

On Mon, Jan 26, 2026 at 11:47 PM Samuel Thibault 
wrote:

> Milos Nikic, le lun. 26 janv. 2026 23:13:52 -0800, a ecrit:
> > Should we go to 1999 version of C and modernize the struct
> initialization in
> > these few files a bit?
>
> Yes, that'd be nice.
>
> > its very easy to make a mistake in the positional struct init, at least
> for me,
> > especially with so many NULLs (0s) next to each other.
> >
> > Also in 3 out of 4 files in that patch 0 is used, and in 1 file explicit
> NULL
> > is used to denote that function is not present, should we align them all
> to 0?
>
> I'd rather say NULL.
>
> Samuel
>
From a4638008c0d2a853dd0408a1a02e9cfac1f16495 Mon Sep 17 00:00:00 2001
From: Milos Nikic 
Date: Tue, 27 Jan 2026 10:30:18 -0800
Subject: [PATCH] libstore: add sync function for file, mvol, remap and stripe.

Along side the store_sync this adds sync to other often used stores.
Also change struct init from positional to named for store_class in
these files.
---
 libstore/file.c   | 22 +++---
 libstore/mvol.c   | 31 ---
 libstore/remap.c  | 24 +++-
 libstore/stripe.c | 33 ++---
 4 files changed, 96 insertions(+), 14 deletions(-)

diff --git a/libstore/file.c b/libstore/file.c
index 80d6812a..55f97d0c 100644
--- a/libstore/file.c
+++ b/libstore/file.c
@@ -76,6 +76,12 @@ file_write (struct store *store,
   return io_write (store->port, (void *) buf, len, addr * bsize, amount);
 }
 
+static error_t
+file_store_sync (struct store *store)
+{
+  return file_sync (store->port, 1, 0);
+}
+
 static error_t
 file_store_set_size (struct store *store, size_t newsize)
 {
@@ -223,9 +229,19 @@ file_map (const struct store *store, vm_prot_t prot, mach_port_t *memobj)
 const struct store_class
 store_file_class =
 {
-  STORAGE_HURD_FILE, "file", file_read, file_write, file_store_set_size,
-  store_std_leaf_allocate_encoding, store_std_leaf_encode, file_decode,
-  file_set_flags, file_clear_flags, 0, 0, 0, file_open, 0, file_map
+  .id = STORAGE_HURD_FILE,
+  .name = "file",
+  .read = file_read,
+  .write = file_write,
+  .set_size = file_store_set_size,
+  .allocate_encoding = store_std_leaf_allocate_encoding,
+  .encode = store_std_leaf_encode,
+  .decode = file_decode,
+  .set_flags = file_set_flags,
+  .clear_flags = file_clear_flags,
+  .open = file_open,
+  .map = file_map,
+  .sync = file_store_sync,
 };
 STORE_STD_CLASS (file);
 
diff --git a/libstore/mvol.c b/libstore/mvol.c
index ee1526be..737ae640 100644
--- a/libstore/mvol.c
+++ b/libstore/mvol.c
@@ -70,6 +70,25 @@ mvol_write (struct store *store,
   return err;
 }
 
+static error_t
+mvol_sync (struct store *store)
+{
+  size_t i;
+  error_t err = 0;
+
+  /* We iterate through all known volumes to ensure
+even inactive volumes are flushed to disk. */
+  for (i = 0; i < store->num_children; i++)
+{
+  error_t e = store_sync (store->children[i]);
+  /* If any child fails, we record the error but continue
+	syncing the others to maximize data safety. */
+  if (e)
+	err = e;
+}
+  return err;
+}
+
 static error_t
 mvol_set_size (struct store *store, size_t newsize)
 {
@@ -87,9 +106,15 @@ mvol_remap (struct store *source,
 const struct store_class
 store_mvol_class =
 {
-  -1, "mvol", mvol_read, mvol_write, mvol_set_size,
-  0, 0, 0,
-  store_set_child_flags, store_clear_child_flags, 0, 0, mvol_remap
+  .id = -1,
+  .name = "mvol",
+  .read = mvol_read,
+  .write = mvol_write,
+  .set_size = mvol_set_size,
+  .set_flags = store_set_child_flags,
+  .clear_flags = store_clear_child_flags,
+  .remap = mvol_remap,
+  .sync = mvol_sync,
 };
 STORE_STD_CLASS (mvol);
 
diff --git a/libstore/remap.c b/libstore/remap.c
index de21812e..bbe78509 100644
--- a/libstore/remap.c
+++ b/libstore/remap.c
@@ -41,6 +41,12 @@ remap_write (struct store *store,
   return store_write (store->children[0], addr, buf, len, amount);
 }
 
+static error_t
+remap_sync (struct store *store)
+{
+  return store_sync (store->children[0]);
+}
+
 static error_t
 remap_set_size (struct store *store, size_t newsize)
 {
@@ -206,11 +212,19 @@ remap_validate_name (const char *name,
 const struct store_class
 store_remap_class =
 {
-  STORAGE_REMAP, "remap", remap_read, remap_write, remap_set_size,
-  remap_allocate_encoding, remap_encode, remap_decode,
-  store_set_child_flags, store_clear_child_flags,
-  NULL, NULL, NULL,		/* cleanup, clone, remap */
-  remap_open, remap_validate_name
+  .id = STORAGE_REMAP,
+  .name = "remap",
+  .read = remap_read,
+  .write = remap_write,
+  .set_size = remap_set_size,
+  .allocate_encoding = remap_allocate_encoding,
+  .

Re: [PATCH] libstore: add sync function for file, mvol, remap and stripe

2026-01-26 Thread Samuel Thibault 
Milos Nikic, le lun. 26 janv. 2026 23:13:52 -0800, a ecrit:
> Should we go to 1999 version of C and modernize the struct initialization in
> these few files a bit?

Yes, that'd be nice.

> its very easy to make a mistake in the positional struct init, at least for 
> me,
> especially with so many NULLs (0s) next to each other.
> 
> Also in 3 out of 4 files in that patch 0 is used, and in 1 file explicit NULL
> is used to denote that function is not present, should we align them all to 0?

I'd rather say NULL.

Samuel

Re: [PATCH] libstore: add sync function for file, mvol, remap and stripe

2026-01-26 Thread Milos Nikic 
A question:
Should we go to 1999 version of C and modernize the struct initialization
in these few files a bit?
its very easy to make a mistake in the positional struct init, at least for
me, especially with so many NULLs (0s) next to each other.

Also in 3 out of 4 files in that patch 0 is used, and in 1 file explicit
NULL is used to denote that function is not present, should we align them
all to 0?

Regards,
Milos


On Mon, Jan 26, 2026 at 9:22 PM Milos Nikic  wrote:

> Along side the store_sync this adds sync to other often used stores.
> ---
>  libstore/file.c   |  9 -
>  libstore/mvol.c   | 20 +++-
>  libstore/remap.c  |  9 -
>  libstore/stripe.c | 21 -
>  4 files changed, 55 insertions(+), 4 deletions(-)
>
> diff --git a/libstore/file.c b/libstore/file.c
> index 80d6812a..5f6d7a46 100644
> --- a/libstore/file.c
> +++ b/libstore/file.c
> @@ -76,6 +76,12 @@ file_write (struct store *store,
>return io_write (store->port, (void *) buf, len, addr * bsize, amount);
>  }
>
> +static error_t
> +file_store_sync (struct store *store)
> +{
> +  return file_sync (store->port, 1, 0);
> +}
> +
>  static error_t
>  file_store_set_size (struct store *store, size_t newsize)
>  {
> @@ -225,7 +231,8 @@ store_file_class =
>  {
>STORAGE_HURD_FILE, "file", file_read, file_write, file_store_set_size,
>store_std_leaf_allocate_encoding, store_std_leaf_encode, file_decode,
> -  file_set_flags, file_clear_flags, 0, 0, 0, file_open, 0, file_map
> +  file_set_flags, file_clear_flags, 0, 0, 0, file_open, 0, file_map,
> +  file_store_sync
>  };
>  STORE_STD_CLASS (file);
>
> diff --git a/libstore/mvol.c b/libstore/mvol.c
> index ee1526be..06d894b1 100644
> --- a/libstore/mvol.c
> +++ b/libstore/mvol.c
> @@ -70,6 +70,23 @@ mvol_write (struct store *store,
>return err;
>  }
>
> +static error_t
> +mvol_sync (struct store *store)
> +{
> +  size_t i;
> +  error_t err = 0;
> +
> +  /* We iterate through all known volumes to ensure
> +even inactive volumes are flushed to disk. */
> +  for (i = 0; i < store->num_children; i++)
> +{
> +  error_t e = store_sync (store->children[i]);
> +  if (e)
> +   err = e;
> +}
> +  return err;
> +}
> +
>  static error_t
>  mvol_set_size (struct store *store, size_t newsize)
>  {
> @@ -89,7 +106,8 @@ store_mvol_class =
>  {
>-1, "mvol", mvol_read, mvol_write, mvol_set_size,
>0, 0, 0,
> -  store_set_child_flags, store_clear_child_flags, 0, 0, mvol_remap
> +  store_set_child_flags, store_clear_child_flags, 0, 0, mvol_remap,
> +  0, 0, 0, mvol_sync
>  };
>  STORE_STD_CLASS (mvol);
>
> diff --git a/libstore/remap.c b/libstore/remap.c
> index de21812e..54ae31c2 100644
> --- a/libstore/remap.c
> +++ b/libstore/remap.c
> @@ -41,6 +41,12 @@ remap_write (struct store *store,
>return store_write (store->children[0], addr, buf, len, amount);
>  }
>
> +static error_t
> +remap_sync (struct store * store)
> +{
> +  return store_sync (store->children[0]);
> +}
> +
>  static error_t
>  remap_set_size (struct store *store, size_t newsize)
>  {
> @@ -210,7 +216,8 @@ store_remap_class =
>remap_allocate_encoding, remap_encode, remap_decode,
>store_set_child_flags, store_clear_child_flags,
>NULL, NULL, NULL,/* cleanup, clone, remap */
> -  remap_open, remap_validate_name
> +  remap_open, remap_validate_name,
> +  NULL, remap_sync
>  };
>  STORE_STD_CLASS (remap);
>
> diff --git a/libstore/stripe.c b/libstore/stripe.c
> index e9c58466..7e437f86 100644
> --- a/libstore/stripe.c
> +++ b/libstore/stripe.c
> @@ -57,6 +57,24 @@ stripe_write (struct store *store,
>  store_write (stripe, addr_adj (addr, store, stripe), buf, len,
> amount);
>  }
>
> +static error_t
> +stripe_sync (struct store *store)
> +{
> +  size_t i;
> +  error_t err = 0;
> +
> +  for (i = 0; i < store->num_children; i++)
> +{
> +  error_t e = store_sync (store->children[i]);
> +  /* If any child fails, we record the error but continue
> +   syncing the others to maximize data safety. */
> +  if (e)
> +   err = e;
> +}
> +
> +  return err;
> +}
> +
>  error_t
>  stripe_set_size (struct store *store, size_t newsize)
>  {
> @@ -113,7 +131,8 @@ store_ileave_class =
>  {
>STORAGE_INTERLEAVE, "interleave", stripe_read, stripe_write,
> stripe_set_size,
>ileave_allocate_encoding, ileave_encode, ileave_decode,
> -  store_set_child_flags, store_clear_child_flags, 0, 0, stripe_remap
> +  store_set_child_flags, store_clear_child_flags, 0, 0, stripe_remap,
> +  0, 0, 0, stripe_sync
>  };
>  STORE_STD_CLASS (ileave);
>
> --
> 2.52.0
>
>

[PATCH] libstore: add sync function for file, mvol, remap and stripe

2026-01-26 Thread Milos Nikic 
Along side the store_sync this adds sync to other often used stores.
---
 libstore/file.c   |  9 -
 libstore/mvol.c   | 20 +++-
 libstore/remap.c  |  9 -
 libstore/stripe.c | 21 -
 4 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/libstore/file.c b/libstore/file.c
index 80d6812a..5f6d7a46 100644
--- a/libstore/file.c
+++ b/libstore/file.c
@@ -76,6 +76,12 @@ file_write (struct store *store,
   return io_write (store->port, (void *) buf, len, addr * bsize, amount);
 }
 
+static error_t
+file_store_sync (struct store *store)
+{
+  return file_sync (store->port, 1, 0);
+}
+
 static error_t
 file_store_set_size (struct store *store, size_t newsize)
 {
@@ -225,7 +231,8 @@ store_file_class =
 {
   STORAGE_HURD_FILE, "file", file_read, file_write, file_store_set_size,
   store_std_leaf_allocate_encoding, store_std_leaf_encode, file_decode,
-  file_set_flags, file_clear_flags, 0, 0, 0, file_open, 0, file_map
+  file_set_flags, file_clear_flags, 0, 0, 0, file_open, 0, file_map,
+  file_store_sync
 };
 STORE_STD_CLASS (file);
 
diff --git a/libstore/mvol.c b/libstore/mvol.c
index ee1526be..06d894b1 100644
--- a/libstore/mvol.c
+++ b/libstore/mvol.c
@@ -70,6 +70,23 @@ mvol_write (struct store *store,
   return err;
 }
 
+static error_t
+mvol_sync (struct store *store)
+{
+  size_t i;
+  error_t err = 0;
+
+  /* We iterate through all known volumes to ensure
+even inactive volumes are flushed to disk. */
+  for (i = 0; i < store->num_children; i++)
+{
+  error_t e = store_sync (store->children[i]);
+  if (e)
+   err = e;
+}
+  return err;
+}
+
 static error_t
 mvol_set_size (struct store *store, size_t newsize)
 {
@@ -89,7 +106,8 @@ store_mvol_class =
 {
   -1, "mvol", mvol_read, mvol_write, mvol_set_size,
   0, 0, 0,
-  store_set_child_flags, store_clear_child_flags, 0, 0, mvol_remap
+  store_set_child_flags, store_clear_child_flags, 0, 0, mvol_remap,
+  0, 0, 0, mvol_sync
 };
 STORE_STD_CLASS (mvol);
 
diff --git a/libstore/remap.c b/libstore/remap.c
index de21812e..54ae31c2 100644
--- a/libstore/remap.c
+++ b/libstore/remap.c
@@ -41,6 +41,12 @@ remap_write (struct store *store,
   return store_write (store->children[0], addr, buf, len, amount);
 }
 
+static error_t
+remap_sync (struct store * store)
+{
+  return store_sync (store->children[0]);
+}
+
 static error_t
 remap_set_size (struct store *store, size_t newsize)
 {
@@ -210,7 +216,8 @@ store_remap_class =
   remap_allocate_encoding, remap_encode, remap_decode,
   store_set_child_flags, store_clear_child_flags,
   NULL, NULL, NULL,/* cleanup, clone, remap */
-  remap_open, remap_validate_name
+  remap_open, remap_validate_name,
+  NULL, remap_sync
 };
 STORE_STD_CLASS (remap);
 
diff --git a/libstore/stripe.c b/libstore/stripe.c
index e9c58466..7e437f86 100644
--- a/libstore/stripe.c
+++ b/libstore/stripe.c
@@ -57,6 +57,24 @@ stripe_write (struct store *store,
 store_write (stripe, addr_adj (addr, store, stripe), buf, len, amount);
 }
 
+static error_t
+stripe_sync (struct store *store)
+{
+  size_t i;
+  error_t err = 0;
+
+  for (i = 0; i < store->num_children; i++)
+{
+  error_t e = store_sync (store->children[i]);
+  /* If any child fails, we record the error but continue
+   syncing the others to maximize data safety. */
+  if (e)
+   err = e;
+}
+
+  return err;
+}
+
 error_t
 stripe_set_size (struct store *store, size_t newsize)
 {
@@ -113,7 +131,8 @@ store_ileave_class =
 {
   STORAGE_INTERLEAVE, "interleave", stripe_read, stripe_write, stripe_set_size,
   ileave_allocate_encoding, ileave_encode, ileave_decode,
-  store_set_child_flags, store_clear_child_flags, 0, 0, stripe_remap
+  store_set_child_flags, store_clear_child_flags, 0, 0, stripe_remap,
+  0, 0, 0, stripe_sync
 };
 STORE_STD_CLASS (ileave);
 
-- 
2.52.0