* [PATCH 0/1] -----BEGIN SSH SIGNATURE----- @ 2026-05-31 10:34 Christian Hergert 2026-05-31 10:34 ` [PATCH 1/1] block/vvfat: defer write commits to half-second timer Christian Hergert 0 siblings, 1 reply; 4+ messages in thread From: Christian Hergert @ 2026-05-31 10:34 UTC (permalink / raw) To: qemu-devel; +Cc: Hanna Reitz, qemu-block, Kevin Wolf, Christian Hergert 7J2x6bWnGCPV5Pt6sAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3NzaC1lZDI1NTE5 AAAAQN+I7C3v3pmZpDIZyAgcq5c1+4BDylqpsGbqFBhFpDLXcm/mWrZoJii3GK3Ce+qdGe F1dW+FOyJOYsDar+YVyQM= -----END SSH SIGNATURE----- -----BEGIN SSH SIGNATURE----- U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgn0Noijiu+xEWY4CvDD7yVv6k3T 7J2x6bWnGCPV5Pt6sAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3NzaC1lZDI1NTE5 AAAAQBHUGCB70DC5aAkYJdtqoTAlDS3+N6kv0bmmZ15j9QKKFpx/fN4/JTA3hPxbubdeVr mphBILleJBL2b565llIws= -----END SSH SIGNATURE----- Christian Hergert (1): block/vvfat: defer write commits to half-second timer block/vvfat.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 125 insertions(+), 5 deletions(-) -- 2.54.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/1] block/vvfat: defer write commits to half-second timer 2026-05-31 10:34 [PATCH 0/1] -----BEGIN SSH SIGNATURE----- Christian Hergert @ 2026-05-31 10:34 ` Christian Hergert 2026-06-23 11:22 ` Daniel P. Berrangé 0 siblings, 1 reply; 4+ messages in thread From: Christian Hergert @ 2026-05-31 10:34 UTC (permalink / raw) To: qemu-devel; +Cc: Hanna Reitz, qemu-block, Kevin Wolf, Christian Hergert This keeps consistent reads but defers writes to a half-second timer by implementing the suggested TODO in block/vvfat.c. I noticed slow boot performance when testing a UKI image. It was roughly 18 seconds to a login prompt with a simple userspace. I profiled quickly with Sysprof which showed an overwhelming amount of time spent in check_directory_consistency(). The -drive in question here is the following: file=fat:rw:/tmp/machines-uki-esp-HVCXP3,format=raw,media=disk which has a temporary NvVars in it for quick throwaway VM testing which I use in conjunction with mkosi. With this commit in place, boot to login reduces from 18 seconds to 6. Signed-off-by: Christian Hergert <christian@sourceandstack.com> --- block/vvfat.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 125 insertions(+), 5 deletions(-) diff --git a/block/vvfat.c b/block/vvfat.c index e334b9febb..49756ac5e0 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -32,6 +32,7 @@ #include "block/qdict.h" #include "qemu/module.h" #include "qemu/option.h" +#include "qemu/timer.h" #include "qemu/bswap.h" #include "migration/blocker.h" #include "qobject/qdict.h" @@ -53,8 +54,6 @@ Note that DOS assumes the system files to be the first files in the file system (test if the boot sector still relies on that fact)! */ /* MAYBE TODO: write block-visofs.c */ -/* TODO: call try_commit() only after a timeout */ - /* #define DEBUG */ #ifdef DEBUG @@ -80,6 +79,8 @@ static void checkpoint(void); #define DIR_KANJI_FAKE 0x05 #define DIR_FREE 0x00 +#define VVFAT_COMMIT_DELAY_NS (NANOSECONDS_PER_SECOND / 2) + /* dynamic array functions */ typedef struct array_t { char* pointer; @@ -332,6 +333,8 @@ typedef struct BDRVVVFATState { void* fat2; char* used_clusters; array_t commits; + QEMUTimer *commit_timer; + bool commit_pending; const char* path; int downcase_short_names; @@ -1052,6 +1055,8 @@ static BDRVVVFATState *vvv = NULL; static int enable_write_target(BlockDriverState *bs, Error **errp); static int coroutine_fn is_consistent(BDRVVVFATState *s); +static void vvfat_attach_aio_context(BlockDriverState *bs, + AioContext *new_context); static QemuOptsList runtime_opts = { .name = "vvfat", @@ -1278,6 +1283,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags, } qemu_co_mutex_init(&s->lock); + vvfat_attach_aio_context(bs, bdrv_get_aio_context(bs)); qemu_opts_del(opts); @@ -2973,6 +2979,60 @@ DLOG(checkpoint()); return do_commit(s); } +static int coroutine_fn GRAPH_RDLOCK +vvfat_commit_now(BDRVVVFATState *s) +{ + int ret; + + if (s->commit_timer) { + timer_del(s->commit_timer); + } + if (!s->commit_pending) { + return 0; + } + + qemu_co_mutex_lock(&s->lock); + ret = try_commit(s); + qemu_co_mutex_unlock(&s->lock); + + if (ret == 0) { + s->commit_pending = false; + } + + return ret; +} + +static void coroutine_fn vvfat_commit_timer_entry(void *opaque) +{ + BlockDriverState *bs = opaque; + BDRVVVFATState *s = bs->opaque; + GRAPH_RDLOCK_GUARD(); + + vvfat_commit_now(s); + bdrv_dec_in_flight(bs); +} + +static void vvfat_commit_timer_cb(void *opaque) +{ + BlockDriverState *bs = opaque; + Coroutine *co; + + co = qemu_coroutine_create(vvfat_commit_timer_entry, bs); + bdrv_inc_in_flight(bs); + qemu_coroutine_enter(co); +} + +static void vvfat_schedule_commit(BDRVVVFATState *s) +{ + s->commit_pending = true; + + if (s->commit_timer) { + timer_mod_ns(s->commit_timer, + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + + VVFAT_COMMIT_DELAY_NS); + } +} + static int coroutine_fn GRAPH_RDLOCK vvfat_write(BlockDriverState *bs, int64_t sector_num, const uint8_t *buf, int nb_sectors) @@ -3098,8 +3158,7 @@ DLOG(fprintf(stderr, "Write to qcow backend: %d + %d\n", (int)sector_num, nb_sec } DLOG(checkpoint()); - /* TODO: add timeout */ - try_commit(s); + vvfat_schedule_commit(s); DLOG(checkpoint()); return 0; @@ -3133,6 +3192,34 @@ vvfat_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes, return ret; } +static int coroutine_fn GRAPH_RDLOCK +vvfat_co_flush(BlockDriverState *bs) +{ + BDRVVVFATState *s = bs->opaque; + + if (s->qcow == NULL) { + return 0; + } + + return vvfat_commit_now(s); +} + +static void vvfat_drain_begin(BlockDriverState *bs) +{ + BDRVVVFATState *s = bs->opaque; + Coroutine *co; + + if (!s->commit_timer || !timer_pending(s->commit_timer)) { + return; + } + + timer_del(s->commit_timer); + + co = qemu_coroutine_create(vvfat_commit_timer_entry, bs); + bdrv_inc_in_flight(bs); + aio_co_enter(bdrv_get_aio_context(bs), co); +} + static int coroutine_fn vvfat_co_block_status(BlockDriverState *bs, unsigned int mode, int64_t offset, int64_t bytes, @@ -3229,6 +3316,11 @@ static void vvfat_close(BlockDriverState *bs) { BDRVVVFATState *s = bs->opaque; + if (s->commit_timer) { + timer_free(s->commit_timer); + s->commit_timer = NULL; + } + vvfat_close_current_file(s); array_free(&(s->fat)); array_free(&(s->directory)); @@ -3240,6 +3332,30 @@ static void vvfat_close(BlockDriverState *bs) } } +static void vvfat_detach_aio_context(BlockDriverState *bs) +{ + BDRVVVFATState *s = bs->opaque; + + if (s->commit_timer) { + timer_free(s->commit_timer); + s->commit_timer = NULL; + } +} + +static void vvfat_attach_aio_context(BlockDriverState *bs, + AioContext *new_context) +{ + BDRVVVFATState *s = bs->opaque; + + if (s->qcow) { + s->commit_timer = aio_timer_new(new_context, QEMU_CLOCK_VIRTUAL, + SCALE_NS, vvfat_commit_timer_cb, bs); + if (s->commit_pending) { + vvfat_schedule_commit(s); + } + } +} + static const char *const vvfat_strong_runtime_opts[] = { "dir", "fat-type", @@ -3263,7 +3379,11 @@ static BlockDriver bdrv_vvfat = { .bdrv_co_preadv = vvfat_co_preadv, .bdrv_co_pwritev = vvfat_co_pwritev, - .bdrv_co_block_status = vvfat_co_block_status, + .bdrv_co_flush = vvfat_co_flush, + .bdrv_drain_begin = vvfat_drain_begin, + .bdrv_co_block_status = vvfat_co_block_status, + .bdrv_detach_aio_context = vvfat_detach_aio_context, + .bdrv_attach_aio_context = vvfat_attach_aio_context, .strong_runtime_opts = vvfat_strong_runtime_opts, }; -- 2.54.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] block/vvfat: defer write commits to half-second timer 2026-05-31 10:34 ` [PATCH 1/1] block/vvfat: defer write commits to half-second timer Christian Hergert @ 2026-06-23 11:22 ` Daniel P. Berrangé 0 siblings, 0 replies; 4+ messages in thread From: Daniel P. Berrangé @ 2026-06-23 11:22 UTC (permalink / raw) To: Christian Hergert; +Cc: qemu-devel, Hanna Reitz, qemu-block, Kevin Wolf On Sun, May 31, 2026 at 12:34:37PM +0200, Christian Hergert wrote: > This keeps consistent reads but defers writes to a half-second timer by > implementing the suggested TODO in block/vvfat.c. > > I noticed slow boot performance when testing a UKI image. It was roughly > 18 seconds to a login prompt with a simple userspace. I profiled quickly > with Sysprof which showed an overwhelming amount of time spent in > check_directory_consistency(). > > The -drive in question here is the following: > > file=fat:rw:/tmp/machines-uki-esp-HVCXP3,format=raw,media=disk > > which has a temporary NvVars in it for quick throwaway VM testing which > I use in conjunction with mkosi. By NvVars are you referring to a NVRAM backing store for use with EDK2 ? If so, then newer EDK2 supports a JSON backing store which would be a better bet for quick & easy VM testing IMHO. Not an objection to this patch, but we tend to recommend that any use of vvfat is best avoided where any other alternative is available. > > With this commit in place, boot to login reduces from 18 seconds to 6. > > Signed-off-by: Christian Hergert <christian@sourceandstack.com> > --- > block/vvfat.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 125 insertions(+), 5 deletions(-) FYI, applying this patch appears to break the I/O tests for vvfat: $ ./build/run ./build/tests/qemu-iotests/check -vvfat vvfat QEMU -- "/home/berrange/src/virt/qemu/build/qemu-system-x86_64" -nodefaults -display none -accel qtest QEMU_IMG -- "/home/berrange/src/virt/qemu/build/qemu-img" QEMU_IO -- "/home/berrange/src/virt/qemu/build/qemu-io" --cache writeback --aio threads -f vvfat QEMU_NBD -- "/home/berrange/src/virt/qemu/build/qemu-nbd" IMGFMT -- vvfat IMGPROTO -- file PLATFORM -- Linux/x86_64 berrange.csb 7.0.9-205.fc44.x86_64 TEST_DIR -- /home/berrange/src/virt/qemu/scratch SOCK_DIR -- /tmp/qemu-iotests-4_wjh3ml GDB_OPTIONS -- VALGRIND_QEMU -- PRINT_QEMU_OUTPUT -- vvfat fail [12:16:35] [12:16:39] 3.3s (last: 3.2s) failed, exit status 1 --- /home/berrange/src/virt/qemu/tests/qemu-iotests/tests/vvfat.out +++ /home/berrange/src/virt/qemu/scratch/vvfat-file-vvfat/vvfat.out.bad @@ -1,5 +1,105 @@ -................ +E..F..FFFFFFF..F +====================================================================== +ERROR: test_create_file (__main__.TestVVFatDriver.test_create_file) +Test creating a new file +---------------------------------------------------------------------- +Traceback (most recent call last): + File "/home/berrange/src/virt/qemu/tests/qemu-iotests/tests/vvfat", line 477, in test_create_file + with open(os.path.join(filesystem, "newfile.txt"), "rb") as f: + ~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +FileNotFoundError: [Errno 2] No such file or directory: '/home/berrange/src/virt/qemu/scratch/vvfat-file-vvfat/filesystem/newfile.txt' + +====================================================================== +FAIL: test_modify_content_same_clusters (__main__.TestVVFatDriver.test_modify_content_same_clusters) +Test modifying the content of the file without changing the number of +---------------------------------------------------------------------- +Traceback (most recent call last): + File "/home/berrange/src/virt/qemu/tests/qemu-iotests/tests/vvfat", line 300, in test_modify_content_same_clusters + self.assertEqual(f.read(), new_content) + ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^ +AssertionError: b'Hello, world! 0\n' != b'Hello, world! Modified\n' + +====================================================================== +FAIL: test_truncate_file_change_clusters_less (__main__.TestVVFatDriver.test_truncate_file_change_clusters_less) +Test truncating a file by reducing the number of clusters +---------------------------------------------------------------------- +Traceback (most recent call last): + File "/home/berrange/src/virt/qemu/tests/qemu-iotests/tests/vvfat", line 377, in test_truncate_file_change_clusters_less + self.assertEqual(f.read(), b"A") + ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^ +AssertionError: b'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA[16338 chars]PPPP' != b'A' + +====================================================================== +FAIL: test_truncate_file_same_clusters_less (__main__.TestVVFatDriver.test_truncate_file_same_clusters_less) +Test truncating the file without changing number of clusters +---------------------------------------------------------------------- +Traceback (most recent call last): + File "/home/berrange/src/virt/qemu/tests/qemu-iotests/tests/vvfat", line 319, in test_truncate_file_same_clusters_less + self.assertEqual(f.read(), new_content) + ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^ +AssertionError: b'Hello, world! 0\n' != b'Hello' + +====================================================================== +FAIL: test_truncate_file_same_clusters_more (__main__.TestVVFatDriver.test_truncate_file_same_clusters_more) +Test truncating the file without changing number of clusters +---------------------------------------------------------------------- +Traceback (most recent call last): + File "/home/berrange/src/virt/qemu/tests/qemu-iotests/tests/vvfat", line 343, in test_truncate_file_same_clusters_more + self.assertEqual(f.read(), new_content) + ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^ +AssertionError: b'Hello, world! 0\n' != b'Hello, world! 0\n((((' + +====================================================================== +FAIL: test_write_file_change_clusters_less (__main__.TestVVFatDriver.test_write_file_change_clusters_less) +Test truncating a file by reducing the number of clusters +---------------------------------------------------------------------- +Traceback (most recent call last): + File "/home/berrange/src/virt/qemu/tests/qemu-iotests/tests/vvfat", line 393, in test_write_file_change_clusters_less + self.assertEqual(f.read(), new_content) + ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^ +AssertionError: b'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA[24531 chars]XXXX' != b'XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX[16339 chars]YYYY' + +====================================================================== +FAIL: test_write_file_change_clusters_more (__main__.TestVVFatDriver.test_write_file_change_clusters_more) +Test truncating a file by increasing the number of clusters +---------------------------------------------------------------------- +Traceback (most recent call last): + File "/home/berrange/src/virt/qemu/tests/qemu-iotests/tests/vvfat", line 415, in test_write_file_change_clusters_more + self.assertEqual(f.read(), new_content) + ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^ +AssertionError: b'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA[24531 chars]XXXX' != b'WWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWW[32723 chars]ZZZZ' + +====================================================================== +FAIL: test_write_file_change_clusters_more_non_contiguous_2_mappings (__main__.TestVVFatDriver.test_write_file_change_clusters_more_non_contiguous_2_mappings) +Test truncating a file by increasing the number of clusters Here we +---------------------------------------------------------------------- +Traceback (most recent call last): + File "/home/berrange/src/virt/qemu/tests/qemu-iotests/tests/vvfat", line 435, in test_write_file_change_clusters_more_non_contiguous_2_mappings + self.assertEqual(f.read(), new_content) + ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^ +AssertionError: b'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA[16339 chars]PPPP' != b'XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX[24531 chars]ZZZZ' + +====================================================================== +FAIL: test_write_file_change_clusters_more_non_contiguous_3_mappings (__main__.TestVVFatDriver.test_write_file_change_clusters_more_non_contiguous_3_mappings) +Test truncating a file by increasing the number of clusters Here we +---------------------------------------------------------------------- +Traceback (most recent call last): + File "/home/berrange/src/virt/qemu/tests/qemu-iotests/tests/vvfat", line 460, in test_write_file_change_clusters_more_non_contiguous_3_mappings + self.assertEqual(f.read(), new_content) + ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^ +AssertionError: b'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA[16339 chars]PPPP' != b'WWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWW[32723 chars]ZZZZ' + +====================================================================== +FAIL: test_write_large_file (__main__.TestVVFatDriver.test_write_large_file) +Test writing a large file +---------------------------------------------------------------------- +Traceback (most recent call last): + File "/home/berrange/src/virt/qemu/tests/qemu-iotests/tests/vvfat", line 362, in test_write_large_file + self.assertEqual(f.read(), new_content) + ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^ +AssertionError: b'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA[16339 chars]PPPP' != b'ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ[16339 chars]KKKK' + ---------------------------------------------------------------------- Ran 16 tests -OK +FAILED (failures=9, errors=1) Failures: vvfat Failed 1 of 1 iotests I didn't investigate whether this is a real functional bug, or whether the tests are making bad assumptions that need fixing, or whether this is just inherant behaviour from delaying writes. > > diff --git a/block/vvfat.c b/block/vvfat.c > index e334b9febb..49756ac5e0 100644 > --- a/block/vvfat.c > +++ b/block/vvfat.c > @@ -32,6 +32,7 @@ > #include "block/qdict.h" > #include "qemu/module.h" > #include "qemu/option.h" > +#include "qemu/timer.h" > #include "qemu/bswap.h" > #include "migration/blocker.h" > #include "qobject/qdict.h" > @@ -53,8 +54,6 @@ > Note that DOS assumes the system files to be the first files in the > file system (test if the boot sector still relies on that fact)! */ > /* MAYBE TODO: write block-visofs.c */ > -/* TODO: call try_commit() only after a timeout */ > - > /* #define DEBUG */ > > #ifdef DEBUG > @@ -80,6 +79,8 @@ static void checkpoint(void); > #define DIR_KANJI_FAKE 0x05 > #define DIR_FREE 0x00 > > +#define VVFAT_COMMIT_DELAY_NS (NANOSECONDS_PER_SECOND / 2) > + > /* dynamic array functions */ > typedef struct array_t { > char* pointer; > @@ -332,6 +333,8 @@ typedef struct BDRVVVFATState { > void* fat2; > char* used_clusters; > array_t commits; > + QEMUTimer *commit_timer; > + bool commit_pending; > const char* path; > int downcase_short_names; > > @@ -1052,6 +1055,8 @@ static BDRVVVFATState *vvv = NULL; > > static int enable_write_target(BlockDriverState *bs, Error **errp); > static int coroutine_fn is_consistent(BDRVVVFATState *s); > +static void vvfat_attach_aio_context(BlockDriverState *bs, > + AioContext *new_context); > > static QemuOptsList runtime_opts = { > .name = "vvfat", > @@ -1278,6 +1283,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags, > } > > qemu_co_mutex_init(&s->lock); > + vvfat_attach_aio_context(bs, bdrv_get_aio_context(bs)); > > qemu_opts_del(opts); > > @@ -2973,6 +2979,60 @@ DLOG(checkpoint()); > return do_commit(s); > } > > +static int coroutine_fn GRAPH_RDLOCK > +vvfat_commit_now(BDRVVVFATState *s) > +{ > + int ret; > + > + if (s->commit_timer) { > + timer_del(s->commit_timer); > + } > + if (!s->commit_pending) { > + return 0; > + } > + > + qemu_co_mutex_lock(&s->lock); > + ret = try_commit(s); > + qemu_co_mutex_unlock(&s->lock); > + > + if (ret == 0) { > + s->commit_pending = false; > + } > + > + return ret; > +} > + > +static void coroutine_fn vvfat_commit_timer_entry(void *opaque) > +{ > + BlockDriverState *bs = opaque; > + BDRVVVFATState *s = bs->opaque; > + GRAPH_RDLOCK_GUARD(); > + > + vvfat_commit_now(s); > + bdrv_dec_in_flight(bs); > +} > + > +static void vvfat_commit_timer_cb(void *opaque) > +{ > + BlockDriverState *bs = opaque; > + Coroutine *co; > + > + co = qemu_coroutine_create(vvfat_commit_timer_entry, bs); > + bdrv_inc_in_flight(bs); > + qemu_coroutine_enter(co); > +} > + > +static void vvfat_schedule_commit(BDRVVVFATState *s) > +{ > + s->commit_pending = true; > + > + if (s->commit_timer) { > + timer_mod_ns(s->commit_timer, > + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + > + VVFAT_COMMIT_DELAY_NS); > + } > +} > + > static int coroutine_fn GRAPH_RDLOCK > vvfat_write(BlockDriverState *bs, int64_t sector_num, > const uint8_t *buf, int nb_sectors) > @@ -3098,8 +3158,7 @@ DLOG(fprintf(stderr, "Write to qcow backend: %d + %d\n", (int)sector_num, nb_sec > } > > DLOG(checkpoint()); > - /* TODO: add timeout */ > - try_commit(s); > + vvfat_schedule_commit(s); > > DLOG(checkpoint()); > return 0; > @@ -3133,6 +3192,34 @@ vvfat_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes, > return ret; > } > > +static int coroutine_fn GRAPH_RDLOCK > +vvfat_co_flush(BlockDriverState *bs) > +{ > + BDRVVVFATState *s = bs->opaque; > + > + if (s->qcow == NULL) { > + return 0; > + } > + > + return vvfat_commit_now(s); > +} > + > +static void vvfat_drain_begin(BlockDriverState *bs) > +{ > + BDRVVVFATState *s = bs->opaque; > + Coroutine *co; > + > + if (!s->commit_timer || !timer_pending(s->commit_timer)) { > + return; > + } > + > + timer_del(s->commit_timer); > + > + co = qemu_coroutine_create(vvfat_commit_timer_entry, bs); > + bdrv_inc_in_flight(bs); > + aio_co_enter(bdrv_get_aio_context(bs), co); > +} > + > static int coroutine_fn vvfat_co_block_status(BlockDriverState *bs, > unsigned int mode, > int64_t offset, int64_t bytes, > @@ -3229,6 +3316,11 @@ static void vvfat_close(BlockDriverState *bs) > { > BDRVVVFATState *s = bs->opaque; > > + if (s->commit_timer) { > + timer_free(s->commit_timer); > + s->commit_timer = NULL; > + } > + > vvfat_close_current_file(s); > array_free(&(s->fat)); > array_free(&(s->directory)); > @@ -3240,6 +3332,30 @@ static void vvfat_close(BlockDriverState *bs) > } > } > > +static void vvfat_detach_aio_context(BlockDriverState *bs) > +{ > + BDRVVVFATState *s = bs->opaque; > + > + if (s->commit_timer) { > + timer_free(s->commit_timer); > + s->commit_timer = NULL; > + } > +} > + > +static void vvfat_attach_aio_context(BlockDriverState *bs, > + AioContext *new_context) > +{ > + BDRVVVFATState *s = bs->opaque; > + > + if (s->qcow) { > + s->commit_timer = aio_timer_new(new_context, QEMU_CLOCK_VIRTUAL, > + SCALE_NS, vvfat_commit_timer_cb, bs); > + if (s->commit_pending) { > + vvfat_schedule_commit(s); > + } > + } > +} > + > static const char *const vvfat_strong_runtime_opts[] = { > "dir", > "fat-type", > @@ -3263,7 +3379,11 @@ static BlockDriver bdrv_vvfat = { > > .bdrv_co_preadv = vvfat_co_preadv, > .bdrv_co_pwritev = vvfat_co_pwritev, > - .bdrv_co_block_status = vvfat_co_block_status, > + .bdrv_co_flush = vvfat_co_flush, > + .bdrv_drain_begin = vvfat_drain_begin, > + .bdrv_co_block_status = vvfat_co_block_status, > + .bdrv_detach_aio_context = vvfat_detach_aio_context, > + .bdrv_attach_aio_context = vvfat_attach_aio_context, > > .strong_runtime_opts = vvfat_strong_runtime_opts, > }; > -- > 2.54.0 > > With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :| ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 0/1] [PATCH v2] block/vvfat: defer write commits to half-second timer @ 2026-06-23 12:01 Christian Hergert 2026-06-23 12:01 ` [PATCH 1/1] " Christian Hergert 0 siblings, 1 reply; 4+ messages in thread From: Christian Hergert @ 2026-06-23 12:01 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Hanna Reitz, qemu-block, Christian Hergert Signed-off-by: Christian Hergert <christian@sourceandstack.com> --- - v2: update tests to flush before checking contents in storage -----BEGIN SSH SIGNATURE----- U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgn0Noijiu+xEWY4CvDD7yVv6k3T 7J2x6bWnGCPV5Pt6sAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3NzaC1lZDI1NTE5 AAAAQN+I7C3v3pmZpDIZyAgcq5c1+4BDylqpsGbqFBhFpDLXcm/mWrZoJii3GK3Ce+qdGe F1dW+FOyJOYsDar+YVyQM= -----END SSH SIGNATURE----- -----BEGIN SSH SIGNATURE----- U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgn0Noijiu+xEWY4CvDD7yVv6k3T 7J2x6bWnGCPV5Pt6sAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3NzaC1lZDI1NTE5 AAAAQBHUGCB70DC5aAkYJdtqoTAlDS3+N6kv0bmmZ15j9QKKFpx/fN4/JTA3hPxbubdeVr mphBILleJBL2b565llIws= -----END SSH SIGNATURE----- -----BEGIN SSH SIGNATURE----- U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgn0Noijiu+xEWY4CvDD7yVv6k3T 7J2x6bWnGCPV5Pt6sAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3NzaC1lZDI1NTE5 AAAAQOnadRs4oAAVWKsCySAOhrJaSTW98aeW+2Tqb5WJR56PY5d3qcZk4I3tdAwKK9/R04 vVwWMol8jAblAi/ijK3QQ= -----END SSH SIGNATURE----- Christian Hergert (1): block/vvfat: defer write commits to half-second timer block/vvfat.c | 130 +++++++++++++++++++++++++++++++-- tests/qemu-iotests/tests/vvfat | 15 ++++ 2 files changed, 140 insertions(+), 5 deletions(-) -- 2.54.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/1] block/vvfat: defer write commits to half-second timer 2026-06-23 12:01 [PATCH 0/1] [PATCH v2] " Christian Hergert @ 2026-06-23 12:01 ` Christian Hergert 0 siblings, 0 replies; 4+ messages in thread From: Christian Hergert @ 2026-06-23 12:01 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Hanna Reitz, qemu-block, Christian Hergert This keeps consistent reads but defers writes to a half-second timer by implementing the suggested TODO in block/vvfat.c. I noticed slow boot performance when testing a UKI image. It was roughly 18 seconds to a login prompt with a simple userspace. I profiled quickly with Sysprof which showed an overwhelming amount of time spent in check_directory_consistency(). The -drive in question here is the following: file=fat:rw:/tmp/machines-uki-esp-HVCXP3,format=raw,media=disk which has a temporary NvVars in it for quick throwaway VM testing which I use in conjunction with mkosi. Tests were updated to ensure that we flush before checking stored contents. With this commit in place, boot to login reduces from 18 seconds to 6. Signed-off-by: Christian Hergert <christian@sourceandstack.com> --- block/vvfat.c | 130 +++++++++++++++++++++++++++++++-- tests/qemu-iotests/tests/vvfat | 15 ++++ 2 files changed, 140 insertions(+), 5 deletions(-) diff --git a/block/vvfat.c b/block/vvfat.c index e334b9febb..49756ac5e0 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -32,6 +32,7 @@ #include "block/qdict.h" #include "qemu/module.h" #include "qemu/option.h" +#include "qemu/timer.h" #include "qemu/bswap.h" #include "migration/blocker.h" #include "qobject/qdict.h" @@ -53,8 +54,6 @@ Note that DOS assumes the system files to be the first files in the file system (test if the boot sector still relies on that fact)! */ /* MAYBE TODO: write block-visofs.c */ -/* TODO: call try_commit() only after a timeout */ - /* #define DEBUG */ #ifdef DEBUG @@ -80,6 +79,8 @@ static void checkpoint(void); #define DIR_KANJI_FAKE 0x05 #define DIR_FREE 0x00 +#define VVFAT_COMMIT_DELAY_NS (NANOSECONDS_PER_SECOND / 2) + /* dynamic array functions */ typedef struct array_t { char* pointer; @@ -332,6 +333,8 @@ typedef struct BDRVVVFATState { void* fat2; char* used_clusters; array_t commits; + QEMUTimer *commit_timer; + bool commit_pending; const char* path; int downcase_short_names; @@ -1052,6 +1055,8 @@ static BDRVVVFATState *vvv = NULL; static int enable_write_target(BlockDriverState *bs, Error **errp); static int coroutine_fn is_consistent(BDRVVVFATState *s); +static void vvfat_attach_aio_context(BlockDriverState *bs, + AioContext *new_context); static QemuOptsList runtime_opts = { .name = "vvfat", @@ -1278,6 +1283,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags, } qemu_co_mutex_init(&s->lock); + vvfat_attach_aio_context(bs, bdrv_get_aio_context(bs)); qemu_opts_del(opts); @@ -2973,6 +2979,60 @@ DLOG(checkpoint()); return do_commit(s); } +static int coroutine_fn GRAPH_RDLOCK +vvfat_commit_now(BDRVVVFATState *s) +{ + int ret; + + if (s->commit_timer) { + timer_del(s->commit_timer); + } + if (!s->commit_pending) { + return 0; + } + + qemu_co_mutex_lock(&s->lock); + ret = try_commit(s); + qemu_co_mutex_unlock(&s->lock); + + if (ret == 0) { + s->commit_pending = false; + } + + return ret; +} + +static void coroutine_fn vvfat_commit_timer_entry(void *opaque) +{ + BlockDriverState *bs = opaque; + BDRVVVFATState *s = bs->opaque; + GRAPH_RDLOCK_GUARD(); + + vvfat_commit_now(s); + bdrv_dec_in_flight(bs); +} + +static void vvfat_commit_timer_cb(void *opaque) +{ + BlockDriverState *bs = opaque; + Coroutine *co; + + co = qemu_coroutine_create(vvfat_commit_timer_entry, bs); + bdrv_inc_in_flight(bs); + qemu_coroutine_enter(co); +} + +static void vvfat_schedule_commit(BDRVVVFATState *s) +{ + s->commit_pending = true; + + if (s->commit_timer) { + timer_mod_ns(s->commit_timer, + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + + VVFAT_COMMIT_DELAY_NS); + } +} + static int coroutine_fn GRAPH_RDLOCK vvfat_write(BlockDriverState *bs, int64_t sector_num, const uint8_t *buf, int nb_sectors) @@ -3098,8 +3158,7 @@ DLOG(fprintf(stderr, "Write to qcow backend: %d + %d\n", (int)sector_num, nb_sec } DLOG(checkpoint()); - /* TODO: add timeout */ - try_commit(s); + vvfat_schedule_commit(s); DLOG(checkpoint()); return 0; @@ -3133,6 +3192,34 @@ vvfat_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes, return ret; } +static int coroutine_fn GRAPH_RDLOCK +vvfat_co_flush(BlockDriverState *bs) +{ + BDRVVVFATState *s = bs->opaque; + + if (s->qcow == NULL) { + return 0; + } + + return vvfat_commit_now(s); +} + +static void vvfat_drain_begin(BlockDriverState *bs) +{ + BDRVVVFATState *s = bs->opaque; + Coroutine *co; + + if (!s->commit_timer || !timer_pending(s->commit_timer)) { + return; + } + + timer_del(s->commit_timer); + + co = qemu_coroutine_create(vvfat_commit_timer_entry, bs); + bdrv_inc_in_flight(bs); + aio_co_enter(bdrv_get_aio_context(bs), co); +} + static int coroutine_fn vvfat_co_block_status(BlockDriverState *bs, unsigned int mode, int64_t offset, int64_t bytes, @@ -3229,6 +3316,11 @@ static void vvfat_close(BlockDriverState *bs) { BDRVVVFATState *s = bs->opaque; + if (s->commit_timer) { + timer_free(s->commit_timer); + s->commit_timer = NULL; + } + vvfat_close_current_file(s); array_free(&(s->fat)); array_free(&(s->directory)); @@ -3240,6 +3332,30 @@ static void vvfat_close(BlockDriverState *bs) } } +static void vvfat_detach_aio_context(BlockDriverState *bs) +{ + BDRVVVFATState *s = bs->opaque; + + if (s->commit_timer) { + timer_free(s->commit_timer); + s->commit_timer = NULL; + } +} + +static void vvfat_attach_aio_context(BlockDriverState *bs, + AioContext *new_context) +{ + BDRVVVFATState *s = bs->opaque; + + if (s->qcow) { + s->commit_timer = aio_timer_new(new_context, QEMU_CLOCK_VIRTUAL, + SCALE_NS, vvfat_commit_timer_cb, bs); + if (s->commit_pending) { + vvfat_schedule_commit(s); + } + } +} + static const char *const vvfat_strong_runtime_opts[] = { "dir", "fat-type", @@ -3263,7 +3379,11 @@ static BlockDriver bdrv_vvfat = { .bdrv_co_preadv = vvfat_co_preadv, .bdrv_co_pwritev = vvfat_co_pwritev, - .bdrv_co_block_status = vvfat_co_block_status, + .bdrv_co_flush = vvfat_co_flush, + .bdrv_drain_begin = vvfat_drain_begin, + .bdrv_co_block_status = vvfat_co_block_status, + .bdrv_detach_aio_context = vvfat_detach_aio_context, + .bdrv_attach_aio_context = vvfat_attach_aio_context, .strong_runtime_opts = vvfat_strong_runtime_opts, }; diff --git a/tests/qemu-iotests/tests/vvfat b/tests/qemu-iotests/tests/vvfat index acdc6ce8ff..1f67b4b200 100755 --- a/tests/qemu-iotests/tests/vvfat +++ b/tests/qemu-iotests/tests/vvfat @@ -148,6 +148,9 @@ class TestVVFatDriver(QMPTestCase): os.remove(temp_file) + def flush(self) -> None: + self.qio.cmd("flush") + def init_fat16(self): mbr = MBR(self.read_sectors(0)) return Fat16( @@ -260,6 +263,7 @@ class TestVVFatDriver(QMPTestCase): data = fat16.read_cluster(file.cluster) fat16.write_cluster(file.cluster, data) + self.flush() with open(os.path.join(filesystem, "file0.txt"), "rb") as f: self.assertEqual(fat16.read_file(file), f.read()) @@ -277,6 +281,7 @@ class TestVVFatDriver(QMPTestCase): fat16.write_file(file, b"Hello, world! 0\n") self.assertEqual(fat16.read_file(file), b"Hello, world! 0\n") + self.flush() with open(os.path.join(filesystem, "file0.txt"), "rb") as f: self.assertEqual(f.read(), b"Hello, world! 0\n") @@ -296,6 +301,7 @@ class TestVVFatDriver(QMPTestCase): fat16.write_file(file, new_content) self.assertEqual(fat16.read_file(file), new_content) + self.flush() with open(os.path.join(filesystem, "file0.txt"), "rb") as f: self.assertEqual(f.read(), new_content) @@ -315,6 +321,7 @@ class TestVVFatDriver(QMPTestCase): new_content = fat16.read_file(file) self.assertEqual(new_content, b"Hello") + self.flush() with open(os.path.join(filesystem, "file0.txt"), "rb") as f: self.assertEqual(f.read(), new_content) @@ -339,6 +346,7 @@ class TestVVFatDriver(QMPTestCase): self.assertEqual(new_content[:16], b"Hello, world! 0\n") self.assertEqual(len(new_content), 20) + self.flush() with open(os.path.join(filesystem, "file0.txt"), "rb") as f: self.assertEqual(f.read(), new_content) @@ -358,6 +366,7 @@ class TestVVFatDriver(QMPTestCase): fat16.write_file(file, new_content) self.assertEqual(fat16.read_file(file), new_content) + self.flush() with open(os.path.join(filesystem, "large1.txt"), "rb") as f: self.assertEqual(f.read(), new_content) @@ -373,6 +382,7 @@ class TestVVFatDriver(QMPTestCase): fat16.truncate_file(file, 1) self.assertEqual(fat16.read_file(file), b"A") + self.flush() with open(os.path.join(filesystem, "large1.txt"), "rb") as f: self.assertEqual(f.read(), b"A") @@ -389,6 +399,7 @@ class TestVVFatDriver(QMPTestCase): fat16.write_file(file, new_content) self.assertEqual(fat16.read_file(file), new_content) + self.flush() with open(os.path.join(filesystem, "large2.txt"), "rb") as f: self.assertEqual(f.read(), new_content) @@ -411,6 +422,7 @@ class TestVVFatDriver(QMPTestCase): fat16.write_file(file, new_content) self.assertEqual(fat16.read_file(file), new_content) + self.flush() with open(os.path.join(filesystem, "large2.txt"), "rb") as f: self.assertEqual(f.read(), new_content) @@ -431,6 +443,7 @@ class TestVVFatDriver(QMPTestCase): fat16.write_file(file, new_content) self.assertEqual(fat16.read_file(file), new_content) + self.flush() with open(os.path.join(filesystem, "large1.txt"), "rb") as f: self.assertEqual(f.read(), new_content) @@ -456,6 +469,7 @@ class TestVVFatDriver(QMPTestCase): fat16.write_file(file, new_content) self.assertEqual(fat16.read_file(file), new_content) + self.flush() with open(os.path.join(filesystem, "large1.txt"), "rb") as f: self.assertEqual(f.read(), new_content) @@ -474,6 +488,7 @@ class TestVVFatDriver(QMPTestCase): fat16.write_file(new_file, new_content) self.assertEqual(fat16.read_file(new_file), new_content) + self.flush() with open(os.path.join(filesystem, "newfile.txt"), "rb") as f: self.assertEqual(f.read(), new_content) -- 2.54.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-06-23 12:02 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-31 10:34 [PATCH 0/1] -----BEGIN SSH SIGNATURE----- Christian Hergert 2026-05-31 10:34 ` [PATCH 1/1] block/vvfat: defer write commits to half-second timer Christian Hergert 2026-06-23 11:22 ` Daniel P. Berrangé -- strict thread matches above, loose matches on Subject: below -- 2026-06-23 12:01 [PATCH 0/1] [PATCH v2] " Christian Hergert 2026-06-23 12:01 ` [PATCH 1/1] " Christian Hergert
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.