Re: [PATCH V12 05/15] block/export: track IOThread references

Stefan Hajnoczi <[email protected]>
Newsgroups gmane.comp.emulators.qemu
Message-ID <20260818204938.GB222512@fedora>
On Sat, Aug 15, 2026 at 01:48:19AM +0800, Zhang Chen wrote:
> Track the IOThreads used by a block export and identify the holder
> with the unique BlockExportOptions id.
> 
> Acquire holder-aware references during export creation and release
> them on error or export deletion.  Support both single- and
> multi-iothread exports.
> 
> Signed-off-by: Zhang Chen <[email protected]>
> ---
>  block/export/export.c  | 79 +++++++++++++++++++++++++++++++++++++-----
>  include/block/export.h |  6 ++++
>  2 files changed, 76 insertions(+), 9 deletions(-)
> 
> diff --git a/block/export/export.c b/block/export/export.c
> index b733f269f3..09136cda34 100644
> --- a/block/export/export.c
> +++ b/block/export/export.c
> @@ -15,7 +15,6 @@
>  
>  #include "block/block.h"
>  #include "system/block-backend.h"
> -#include "system/iothread.h"
>  #include "block/export.h"
>  #include "block/fuse.h"
>  #include "block/nbd.h"
> @@ -85,6 +84,8 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
>      AioContext *ctx;
>      AioContext **multithread_ctxs = NULL;
>      size_t multithread_count = 0;
> +    g_autofree IOThread **local_iothreads = NULL;
> +    char *holder_id = NULL;
>      uint64_t perm;
>      int ret;
>  
> @@ -139,7 +140,16 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
>              goto fail;
>          }
>  
> -        new_ctx = iothread_get_aio_context(iothread);
> +        holder_id = export->id;
> +        const IOThreadHolder holder = {
> +            .type = IO_THREAD_HOLDER_KIND_BLOCK_EXPORT,
> +            .u.block_export.export_id = holder_id,
> +        };
> +
> +        new_ctx = iothread_ref_and_get_aio_context(iothread, &holder);
> +        multithread_count = 1;
> +        local_iothreads = g_new0(IOThread *, 1);
> +        local_iothreads[0] = iothread;
>  
>          /* Ignore errors with fixed-iothread=false */
>          set_context_errp = fixed_iothread ? errp : NULL;
> @@ -163,8 +173,15 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
>              return NULL;
>          }
>  
> +        local_iothreads = g_new0(IOThread *, multithread_count);
>          multithread_ctxs = g_new(AioContext *, multithread_count);
>          i = 0;
> +        holder_id = export->id;
> +        const IOThreadHolder holder = {
> +            .type = IO_THREAD_HOLDER_KIND_BLOCK_EXPORT,
> +            .u.block_export.export_id = holder_id,
> +        };
> +
>          for (strList *e = iothread_list; e; e = e->next) {
>              IOThread *iothread = iothread_by_id(e->value);
>  
> @@ -172,7 +189,9 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
>                  error_setg(errp, "iothread \"%s\" not found", e->value);
>                  goto fail;
>              }
> -            multithread_ctxs[i++] = iothread_get_aio_context(iothread);
> +            local_iothreads[i] = iothread;
> +            multithread_ctxs[i++] = iothread_ref_and_get_aio_context(iothread,
> +                                                                     &holder);
>          }
>          assert(i == multithread_count);
>      }
> @@ -225,12 +244,15 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
>      assert(drv->instance_size >= sizeof(BlockExport));
>      exp = g_malloc0(drv->instance_size);
>      *exp = (BlockExport) {
> -        .drv        = drv,
> -        .refcount   = 1,
> -        .user_owned = true,
> -        .id         = g_strdup(export->id),
> -        .ctx        = ctx,
> -        .blk        = blk,
> +        .drv                  = drv,
> +        .refcount             = 1,
> +        .user_owned           = true,
> +        .id                   = g_strdup(export->id),
> +        .ctx                  = ctx,
> +        .blk                  = blk,
> +        .iothreads            = g_steal_pointer(&local_iothreads),
> +        .iothread_count       = multithread_count,
> +        .iothread_holder_id   = g_strdup(holder_id),

Is this field necessary, it always seems to be the same as exp->id?

>      };
>  
>      ret = drv->create(exp, export, multithread_ctxs, multithread_count, errp);
> @@ -250,8 +272,33 @@ fail:
>          blk_unref(blk);
>      }
>      if (exp) {
> +        if (exp->iothreads) {
> +            const IOThreadHolder holder = {
> +                .type = IO_THREAD_HOLDER_KIND_BLOCK_EXPORT,
> +                .u.block_export.export_id = exp->iothread_holder_id,
> +            };
> +            for (size_t j = 0; j < exp->iothread_count; j++) {
> +                if (exp->iothreads[j]) {
> +                    iothread_unref_and_put_aio_context(exp->iothreads[j],
> +                                                       &holder);
> +                }
> +            }
> +            g_free(exp->iothreads);
> +        }
> +        g_free(exp->iothread_holder_id);
>          g_free(exp->id);
>          g_free(exp);
> +    } else if (local_iothreads) {
> +        const IOThreadHolder holder = {
> +            .type = IO_THREAD_HOLDER_KIND_BLOCK_EXPORT,
> +            .u.block_export.export_id = holder_id,
> +        };
> +
> +        for (size_t j = 0; j < multithread_count; j++) {
> +            if (local_iothreads[j]) {
> +                iothread_unref_and_put_aio_context(local_iothreads[j], &holder);
> +            }
> +        }
>      }
>      g_free(multithread_ctxs);
>      return NULL;
> @@ -273,6 +320,20 @@ static void blk_exp_delete_bh(void *opaque)
>      exp->drv->delete(exp);
>      blk_set_dev_ops(exp->blk, NULL, NULL);
>      blk_unref(exp->blk);
> +
> +    if (exp->iothreads) {
> +        const IOThreadHolder holder = {
> +            .type = IO_THREAD_HOLDER_KIND_BLOCK_EXPORT,
> +            .u.block_export.export_id = exp->iothread_holder_id,
> +        };
> +
> +        for (size_t i = 0; i < exp->iothread_count; i++) {
> +            iothread_unref_and_put_aio_context(exp->iothreads[i], &holder);
> +        }
> +        g_free(exp->iothreads);
> +    }
> +
> +    g_free(exp->iothread_holder_id);
>      qapi_event_send_block_export_deleted(exp->id);
>      g_free(exp->id);
>      g_free(exp);

There is quite a bit of code duplication with the loops that call
iothread_ref_and_get_aio_context() or
iothread_unref_and_put_aio_context().

Two helper functions could avoid duplication:

  static void init_iothreads(const char *holder_id,
                             IOThread **iothreads,
			     size_t num_iothreads,
                             AioContext **aio_ctxs);

  static void cleanup_iothreads(const char *holder_id,
                                IOThread **iothreads,
				size_t num_iothreads);

> diff --git a/include/block/export.h b/include/block/export.h
> index ca45da928c..a093dea0b6 100644
> --- a/include/block/export.h
> +++ b/include/block/export.h
> @@ -16,6 +16,7 @@
>  
>  #include "qapi/qapi-types-block-export.h"
>  #include "qemu/queue.h"
> +#include "system/iothread.h"
>  
>  typedef struct BlockExport BlockExport;
>  
> @@ -89,6 +90,11 @@ struct BlockExport {
>  
>      /* List entry for block_exports */
>      QLIST_ENTRY(BlockExport) next;
> +
> +    /* The IOThreads utilized by this specific block export */
> +    IOThread **iothreads;
> +    size_t iothread_count;
> +    char *iothread_holder_id;
>  };
>  
>  BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp);
> -- 
> 2.54.0
>
signature.asc (application/pgp-signature, 488 B)
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEhpWov9P5fNqsNXdanKSrs4Grc8gFAmqExWIACgkQnKSrs4Gr
c8jEUwf/T3EPLitquBFcIhRGDbshdM4Z3eW7h6ani0ult1VjrMcRjtHtu2xTRSFa
MRsA731C6KWySSGPWDtjEZNEHWaMMWuNGw9WJUFh/y4J6P49wDRBR/Ka9Kdcjeih
qpmDnY6UwoJFAqPqtqLFsefu+4YENSAT4/zuL2DGA8wif+ijWh2NLeAiOxV3Bw1O
sXoSTpM8PLld3hFjXxi4ZT9uC6xEUvPA9cacqMlIwal6k1Q5lYc2xj6A8qXLcL4z
1jVODDAU74foBmSko0eLlua3rFHo5arHZ0114LdIIcp6tcQOXTr/Uj9O87uVVSnE
h45yv/f3NbKgxXPbhSUvNKzz6DpkMw==
=IDpz
-----END PGP SIGNATURE-----
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.