qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH 3/4] include/sysemu/block-backend: split header into I/O


From: Stefan Hajnoczi
Subject: Re: [RFC PATCH 3/4] include/sysemu/block-backend: split header into I/O and graph API
Date: Mon, 13 Sep 2021 14:27:42 +0100

On Wed, Sep 08, 2021 at 09:10:20AM -0400, Emanuele Giuseppe Esposito wrote:
> diff --git a/include/sysemu/block-backend-graph.h 
> b/include/sysemu/block-backend-graph.h
> new file mode 100644
> index 0000000000..3310987b16
> --- /dev/null
> +++ b/include/sysemu/block-backend-graph.h
> @@ -0,0 +1,132 @@
> +/*
> + * QEMU Block backends
> + *
> + * Copyright (C) 2014-2016 Red Hat, Inc.
> + *
> + * Authors:
> + *  Markus Armbruster <armbru@redhat.com>,
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1
> + * or later.  See the COPYING.LIB file in the top-level directory.
> + */
> +
> +#ifndef BLOCK_BACKEND_GRAPH_H
> +#define BLOCK_BACKEND_GRAPH_H
> +
> +#include "block-backend-common.h"
> +
> +/*
> + * Graph API. These functions run under the BQL lock.
> + *
> + * If a function modifies the graph, it uses drain and/or
> + * aio_context_acquire/release to be sure it has unique access.

It's not obvious why additional locking is needed if the BQL is already
held. Please refer to the thread-safe I/O API functions that can be
running concurrently without the BQL.

> + *
> + * All functions in this header must use this assertion:
> + * g_assert(qemu_in_main_thread());
> + * to be sure they belong here.

I suggest "to catch when they are accidentally called without the BQL".
It explains the rationale whereas "to be sure they belong here" doesn't
explain anything.

> diff --git a/include/sysemu/block-backend-io.h 
> b/include/sysemu/block-backend-io.h
> new file mode 100644
> index 0000000000..66a7bed9f0
> --- /dev/null
> +++ b/include/sysemu/block-backend-io.h
> @@ -0,0 +1,123 @@
> +/*
> + * QEMU Block backends
> + *
> + * Copyright (C) 2014-2016 Red Hat, Inc.
> + *
> + * Authors:
> + *  Markus Armbruster <armbru@redhat.com>,
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1
> + * or later.  See the COPYING.LIB file in the top-level directory.
> + */
> +
> +#ifndef BLOCK_BACKEND_IO_H
> +#define BLOCK_BACKEND_IO_H
> +
> +#include "block-backend-common.h"
> +
> +/*
> + * I/O API functions. These functions are thread-safe, and therefore
> + * can run in any AioContext.

"can run in any AioContext" makes me wonder what the exact requirements
are. Can they run in any *thread* (regardless of whether an AioContext
even exists for that thread) or do they need to run in a thread that has
called qemu_set_current_aio_context()?

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]