Re: [PATCH 4/7] reftable/stack: stop using `fsync_component()` directly
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Nov 8, 2024, 02:09 UTC
- Message-ID
- <6ghxx5fmuzujegducuva77vpybihz6b5cnk75wdgv4pv2knac5@m3trfmg2dk5s>
- In-Reply-To
- <86269fc4fcad9a97709aa0d080c4c077a85ca667.1729677003.git.ps@pks.im>
On 24/10/23 11:56AM, Patrick Steinhardt wrote: [snip]
Show 29 quoted lines
> diff --git a/reftable/stack.c b/reftable/stack.c
> index 9ae716ff375..df4f3237007 100644
> --- a/reftable/stack.c
> +++ b/reftable/stack.c
> @@ -8,7 +8,6 @@ license that can be found in the LICENSE file or at
>
> #include "stack.h"
>
> -#include "../write-or-die.h"
> #include "system.h"
> #include "constants.h"
> #include "merged.h"
> @@ -43,17 +42,28 @@ static int stack_filename(struct reftable_buf *dest, struct reftable_stack *st,
> return 0;
> }
>
> -static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz)
> +static int stack_fsync(struct reftable_stack *st, int fd)
> {
> - int *fdp = (int *)arg;
> - return write_in_full(*fdp, data, sz);
> + if (st->opts.fsync)
> + return st->opts.fsync(fd);
> + return fsync(fd);
> }
>
> -static int reftable_fd_flush(void *arg)
> +struct fd_writer {
> + struct reftable_stack *stack;Out of curiousity, from the stack I think we only need the callback in the options. Any reason we provide the whole stack here?
Show 11 quoted lines
> + int fd;
> +};
> +
> +static ssize_t fd_writer_write(void *arg, const void *data, size_t sz)
> {
> - int *fdp = (int *)arg;
> + struct fd_writer *writer = arg;
> + return write_in_full(writer->fd, data, sz);
> +}
>
> - return fsync_component(FSYNC_COMPONENT_REFERENCE, *fdp);Previously when the writer was flushed it would invoke `fsync_component()`. Now that a callback can be configured in the stack options, the callback needs to also be propagated to `fd_writer_write()` in addition to the file descriptor being synced. This explains why `fd_writer` is now used.
The rest of the patch updates writer configuration and fsync call sites. Looks good.
-Justin