[PULL 14/30] qapi job: Elide redundant has_FOO in generated C
The has_FOO for pointer-valued FOO are redundant, except for arrays. They are also a nuisance to work with. Recent commit "qapi: Start to elide redundant has_FOO in generated C" provided the means to elide them step by step. This is the step for qapi/job.json. Said commit explains the transformation in more detail. The invariant violations mentioned there do not occur here. Cc: John Snow Cc: Vladimir Sementsov-Ogievskiy Cc: qemu-block@nongnu.org Signed-off-by: Markus Armbruster Reviewed-by: Vladimir Sementsov-Ogievskiy Message-Id: <20221104160712.3005652-15-arm...@redhat.com> --- job-qmp.c | 3 +-- scripts/qapi/schema.py | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/job-qmp.c b/job-qmp.c index d498fc89c0..9e26fa899f 100644 --- a/job-qmp.c +++ b/job-qmp.c @@ -156,8 +156,7 @@ static JobInfo *job_query_single_locked(Job *job, Error **errp) .status = job->status, .current_progress = progress_current, .total_progress = progress_total, -.has_error = !!job->err, -.error = job->err ? \ +.error = job->err ? g_strdup(error_get_pretty(job->err)) : NULL, }; diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 07e2a0f263..ff73fdb0b3 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -759,7 +759,6 @@ def need_has(self): assert self.type # Temporary hack to support dropping the has_FOO in reviewable chunks opt_out = [ -'qapi/job.json', 'qapi/machine.json', 'qapi/machine-target.json', 'qapi/migration.json', -- 2.37.3
[PULL 10/30] qapi block: Elide redundant has_FOO in generated C
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/block*.json.
Said commit explains the transformation in more detail.
There is one instance of the invariant violation mentioned there:
qcow2_signal_corruption() passes false, "" when node_name is an empty
string. Take care to pass NULL then.
The previous two commits cleaned up two more.
Additionally, helper bdrv_latency_histogram_stats() loses its output
parameters and returns a value instead.
Cc: Kevin Wolf
Cc: Hanna Reitz
Cc: qemu-block@nongnu.org
Signed-off-by: Markus Armbruster
Message-Id: <20221104160712.3005652-11-arm...@redhat.com>
---
block/block-backend.c | 2 +-
block/copy-before-write.c | 2 +-
block/dirty-bitmap.c | 1 -
block/export/export.c | 2 +-
block/export/vduse-blk.c | 3 +-
block/gluster.c| 3 -
block/monitor/block-hmp-cmds.c | 48 --
block/qapi-sysemu.c| 73 +--
block/qapi.c | 62 +
block/qcow.c | 10 +-
block/qcow2.c | 18 ++--
block/qed.c| 2 +-
block/quorum.c | 2 +-
block/rbd.c| 15 +--
block/ssh.c| 2 +-
blockdev-nbd.c | 9 +-
blockdev.c | 164 +
blockjob.c | 2 -
monitor/hmp-cmds.c | 3 +-
qemu-img.c | 13 ++-
qemu-nbd.c | 2 -
scripts/qapi/schema.py | 3 -
22 files changed, 167 insertions(+), 274 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index d98a96ff37..bf0ea3cfed 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1860,7 +1860,7 @@ static void send_qmp_error_event(BlockBackend *blk,
BlockDriverState *bs = blk_bs(blk);
optype = is_read ? IO_OPERATION_TYPE_READ : IO_OPERATION_TYPE_WRITE;
-qapi_event_send_block_io_error(blk_name(blk), !!bs,
+qapi_event_send_block_io_error(blk_name(blk),
bs ? bdrv_get_node_name(bs) : NULL, optype,
action, blk_iostatus_is_enabled(blk),
error == ENOSPC, strerror(error));
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 4abaa7339e..b28ae25ec1 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -432,7 +432,7 @@ static int cbw_open(BlockDriverState *bs, QDict *options,
int flags,
return -EINVAL;
}
-if (opts->has_bitmap) {
+if (opts->bitmap) {
bitmap = block_dirty_bitmap_lookup(opts->bitmap->node,
opts->bitmap->name, NULL, errp);
if (!bitmap) {
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index bf3dc0512a..9c39550698 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -541,7 +541,6 @@ BlockDirtyInfoList
*bdrv_query_dirty_bitmaps(BlockDriverState *bs)
info->count = bdrv_get_dirty_count(bm);
info->granularity = bdrv_dirty_bitmap_granularity(bm);
-info->has_name = !!bm->name;
info->name = g_strdup(bm->name);
info->recording = bdrv_dirty_bitmap_recording(bm);
info->busy = bdrv_dirty_bitmap_busy(bm);
diff --git a/block/export/export.c b/block/export/export.c
index 7cc0c25c1c..28a91c9c42 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -114,7 +114,7 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error
**errp)
ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
-if (export->has_iothread) {
+if (export->iothread) {
IOThread *iothread;
AioContext *new_ctx;
Error **set_context_errp;
diff --git a/block/export/vduse-blk.c b/block/export/vduse-blk.c
index f101c24c3f..350d6fdaf0 100644
--- a/block/export/vduse-blk.c
+++ b/block/export/vduse-blk.c
@@ -265,8 +265,7 @@ static int vduse_blk_exp_create(BlockExport *exp,
BlockExportOptions *opts,
}
vblk_exp->num_queues = num_queues;
vblk_exp->handler.blk = exp->blk;
-vblk_exp->handler.serial = g_strdup(vblk_opts->has_serial ?
-vblk_opts->serial : "");
+vblk_exp->handler.serial = g_strdup(vblk_opts->serial ?: "");
vblk_exp->handler.logical_block_size = logical_block_size;
vblk_exp->handler.writable = opts->writable;
diff --git a/block/gluster.c b/block/gluster.c
index 7c90f7ba4b..7efc296399 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -830,7 +830,6 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict
*options,
s->logfile = g_strdup(logfile ? logfile : GLUSTER_LOGFILE_DEFAULT);
gconf->logfile = g_str
Re: [PATCH v2 2/5] Support format or cache specific out file
On Tue, Dec 13, 2022 at 8:09 PM Hanna Reitz wrote:
>
> On 13.12.22 16:56, Nir Soffer wrote:
> > On Mon, Dec 12, 2022 at 12:38 PM Hanna Reitz wrote:
> >> On 28.11.22 15:15, Nir Soffer wrote:
> >>> Extend the test finder to find tests with format (*.out.qcow2) or cache
> >>> specific (*.out.nocache) out file. This worked before only for the
> >>> numbered tests.
> >>> ---
> >>>tests/qemu-iotests/findtests.py | 10 --
> >>>1 file changed, 8 insertions(+), 2 deletions(-)
> >> This patch lacks an S-o-b, too.
> >>
> >>> diff --git a/tests/qemu-iotests/findtests.py
> >>> b/tests/qemu-iotests/findtests.py
> >>> index dd77b453b8..f4344ce78c 100644
> >>> --- a/tests/qemu-iotests/findtests.py
> >>> +++ b/tests/qemu-iotests/findtests.py
> >>> @@ -38,31 +38,37 @@ def chdir(path: Optional[str] = None) ->
> >>> Iterator[None]:
> >>>os.chdir(saved_dir)
> >>>
> >>>
> >>>class TestFinder:
> >>>def __init__(self, test_dir: Optional[str] = None) -> None:
> >>>self.groups = defaultdict(set)
> >>>
> >>>with chdir(test_dir):
> >>>self.all_tests = glob.glob('[0-9][0-9][0-9]')
> >>>self.all_tests += [f for f in glob.iglob('tests/*')
> >>> - if not f.endswith('.out') and
> >>> - os.path.isfile(f + '.out')]
> >>> + if self.is_test(f)]
> >> So previously a file was only considered a test file if there was a
> >> corresponding reference output file (`f + '.out'`), so files without
> >> such a reference output aren’t considered test files...
> >>
> >>>for t in self.all_tests:
> >>>with open(t, encoding="utf-8") as f:
> >>>for line in f:
> >>>if line.startswith('# group: '):
> >>>for g in line.split()[2:]:
> >>>self.groups[g].add(t)
> >>>break
> >>>
> >>> +def is_test(self, fname: str) -> bool:
> >>> +"""
> >>> +The tests directory contains tests (no extension) and out files
> >>> +(*.out, *.out.{format}, *.out.{option}).
> >>> +"""
> >>> +return re.search(r'.+\.out(\.\w+)?$', fname) is None
> >> ...but this new function doesn’t check that. I think we should check it
> >> (just whether there’s any variant of `/{fname}\.out(\.\w+)?/` to go with
> >> `fname`) so that behavior isn’t changed.
> > This means that you cannot add a test without a *.out* file, which may
> > be useful when you don't use the out file for validation, but we can
> > add this later if needed.
>
> I don’t think tests work without a reference output, do they? At least
> a couple of years ago, the ./check script would refuse to run tests
> without a corresponding .out file.
This may be true, but most tests do not really need an out file and better be
verified by asserting. There are some python tests that have pointless out
file with the output of python unittest:
$ cat tests/qemu-iotests/tests/nbd-multiconn.out
...
--
Ran 3 tests
OK
This is not only unhelpful (update the output when adding a 4th test)
but fragile.
if unitests changes the output, maybe adding info about skipped tests, or
changing "---" to "", the test will break.
But for now I agree the test framework should keep the current behavior.
Nir
Re: [PATCH v2 2/5] Support format or cache specific out file
On 13.12.22 16:56, Nir Soffer wrote:
On Mon, Dec 12, 2022 at 12:38 PM Hanna Reitz wrote:
On 28.11.22 15:15, Nir Soffer wrote:
Extend the test finder to find tests with format (*.out.qcow2) or cache
specific (*.out.nocache) out file. This worked before only for the
numbered tests.
---
tests/qemu-iotests/findtests.py | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
This patch lacks an S-o-b, too.
diff --git a/tests/qemu-iotests/findtests.py b/tests/qemu-iotests/findtests.py
index dd77b453b8..f4344ce78c 100644
--- a/tests/qemu-iotests/findtests.py
+++ b/tests/qemu-iotests/findtests.py
@@ -38,31 +38,37 @@ def chdir(path: Optional[str] = None) -> Iterator[None]:
os.chdir(saved_dir)
class TestFinder:
def __init__(self, test_dir: Optional[str] = None) -> None:
self.groups = defaultdict(set)
with chdir(test_dir):
self.all_tests = glob.glob('[0-9][0-9][0-9]')
self.all_tests += [f for f in glob.iglob('tests/*')
- if not f.endswith('.out') and
- os.path.isfile(f + '.out')]
+ if self.is_test(f)]
So previously a file was only considered a test file if there was a
corresponding reference output file (`f + '.out'`), so files without
such a reference output aren’t considered test files...
for t in self.all_tests:
with open(t, encoding="utf-8") as f:
for line in f:
if line.startswith('# group: '):
for g in line.split()[2:]:
self.groups[g].add(t)
break
+def is_test(self, fname: str) -> bool:
+"""
+The tests directory contains tests (no extension) and out files
+(*.out, *.out.{format}, *.out.{option}).
+"""
+return re.search(r'.+\.out(\.\w+)?$', fname) is None
...but this new function doesn’t check that. I think we should check it
(just whether there’s any variant of `/{fname}\.out(\.\w+)?/` to go with
`fname`) so that behavior isn’t changed.
This means that you cannot add a test without a *.out* file, which may
be useful when you don't use the out file for validation, but we can
add this later if needed.
I don’t think tests work without a reference output, do they? At least
a couple of years ago, the ./check script would refuse to run tests
without a corresponding .out file.
Hanna
Re: [PATCH v2 2/5] Support format or cache specific out file
On Mon, Dec 12, 2022 at 12:38 PM Hanna Reitz wrote:
>
> On 28.11.22 15:15, Nir Soffer wrote:
> > Extend the test finder to find tests with format (*.out.qcow2) or cache
> > specific (*.out.nocache) out file. This worked before only for the
> > numbered tests.
> > ---
> > tests/qemu-iotests/findtests.py | 10 --
> > 1 file changed, 8 insertions(+), 2 deletions(-)
>
> This patch lacks an S-o-b, too.
>
> > diff --git a/tests/qemu-iotests/findtests.py
> > b/tests/qemu-iotests/findtests.py
> > index dd77b453b8..f4344ce78c 100644
> > --- a/tests/qemu-iotests/findtests.py
> > +++ b/tests/qemu-iotests/findtests.py
> > @@ -38,31 +38,37 @@ def chdir(path: Optional[str] = None) -> Iterator[None]:
> > os.chdir(saved_dir)
> >
> >
> > class TestFinder:
> > def __init__(self, test_dir: Optional[str] = None) -> None:
> > self.groups = defaultdict(set)
> >
> > with chdir(test_dir):
> > self.all_tests = glob.glob('[0-9][0-9][0-9]')
> > self.all_tests += [f for f in glob.iglob('tests/*')
> > - if not f.endswith('.out') and
> > - os.path.isfile(f + '.out')]
> > + if self.is_test(f)]
>
> So previously a file was only considered a test file if there was a
> corresponding reference output file (`f + '.out'`), so files without
> such a reference output aren’t considered test files...
>
> > for t in self.all_tests:
> > with open(t, encoding="utf-8") as f:
> > for line in f:
> > if line.startswith('# group: '):
> > for g in line.split()[2:]:
> > self.groups[g].add(t)
> > break
> >
> > +def is_test(self, fname: str) -> bool:
> > +"""
> > +The tests directory contains tests (no extension) and out files
> > +(*.out, *.out.{format}, *.out.{option}).
> > +"""
> > +return re.search(r'.+\.out(\.\w+)?$', fname) is None
>
> ...but this new function doesn’t check that. I think we should check it
> (just whether there’s any variant of `/{fname}\.out(\.\w+)?/` to go with
> `fname`) so that behavior isn’t changed.
This means that you cannot add a test without a *.out* file, which may
be useful when you don't use the out file for validation, but we can
add this later if needed.
I'll change the code to check both conditions.
Re: [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state
On 12/13/22 01:30, Philippe Mathieu-Daudé wrote:
On 13/12/22 01:14, Richard Henderson wrote:
On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:
The device endianness doesn't change during runtime.
What are you talking about? Of course it does.
The host CPU certainly does, but the virtio device doesn't... Does it?
This check only consider the device, not the CPU:
bool virtio_access_is_big_endian(VirtIODevice *vdev)
{
#if defined(LEGACY_VIRTIO_IS_BIENDIAN)
return virtio_is_big_endian(vdev);
This is set for both ARM and PPC, which checks current cpu endianness in both
cases.
and once the features are negotiated it doesn't seem to change.
Incorrect.
r~
Re: [RFC PATCH-for-8.0 07/10] hw/virtio: Directly access cached VirtIODevice::access_is_big_endian
On Tue, 13 Dec 2022 00:05:14 +0100
Philippe Mathieu-Daudé wrote:
> Since the device endianness doesn't change at runtime,
> use the cached value instead of evaluating it on each call.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> include/hw/virtio/virtio-access.h | 44 +++
> 1 file changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-access.h
> b/include/hw/virtio/virtio-access.h
> index 07aae69042..985f39fe16 100644
> --- a/include/hw/virtio/virtio-access.h
> +++ b/include/hw/virtio/virtio-access.h
> @@ -43,7 +43,7 @@ static inline uint16_t virtio_lduw_phys(VirtIODevice *vdev,
> hwaddr pa)
> {
> AddressSpace *dma_as = vdev->dma_as;
>
> -if (virtio_access_is_big_endian(vdev)) {
> +if (vdev->access_is_big_endian) {
For x86, virtio_access_is_big_endian() expands to :
static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
{
return false;
}
When I added these memory accessors, there was a strong requirement from MST
that x86 shouldn't have to pay for an out-of-line check when it is known that
everything is always little endian. Not sure exactly what you're trying to
achieve with VirtIODevice::access_is_big_endian but this shouldn't mess with
this fast path.
> return lduw_be_phys(dma_as, pa);
> }
> return lduw_le_phys(dma_as, pa);
> @@ -53,7 +53,7 @@ static inline uint32_t virtio_ldl_phys(VirtIODevice *vdev,
> hwaddr pa)
> {
> AddressSpace *dma_as = vdev->dma_as;
>
> -if (virtio_access_is_big_endian(vdev)) {
> +if (vdev->access_is_big_endian) {
> return ldl_be_phys(dma_as, pa);
> }
> return ldl_le_phys(dma_as, pa);
> @@ -63,7 +63,7 @@ static inline uint64_t virtio_ldq_phys(VirtIODevice *vdev,
> hwaddr pa)
> {
> AddressSpace *dma_as = vdev->dma_as;
>
> -if (virtio_access_is_big_endian(vdev)) {
> +if (vdev->access_is_big_endian) {
> return ldq_be_phys(dma_as, pa);
> }
> return ldq_le_phys(dma_as, pa);
> @@ -74,7 +74,7 @@ static inline void virtio_stw_phys(VirtIODevice *vdev,
> hwaddr pa,
> {
> AddressSpace *dma_as = vdev->dma_as;
>
> -if (virtio_access_is_big_endian(vdev)) {
> +if (vdev->access_is_big_endian) {
> stw_be_phys(dma_as, pa, value);
> } else {
> stw_le_phys(dma_as, pa, value);
> @@ -86,7 +86,7 @@ static inline void virtio_stl_phys(VirtIODevice *vdev,
> hwaddr pa,
> {
> AddressSpace *dma_as = vdev->dma_as;
>
> -if (virtio_access_is_big_endian(vdev)) {
> +if (vdev->access_is_big_endian) {
> stl_be_phys(dma_as, pa, value);
> } else {
> stl_le_phys(dma_as, pa, value);
> @@ -95,7 +95,7 @@ static inline void virtio_stl_phys(VirtIODevice *vdev,
> hwaddr pa,
>
> static inline void virtio_stw_p(VirtIODevice *vdev, void *ptr, uint16_t v)
> {
> -if (virtio_access_is_big_endian(vdev)) {
> +if (vdev->access_is_big_endian) {
> stw_be_p(ptr, v);
> } else {
> stw_le_p(ptr, v);
> @@ -104,7 +104,7 @@ static inline void virtio_stw_p(VirtIODevice *vdev, void
> *ptr, uint16_t v)
>
> static inline void virtio_stl_p(VirtIODevice *vdev, void *ptr, uint32_t v)
> {
> -if (virtio_access_is_big_endian(vdev)) {
> +if (vdev->access_is_big_endian) {
> stl_be_p(ptr, v);
> } else {
> stl_le_p(ptr, v);
> @@ -113,7 +113,7 @@ static inline void virtio_stl_p(VirtIODevice *vdev, void
> *ptr, uint32_t v)
>
> static inline void virtio_stq_p(VirtIODevice *vdev, void *ptr, uint64_t v)
> {
> -if (virtio_access_is_big_endian(vdev)) {
> +if (vdev->access_is_big_endian) {
> stq_be_p(ptr, v);
> } else {
> stq_le_p(ptr, v);
> @@ -122,7 +122,7 @@ static inline void virtio_stq_p(VirtIODevice *vdev, void
> *ptr, uint64_t v)
>
> static inline int virtio_lduw_p(VirtIODevice *vdev, const void *ptr)
> {
> -if (virtio_access_is_big_endian(vdev)) {
> +if (vdev->access_is_big_endian) {
> return lduw_be_p(ptr);
> } else {
> return lduw_le_p(ptr);
> @@ -131,7 +131,7 @@ static inline int virtio_lduw_p(VirtIODevice *vdev, const
> void *ptr)
>
> static inline int virtio_ldl_p(VirtIODevice *vdev, const void *ptr)
> {
> -if (virtio_access_is_big_endian(vdev)) {
> +if (vdev->access_is_big_endian) {
> return ldl_be_p(ptr);
> } else {
> return ldl_le_p(ptr);
> @@ -140,7 +140,7 @@ static inline int virtio_ldl_p(VirtIODevice *vdev, const
> void *ptr)
>
> static inline uint64_t virtio_ldq_p(VirtIODevice *vdev, const void *ptr)
> {
> -if (virtio_access_is_big_endian(vdev)) {
> +if (vdev->access_is_big_endian) {
> return ldq_be_p(ptr);
> } else {
> return ldq_le_p(ptr);
> @@ -150,9 +150,9 @@ static inline uint64_t virtio_ldq_p(VirtIODevice *vdev,
> const void *ptr)
> static inline uint16_t virtio_tswap16(VirtIODevice *vdev, uint16_t s)
> {
> #if HOST_BIG_ENDIAN
> -return virtio_a
[PATCH 05/14] block: Convert bdrv_refresh_total_sectors() to co_wrapper_mixed
From: Emanuele Giuseppe Esposito
BlockDriver->bdrv_getlength is categorized as IO callback, and it
currently doesn't run in a coroutine. We should let it take a graph
rdlock since the callback traverses the block nodes graph, which however
is only possible in a coroutine.
Therefore turn it into a co_wrapper to move the actual function into a
coroutine where the lock can be taken.
Because now this function creates a new coroutine and polls, we need to
take the AioContext lock where it is missing, for the only reason that
internally co_wrapper calls AIO_WAIT_WHILE and it expects to release the
AioContext lock.
This is especially messy when a co_wrapper creates a coroutine and polls
in bdrv_open_driver, because this function has so many callers in so
many context that it can easily lead to deadlocks. Therefore the new
rule for bdrv_open_driver is that the caller must always hold the
AioContext lock of the given bs (except if it is a coroutine), because
the function calls bdrv_refresh_total_sectors() which is now a
co_wrapper.
Once the rwlock is ultimated and placed in every place it needs to be,
we will poll using AIO_WAIT_WHILE_UNLOCKED and remove the AioContext
lock.
Signed-off-by: Emanuele Giuseppe Esposito
Signed-off-by: Kevin Wolf
---
include/block/block-io.h | 8 ++--
include/block/block_int-common.h | 2 +-
include/block/block_int-io.h | 5 -
include/sysemu/block-backend-io.h | 10 --
block.c | 26 +-
block/block-backend.c | 10 ++
block/commit.c| 4 ++--
block/mirror.c| 4 ++--
hw/scsi/scsi-disk.c | 5 +
tests/unit/test-block-iothread.c | 3 +++
block/meson.build | 1 +
11 files changed, 59 insertions(+), 19 deletions(-)
diff --git a/include/block/block-io.h b/include/block/block-io.h
index 3bf201f7f4..fe1fa339be 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -73,8 +73,12 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t
offset, bool exact,
PreallocMode prealloc, BdrvRequestFlags
flags,
Error **errp);
-int64_t bdrv_nb_sectors(BlockDriverState *bs);
-int64_t bdrv_getlength(BlockDriverState *bs);
+int64_t coroutine_fn bdrv_co_nb_sectors(BlockDriverState *bs);
+int64_t co_wrapper_mixed bdrv_nb_sectors(BlockDriverState *bs);
+
+int64_t coroutine_fn bdrv_co_getlength(BlockDriverState *bs);
+int64_t co_wrapper_mixed bdrv_getlength(BlockDriverState *bs);
+
int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts,
BlockDriverState *in_bs, Error **errp);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index adccb4e540..0e705e08f4 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -684,7 +684,7 @@ struct BlockDriver {
int coroutine_fn (*bdrv_co_truncate)(BlockDriverState *bs, int64_t offset,
bool exact, PreallocMode prealloc,
BdrvRequestFlags flags, Error **errp);
-int64_t (*bdrv_getlength)(BlockDriverState *bs);
+int64_t coroutine_fn (*bdrv_getlength)(BlockDriverState *bs);
int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
BlockMeasureInfo *(*bdrv_measure)(QemuOpts *opts, BlockDriverState *in_bs,
Error **errp);
diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h
index 6b285fb520..d1559a501f 100644
--- a/include/block/block_int-io.h
+++ b/include/block/block_int-io.h
@@ -120,7 +120,10 @@ int coroutine_fn bdrv_co_copy_range_to(BdrvChild *src,
int64_t src_offset,
BdrvRequestFlags read_flags,
BdrvRequestFlags write_flags);
-int bdrv_refresh_total_sectors(BlockDriverState *bs, int64_t hint);
+int coroutine_fn bdrv_co_refresh_total_sectors(BlockDriverState *bs,
+ int64_t hint);
+int co_wrapper_mixed
+bdrv_refresh_total_sectors(BlockDriverState *bs, int64_t hint);
BdrvChild *bdrv_cow_child(BlockDriverState *bs);
BdrvChild *bdrv_filter_child(BlockDriverState *bs);
diff --git a/include/sysemu/block-backend-io.h
b/include/sysemu/block-backend-io.h
index d8cc8d74f5..8ad8854ecb 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -57,9 +57,15 @@ bool blk_is_inserted(BlockBackend *blk);
bool blk_is_available(BlockBackend *blk);
void blk_lock_medium(BlockBackend *blk, bool locked);
void blk_eject(BlockBackend *blk, bool eject_flag);
-int64_t blk_getlength(BlockBackend *blk);
+
+int64_t coroutine_fn blk_co_getlength(BlockBackend *blk);
+int64_t co_wrapper_mixed blk_getlength(BlockBackend *blk);
+
void blk_ge
[PATCH 10/14] block: Convert bdrv_is_inserted() to co_wrapper
From: Emanuele Giuseppe Esposito
bdrv_is_inserted() is categorized as an I/O function, and it currently
doesn't run in a coroutine. We should let it take a graph rdlock since
it traverses the block nodes graph, which however is only possible in a
coroutine.
Therefore turn it into a co_wrapper to move the actual function into a
coroutine where the lock can be taken.
At the same time, add also blk_is_inserted as co_wrapper_mixed, since it
is called in both coroutine and non-coroutine contexts.
Because now this function creates a new coroutine and polls, we need to
take the AioContext lock where it is missing, for the only reason that
internally c_w_mixed_bdrv_rdlock calls AIO_WAIT_WHILE and it expects to
release the AioContext lock. Once the rwlock is ultimated and placed in
every place it needs to be, we will poll using AIO_WAIT_WHILE_UNLOCKED
and remove the AioContext lock.
Signed-off-by: Emanuele Giuseppe Esposito
Signed-off-by: Kevin Wolf
---
include/block/block-io.h | 5 -
include/block/block_int-common.h | 2 +-
include/sysemu/block-backend-io.h | 5 -
block.c | 4 ++--
block/block-backend.c | 4 ++--
block/io.c| 12 ++--
blockdev.c| 8 +++-
7 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/include/block/block-io.h b/include/block/block-io.h
index 1d6748463c..e27dc9787b 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -139,7 +139,10 @@ bool bdrv_is_read_only(BlockDriverState *bs);
bool bdrv_is_writable(BlockDriverState *bs);
bool bdrv_is_sg(BlockDriverState *bs);
int bdrv_get_flags(BlockDriverState *bs);
-bool bdrv_is_inserted(BlockDriverState *bs);
+
+bool coroutine_fn bdrv_co_is_inserted(BlockDriverState *bs);
+bool co_wrapper bdrv_is_inserted(BlockDriverState *bs);
+
void bdrv_lock_medium(BlockDriverState *bs, bool locked);
void bdrv_eject(BlockDriverState *bs, bool eject_flag);
const char *bdrv_get_format_name(BlockDriverState *bs);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 2c190c6d75..b954b3cb36 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -710,7 +710,7 @@ struct BlockDriver {
BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos);
/* removable device specific */
-bool (*bdrv_is_inserted)(BlockDriverState *bs);
+bool coroutine_fn (*bdrv_is_inserted)(BlockDriverState *bs);
void (*bdrv_eject)(BlockDriverState *bs, bool eject_flag);
void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
diff --git a/include/sysemu/block-backend-io.h
b/include/sysemu/block-backend-io.h
index 8ad8854ecb..a1eac6c00a 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -53,7 +53,10 @@ BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long
int req, void *buf,
void blk_inc_in_flight(BlockBackend *blk);
void blk_dec_in_flight(BlockBackend *blk);
-bool blk_is_inserted(BlockBackend *blk);
+
+bool coroutine_fn blk_co_is_inserted(BlockBackend *blk);
+bool co_wrapper_mixed blk_is_inserted(BlockBackend *blk);
+
bool blk_is_available(BlockBackend *blk);
void blk_lock_medium(BlockBackend *blk, bool locked);
void blk_eject(BlockBackend *blk, bool eject_flag);
diff --git a/block.c b/block.c
index 7694879a54..a7b9da6a7e 100644
--- a/block.c
+++ b/block.c
@@ -6797,7 +6797,7 @@ out:
/**
* Return TRUE if the media is present
*/
-bool bdrv_is_inserted(BlockDriverState *bs)
+bool coroutine_fn bdrv_co_is_inserted(BlockDriverState *bs)
{
BlockDriver *drv = bs->drv;
BdrvChild *child;
@@ -6810,7 +6810,7 @@ bool bdrv_is_inserted(BlockDriverState *bs)
return drv->bdrv_is_inserted(bs);
}
QLIST_FOREACH(child, &bs->children, next) {
-if (!bdrv_is_inserted(child->bs)) {
+if (!bdrv_co_is_inserted(child->bs)) {
return false;
}
}
diff --git a/block/block-backend.c b/block/block-backend.c
index 5f6b269a79..e4b54f7b0c 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1985,12 +1985,12 @@ void blk_activate(BlockBackend *blk, Error **errp)
bdrv_activate(bs, errp);
}
-bool blk_is_inserted(BlockBackend *blk)
+bool coroutine_fn blk_co_is_inserted(BlockBackend *blk)
{
BlockDriverState *bs = blk_bs(blk);
IO_CODE();
-return bs && bdrv_is_inserted(bs);
+return bs && bdrv_co_is_inserted(bs);
}
bool blk_is_available(BlockBackend *blk)
diff --git a/block/io.c b/block/io.c
index f988e42f26..116d6cf10b 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1612,7 +1612,7 @@ int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
trace_bdrv_co_preadv_part(bs, offset, bytes, flags);
-if (!bdrv_is_inserted(bs)) {
+if (!bdrv_co_is_inserted(bs)) {
return -ENOMEDIUM;
}
@@ -2057,7 +2057,7 @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
trace_bdrv_co_pwritev_part(
[PATCH 12/14] block: convert bdrv_lock_medium in co_wrapper
From: Emanuele Giuseppe Esposito
bdrv_lock_medium() is categorized as an I/O function, and it currently
doesn't run in a coroutine. We should let it take a graph rdlock since
it traverses the block nodes graph, which however is only possible in a
coroutine.
The only caller of this function is blk_lock_medium(). Therefore make
blk_lock_medium() a co_wrapper, so that it always creates a new
coroutine, and then make bdrv_lock_medium() a coroutine_fn where the
lock can be taken.
Signed-off-by: Emanuele Giuseppe Esposito
Signed-off-by: Kevin Wolf
---
include/block/block-io.h | 2 +-
include/block/block_int-common.h | 2 +-
include/sysemu/block-backend-io.h | 4 +++-
block.c | 2 +-
block/block-backend.c | 4 ++--
block/copy-on-read.c | 2 +-
block/filter-compress.c | 2 +-
block/raw-format.c| 2 +-
8 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/include/block/block-io.h b/include/block/block-io.h
index f3d49ea05f..7e76bb647a 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -143,7 +143,7 @@ int bdrv_get_flags(BlockDriverState *bs);
bool coroutine_fn bdrv_co_is_inserted(BlockDriverState *bs);
bool co_wrapper bdrv_is_inserted(BlockDriverState *bs);
-void bdrv_lock_medium(BlockDriverState *bs, bool locked);
+void coroutine_fn bdrv_co_lock_medium(BlockDriverState *bs, bool locked);
void coroutine_fn bdrv_co_eject(BlockDriverState *bs, bool eject_flag);
const char *bdrv_get_format_name(BlockDriverState *bs);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index fd31b37567..0de99682e2 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -712,7 +712,7 @@ struct BlockDriver {
/* removable device specific */
bool coroutine_fn (*bdrv_is_inserted)(BlockDriverState *bs);
void coroutine_fn (*bdrv_eject)(BlockDriverState *bs, bool eject_flag);
-void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
+void coroutine_fn (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
/* to control generic scsi devices */
BlockAIOCB *(*bdrv_aio_ioctl)(BlockDriverState *bs,
diff --git a/include/sysemu/block-backend-io.h
b/include/sysemu/block-backend-io.h
index 00209625e1..780c1e5f77 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -58,7 +58,9 @@ bool coroutine_fn blk_co_is_inserted(BlockBackend *blk);
bool co_wrapper_mixed blk_is_inserted(BlockBackend *blk);
bool blk_is_available(BlockBackend *blk);
-void blk_lock_medium(BlockBackend *blk, bool locked);
+
+void coroutine_fn blk_co_lock_medium(BlockBackend *blk, bool locked);
+void co_wrapper blk_lock_medium(BlockBackend *blk, bool locked);
void coroutine_fn blk_co_eject(BlockBackend *blk, bool eject_flag);
void co_wrapper blk_eject(BlockBackend *blk, bool eject_flag);
diff --git a/block.c b/block.c
index 11647e49db..d5e660bc9d 100644
--- a/block.c
+++ b/block.c
@@ -6834,7 +6834,7 @@ void coroutine_fn bdrv_co_eject(BlockDriverState *bs,
bool eject_flag)
* Lock or unlock the media (if it is locked, the user won't be able
* to eject it manually).
*/
-void bdrv_lock_medium(BlockDriverState *bs, bool locked)
+void coroutine_fn bdrv_co_lock_medium(BlockDriverState *bs, bool locked)
{
BlockDriver *drv = bs->drv;
IO_CODE();
diff --git a/block/block-backend.c b/block/block-backend.c
index 233ce6d05a..fcc9ede031 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1999,13 +1999,13 @@ bool blk_is_available(BlockBackend *blk)
return blk_is_inserted(blk) && !blk_dev_is_tray_open(blk);
}
-void blk_lock_medium(BlockBackend *blk, bool locked)
+void coroutine_fn blk_co_lock_medium(BlockBackend *blk, bool locked)
{
BlockDriverState *bs = blk_bs(blk);
IO_CODE();
if (bs) {
-bdrv_lock_medium(bs, locked);
+bdrv_co_lock_medium(bs, locked);
}
}
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 76f884a6ae..ccc767f37b 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -224,7 +224,7 @@ static void cor_eject(BlockDriverState *bs, bool eject_flag)
static void cor_lock_medium(BlockDriverState *bs, bool locked)
{
-bdrv_lock_medium(bs->file->bs, locked);
+bdrv_co_lock_medium(bs->file->bs, locked);
}
diff --git a/block/filter-compress.c b/block/filter-compress.c
index 571e4684dd..e10312c225 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -124,7 +124,7 @@ static void compress_eject(BlockDriverState *bs, bool
eject_flag)
static void compress_lock_medium(BlockDriverState *bs, bool locked)
{
-bdrv_lock_medium(bs->file->bs, locked);
+bdrv_co_lock_medium(bs->file->bs, locked);
}
diff --git a/block/raw-format.c b/block/raw-format.c
index efe1ef0265..adc766bf79 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -410,7 +410,7 @@ static void raw_eject(BlockDriverSta
[PATCH 14/14] block: Rename newly converted BlockDriver IO coroutine functions
From: Emanuele Giuseppe Esposito
Since these functions always run in coroutine context, adjust
their name to include "_co_", just like all other BlockDriver callbacks.
No functional change intended.
Signed-off-by: Emanuele Giuseppe Esposito
Signed-off-by: Kevin Wolf
---
include/block/block_int-common.h | 27 +-
block.c | 32 ++---
block/blkdebug.c | 4 +--
block/blkio.c| 6 ++--
block/blklogwrites.c | 2 +-
block/blkreplay.c| 2 +-
block/blkverify.c| 2 +-
block/copy-on-read.c | 6 ++--
block/crypto.c | 4 +--
block/curl.c | 8 +++---
block/file-posix.c | 48
block/file-win32.c | 12
block/filter-compress.c | 6 ++--
block/gluster.c | 16 +--
block/io.c | 16 +--
block/iscsi.c| 8 +++---
block/nbd.c | 6 ++--
block/nfs.c | 2 +-
block/null.c | 8 +++---
block/nvme.c | 6 ++--
block/preallocate.c | 2 +-
block/qcow.c | 2 +-
block/qcow2.c| 6 ++--
block/qed.c | 4 +--
block/quorum.c | 2 +-
block/raw-format.c | 8 +++---
block/rbd.c | 4 +--
block/replication.c | 2 +-
block/ssh.c | 2 +-
block/throttle.c | 2 +-
block/vdi.c | 2 +-
block/vhdx.c | 2 +-
block/vmdk.c | 4 +--
block/vpc.c | 2 +-
34 files changed, 132 insertions(+), 133 deletions(-)
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 90702c6dcf..fd8ccaefee 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -684,8 +684,9 @@ struct BlockDriver {
int coroutine_fn (*bdrv_co_truncate)(BlockDriverState *bs, int64_t offset,
bool exact, PreallocMode prealloc,
BdrvRequestFlags flags, Error **errp);
-int64_t coroutine_fn (*bdrv_getlength)(BlockDriverState *bs);
-int64_t coroutine_fn (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
+int64_t coroutine_fn (*bdrv_co_getlength)(BlockDriverState *bs);
+int64_t coroutine_fn (*bdrv_co_get_allocated_file_size)(
+BlockDriverState *bs);
BlockMeasureInfo *(*bdrv_measure)(QemuOpts *opts, BlockDriverState *in_bs,
Error **errp);
@@ -696,23 +697,23 @@ struct BlockDriver {
int64_t offset, int64_t bytes, QEMUIOVector *qiov,
size_t qiov_offset);
-int coroutine_fn (*bdrv_get_info)(BlockDriverState *bs,
- BlockDriverInfo *bdi);
+int coroutine_fn (*bdrv_co_get_info)(BlockDriverState *bs,
+ BlockDriverInfo *bdi);
ImageInfoSpecific *(*bdrv_get_specific_info)(BlockDriverState *bs,
Error **errp);
BlockStatsSpecific *(*bdrv_get_specific_stats)(BlockDriverState *bs);
-int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_save_vmstate)(
+int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_save_vmstate)(
BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos);
-int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_load_vmstate)(
+int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_load_vmstate)(
BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos);
/* removable device specific */
-bool coroutine_fn (*bdrv_is_inserted)(BlockDriverState *bs);
-void coroutine_fn (*bdrv_eject)(BlockDriverState *bs, bool eject_flag);
-void coroutine_fn (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
+bool coroutine_fn (*bdrv_co_is_inserted)(BlockDriverState *bs);
+void coroutine_fn (*bdrv_co_eject)(BlockDriverState *bs, bool eject_flag);
+void coroutine_fn (*bdrv_co_lock_medium)(BlockDriverState *bs, bool
locked);
/* to control generic scsi devices */
BlockAIOCB *(*bdrv_aio_ioctl)(BlockDriverState *bs,
@@ -728,12 +729,12 @@ struct BlockDriver {
int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_check)(
BlockDriverState *bs, BdrvCheckResult *result, BdrvCheckMode fix);
-void coroutine_fn (*bdrv_debug_event)(BlockDriverState *bs,
- BlkdebugEvent event);
+void coroutine_fn (*bdrv_co_debug_event)(BlockDriverState *bs,
+ BlkdebugEvent event);
/* io queue for linux-aio */
-void coroutine_fn (*bdrv_io_plug)(BlockDriverState *bs);
-void coroutine_fn (*bdrv_io_unplug)(BlockDriverState *bs);
+voi
[PATCH 09/14] block: Convert bdrv_get_info() to co_wrapper_mixed
From: Emanuele Giuseppe Esposito
bdrv_get_info() is categorized as an I/O function, and it currently
doesn't run in a coroutine. We should let it take a graph rdlock since
it traverses the block nodes graph, which however is only possible in a
coroutine.
Therefore turn it into a co_wrapper to move the actual function into a
coroutine where the lock can be taken.
Signed-off-by: Emanuele Giuseppe Esposito
Signed-off-by: Kevin Wolf
---
include/block/block-io.h | 5 -
include/block/block_int-common.h | 3 ++-
block.c | 4 ++--
block/crypto.c | 2 +-
block/io.c | 8
block/mirror.c | 2 +-
block/raw-format.c | 2 +-
7 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/include/block/block-io.h b/include/block/block-io.h
index b1c6abee7b..1d6748463c 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -148,7 +148,10 @@ bool bdrv_supports_compressed_writes(BlockDriverState *bs);
const char *bdrv_get_node_name(const BlockDriverState *bs);
const char *bdrv_get_device_name(const BlockDriverState *bs);
const char *bdrv_get_device_or_node_name(const BlockDriverState *bs);
-int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
+
+int coroutine_fn bdrv_co_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
+int co_wrapper_mixed bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
+
ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs,
Error **errp);
BlockStatsSpecific *bdrv_get_specific_stats(BlockDriverState *bs);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 7b324c3bfc..2c190c6d75 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -696,7 +696,8 @@ struct BlockDriver {
int64_t offset, int64_t bytes, QEMUIOVector *qiov,
size_t qiov_offset);
-int (*bdrv_get_info)(BlockDriverState *bs, BlockDriverInfo *bdi);
+int coroutine_fn (*bdrv_get_info)(BlockDriverState *bs,
+ BlockDriverInfo *bdi);
ImageInfoSpecific *(*bdrv_get_specific_info)(BlockDriverState *bs,
Error **errp);
diff --git a/block.c b/block.c
index f3d6693113..7694879a54 100644
--- a/block.c
+++ b/block.c
@@ -6300,7 +6300,7 @@ void bdrv_get_backing_filename(BlockDriverState *bs,
pstrcpy(filename, filename_size, bs->backing_file);
}
-int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
+int coroutine_fn bdrv_co_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
{
int ret;
BlockDriver *drv = bs->drv;
@@ -6312,7 +6312,7 @@ int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo
*bdi)
if (!drv->bdrv_get_info) {
BlockDriverState *filtered = bdrv_filter_bs(bs);
if (filtered) {
-return bdrv_get_info(filtered, bdi);
+return bdrv_co_get_info(filtered, bdi);
}
return -ENOTSUP;
}
diff --git a/block/crypto.c b/block/crypto.c
index bbeb9f437c..1b4a9eb8e7 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -743,7 +743,7 @@ static int block_crypto_get_info_luks(BlockDriverState *bs,
BlockDriverInfo subbdi;
int ret;
-ret = bdrv_get_info(bs->file->bs, &subbdi);
+ret = bdrv_co_get_info(bs->file->bs, &subbdi);
if (ret != 0) {
return ret;
}
diff --git a/block/io.c b/block/io.c
index aef0929202..f988e42f26 100644
--- a/block/io.c
+++ b/block/io.c
@@ -712,14 +712,14 @@ BdrvTrackedRequest *coroutine_fn
bdrv_co_get_self_request(BlockDriverState *bs)
/**
* Round a region to cluster boundaries
*/
-void bdrv_round_to_clusters(BlockDriverState *bs,
+void coroutine_fn bdrv_round_to_clusters(BlockDriverState *bs,
int64_t offset, int64_t bytes,
int64_t *cluster_offset,
int64_t *cluster_bytes)
{
BlockDriverInfo bdi;
IO_CODE();
-if (bdrv_get_info(bs, &bdi) < 0 || bdi.cluster_size == 0) {
+if (bdrv_co_get_info(bs, &bdi) < 0 || bdi.cluster_size == 0) {
*cluster_offset = offset;
*cluster_bytes = bytes;
} else {
@@ -729,12 +729,12 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
}
}
-static int bdrv_get_cluster_size(BlockDriverState *bs)
+static coroutine_fn int bdrv_get_cluster_size(BlockDriverState *bs)
{
BlockDriverInfo bdi;
int ret;
-ret = bdrv_get_info(bs, &bdi);
+ret = bdrv_co_get_info(bs, &bdi);
if (ret < 0 || bdi.cluster_size == 0) {
return bs->bl.request_alignment;
} else {
diff --git a/block/mirror.c b/block/mirror.c
index c7d7ce2f8f..26db3ad0d7 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -956,7 +956,7 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
*/
bdrv_get_backing_filename(target_bs, backing_filename,
[PATCH 08/14] block: Convert bdrv_get_allocated_file_size() to co_wrapper
From: Emanuele Giuseppe Esposito
bdrv_get_allocated_file_size() is categorized as an I/O function, and it
currently doesn't run in a coroutine. We should let it take a graph
rdlock since it traverses the block nodes graph, which however is only
possible in a coroutine.
Therefore turn it into a co_wrapper to move the actual function into a
coroutine where the lock can be taken.
Signed-off-by: Emanuele Giuseppe Esposito
Signed-off-by: Kevin Wolf
---
include/block/block-io.h | 4 +++-
include/block/block_int-common.h | 3 ++-
block.c | 6 +++---
block/qcow2-refcount.c | 2 +-
4 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/include/block/block-io.h b/include/block/block-io.h
index fe1fa339be..b1c6abee7b 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -79,7 +79,9 @@ int64_t co_wrapper_mixed bdrv_nb_sectors(BlockDriverState
*bs);
int64_t coroutine_fn bdrv_co_getlength(BlockDriverState *bs);
int64_t co_wrapper_mixed bdrv_getlength(BlockDriverState *bs);
-int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
+int64_t coroutine_fn bdrv_co_get_allocated_file_size(BlockDriverState *bs);
+int64_t co_wrapper bdrv_get_allocated_file_size(BlockDriverState *bs);
+
BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts,
BlockDriverState *in_bs, Error **errp);
void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 0e705e08f4..7b324c3bfc 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -685,7 +685,8 @@ struct BlockDriver {
bool exact, PreallocMode prealloc,
BdrvRequestFlags flags, Error **errp);
int64_t coroutine_fn (*bdrv_getlength)(BlockDriverState *bs);
-int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
+int64_t coroutine_fn (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
+
BlockMeasureInfo *(*bdrv_measure)(QemuOpts *opts, BlockDriverState *in_bs,
Error **errp);
diff --git a/block.c b/block.c
index e1a8f7af4d..f3d6693113 100644
--- a/block.c
+++ b/block.c
@@ -5732,7 +5732,7 @@ static int64_t
bdrv_sum_allocated_file_size(BlockDriverState *bs)
if (child->role & (BDRV_CHILD_DATA | BDRV_CHILD_METADATA |
BDRV_CHILD_FILTERED))
{
-child_size = bdrv_get_allocated_file_size(child->bs);
+child_size = bdrv_co_get_allocated_file_size(child->bs);
if (child_size < 0) {
return child_size;
}
@@ -5747,7 +5747,7 @@ static int64_t
bdrv_sum_allocated_file_size(BlockDriverState *bs)
* Length of a allocated file in bytes. Sparse files are counted by actual
* allocated space. Return < 0 if error or unknown.
*/
-int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
+int64_t coroutine_fn bdrv_co_get_allocated_file_size(BlockDriverState *bs)
{
BlockDriver *drv = bs->drv;
IO_CODE();
@@ -5768,7 +5768,7 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
return -ENOTSUP;
} else if (drv->is_filter) {
/* Filter drivers default to the size of their filtered child */
-return bdrv_get_allocated_file_size(bdrv_filter_bs(bs));
+return bdrv_co_get_allocated_file_size(bdrv_filter_bs(bs));
} else {
/* Other drivers default to summing their children's sizes */
return bdrv_sum_allocated_file_size(bs);
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 81264740f0..487681d85e 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -3719,7 +3719,7 @@ int coroutine_fn
qcow2_detect_metadata_preallocation(BlockDriverState *bs)
return file_length;
}
-real_allocation = bdrv_get_allocated_file_size(bs->file->bs);
+real_allocation = bdrv_co_get_allocated_file_size(bs->file->bs);
if (real_allocation < 0) {
return real_allocation;
}
--
2.38.1
[PATCH 04/14] block: Rename refresh_total_sectors to bdrv_refresh_total_sectors
From: Emanuele Giuseppe Esposito
The name is not good, not the least because we are going to convert this
to a generated co_wrapper, which adds a _co infix after the first part
of the name.
No functional change intended.
Signed-off-by: Emanuele Giuseppe Esposito
Signed-off-by: Kevin Wolf
---
include/block/block_int-io.h | 2 +-
block.c | 8
block/io.c | 8 +---
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h
index 8bc061ebb8..6b285fb520 100644
--- a/include/block/block_int-io.h
+++ b/include/block/block_int-io.h
@@ -120,7 +120,7 @@ int coroutine_fn bdrv_co_copy_range_to(BdrvChild *src,
int64_t src_offset,
BdrvRequestFlags read_flags,
BdrvRequestFlags write_flags);
-int refresh_total_sectors(BlockDriverState *bs, int64_t hint);
+int bdrv_refresh_total_sectors(BlockDriverState *bs, int64_t hint);
BdrvChild *bdrv_cow_child(BlockDriverState *bs);
BdrvChild *bdrv_filter_child(BlockDriverState *bs);
diff --git a/block.c b/block.c
index 9c2ac757e4..fdcd06d05d 100644
--- a/block.c
+++ b/block.c
@@ -1034,7 +1034,7 @@ static int find_image_format(BlockBackend *file, const
char *filename,
* Set the current 'total_sectors' value
* Return 0 on success, -errno on error.
*/
-int refresh_total_sectors(BlockDriverState *bs, int64_t hint)
+int bdrv_refresh_total_sectors(BlockDriverState *bs, int64_t hint)
{
BlockDriver *drv = bs->drv;
IO_CODE();
@@ -1651,7 +1651,7 @@ static int bdrv_open_driver(BlockDriverState *bs,
BlockDriver *drv,
bs->supported_read_flags |= BDRV_REQ_REGISTERED_BUF;
bs->supported_write_flags |= BDRV_REQ_REGISTERED_BUF;
-ret = refresh_total_sectors(bs, bs->total_sectors);
+ret = bdrv_refresh_total_sectors(bs, bs->total_sectors);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not refresh total sector count");
return ret;
@@ -5808,7 +5808,7 @@ int64_t bdrv_nb_sectors(BlockDriverState *bs)
return -ENOMEDIUM;
if (drv->has_variable_length) {
-int ret = refresh_total_sectors(bs, bs->total_sectors);
+int ret = bdrv_refresh_total_sectors(bs, bs->total_sectors);
if (ret < 0) {
return ret;
}
@@ -6590,7 +6590,7 @@ int bdrv_activate(BlockDriverState *bs, Error **errp)
bdrv_dirty_bitmap_skip_store(bm, false);
}
-ret = refresh_total_sectors(bs, bs->total_sectors);
+ret = bdrv_refresh_total_sectors(bs, bs->total_sectors);
if (ret < 0) {
bs->open_flags |= BDRV_O_INACTIVE;
error_setg_errno(errp, -ret, "Could not refresh total sector
count");
diff --git a/block/io.c b/block/io.c
index c33672e90a..3940026dc1 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3464,15 +3464,17 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child,
int64_t offset, bool exact,
goto out;
}
-ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
+ret = bdrv_refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not refresh total sector count");
} else {
offset = bs->total_sectors * BDRV_SECTOR_SIZE;
}
-/* It's possible that truncation succeeded but refresh_total_sectors
+/*
+ * It's possible that truncation succeeded but bdrv_refresh_total_sectors
* failed, but the latter doesn't affect how we should finish the request.
- * Pass 0 as the last parameter so that dirty bitmaps etc. are handled. */
+ * Pass 0 as the last parameter so that dirty bitmaps etc. are handled.
+ */
bdrv_co_write_req_finish(child, offset - new_bytes, new_bytes, &req, 0);
out:
--
2.38.1
[PATCH 02/14] block: Convert bdrv_io_plug() to co_wrapper
From: Emanuele Giuseppe Esposito
BlockDriver->bdrv_io_plug is categorized as IO callback, and it
currently doesn't run in a coroutine. We should let it take a graph
rdlock since the callback traverses the block nodes graph, which however
is only possible in a coroutine.
The only caller of this function is blk_io_plug(), therefore make
blk_io_plug() a co_wrapper, so that we're always running in a coroutine
where the lock can be taken.
Signed-off-by: Emanuele Giuseppe Esposito
Signed-off-by: Kevin Wolf
---
include/block/block-io.h | 3 ++-
include/block/block_int-common.h | 2 +-
include/sysemu/block-backend-io.h | 4 +++-
block/block-backend.c | 4 ++--
block/io.c| 4 ++--
5 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/include/block/block-io.h b/include/block/block-io.h
index 2ed6214909..d96168375e 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -217,7 +217,8 @@ void bdrv_coroutine_enter(BlockDriverState *bs, Coroutine
*co);
AioContext *child_of_bds_get_parent_aio_context(BdrvChild *c);
-void bdrv_io_plug(BlockDriverState *bs);
+void coroutine_fn bdrv_co_io_plug(BlockDriverState *bs);
+
void bdrv_io_unplug(BlockDriverState *bs);
bool coroutine_fn bdrv_co_can_store_new_dirty_bitmap(BlockDriverState *bs,
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index c34c525fa6..4a1c1e348a 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -729,7 +729,7 @@ struct BlockDriver {
void (*bdrv_debug_event)(BlockDriverState *bs, BlkdebugEvent event);
/* io queue for linux-aio */
-void (*bdrv_io_plug)(BlockDriverState *bs);
+void coroutine_fn (*bdrv_io_plug)(BlockDriverState *bs);
void (*bdrv_io_unplug)(BlockDriverState *bs);
/**
diff --git a/include/sysemu/block-backend-io.h
b/include/sysemu/block-backend-io.h
index 7ec6d978d4..70b73f7d11 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -73,7 +73,9 @@ void blk_iostatus_set_err(BlockBackend *blk, int error);
int blk_get_max_iov(BlockBackend *blk);
int blk_get_max_hw_iov(BlockBackend *blk);
-void blk_io_plug(BlockBackend *blk);
+void coroutine_fn blk_co_io_plug(BlockBackend *blk);
+void co_wrapper blk_io_plug(BlockBackend *blk);
+
void blk_io_unplug(BlockBackend *blk);
AioContext *blk_get_aio_context(BlockBackend *blk);
BlockAcctStats *blk_get_stats(BlockBackend *blk);
diff --git a/block/block-backend.c b/block/block-backend.c
index 2852a892de..f862fd1950 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2315,13 +2315,13 @@ void blk_add_insert_bs_notifier(BlockBackend *blk,
Notifier *notify)
notifier_list_add(&blk->insert_bs_notifiers, notify);
}
-void blk_io_plug(BlockBackend *blk)
+void coroutine_fn blk_co_io_plug(BlockBackend *blk)
{
BlockDriverState *bs = blk_bs(blk);
IO_CODE();
if (bs) {
-bdrv_io_plug(bs);
+bdrv_co_io_plug(bs);
}
}
diff --git a/block/io.c b/block/io.c
index d87788dfbb..3d27836420 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3127,13 +3127,13 @@ void *qemu_try_blockalign0(BlockDriverState *bs, size_t
size)
return mem;
}
-void bdrv_io_plug(BlockDriverState *bs)
+void coroutine_fn bdrv_co_io_plug(BlockDriverState *bs)
{
BdrvChild *child;
IO_CODE();
QLIST_FOREACH(child, &bs->children, next) {
-bdrv_io_plug(child->bs);
+bdrv_co_io_plug(child->bs);
}
if (qatomic_fetch_inc(&bs->io_plugged) == 0) {
--
2.38.1
[PATCH 07/14] block: use bdrv_co_refresh_total_sectors when possible
From: Emanuele Giuseppe Esposito
In some places we are sure we are always running in a
coroutine, therefore it's useless to call the generated_co_wrapper,
instead call directly the _co_ function.
Signed-off-by: Emanuele Giuseppe Esposito
Signed-off-by: Kevin Wolf
---
block/block-backend.c | 6 +++---
block/copy-on-read.c | 2 +-
block/io.c| 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 5b8da86772..5f6b269a79 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1235,8 +1235,8 @@ void blk_set_disable_request_queuing(BlockBackend *blk,
bool disable)
blk->disable_request_queuing = disable;
}
-static int blk_check_byte_request(BlockBackend *blk, int64_t offset,
- int64_t bytes)
+static coroutine_fn int blk_check_byte_request(BlockBackend *blk,
+ int64_t offset, int64_t bytes)
{
int64_t len;
@@ -1253,7 +1253,7 @@ static int blk_check_byte_request(BlockBackend *blk,
int64_t offset,
}
if (!blk->allow_write_beyond_eof) {
-len = bdrv_getlength(blk_bs(blk));
+len = bdrv_co_getlength(blk_bs(blk));
if (len < 0) {
return len;
}
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 815ac1d835..74f7727a02 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -122,7 +122,7 @@ static void cor_child_perm(BlockDriverState *bs, BdrvChild
*c,
static int64_t cor_getlength(BlockDriverState *bs)
{
-return bdrv_getlength(bs->file->bs);
+return bdrv_co_getlength(bs->file->bs);
}
diff --git a/block/io.c b/block/io.c
index 3940026dc1..aef0929202 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3434,7 +3434,7 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child,
int64_t offset, bool exact,
if (new_bytes && backing) {
int64_t backing_len;
-backing_len = bdrv_getlength(backing->bs);
+backing_len = bdrv_co_getlength(backing->bs);
if (backing_len < 0) {
ret = backing_len;
error_setg_errno(errp, -ret, "Could not get backing file size");
@@ -3464,7 +3464,7 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child,
int64_t offset, bool exact,
goto out;
}
-ret = bdrv_refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
+ret = bdrv_co_refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not refresh total sector count");
} else {
--
2.38.1
[PATCH 03/14] block: Convert bdrv_io_unplug() to co_wrapper
From: Emanuele Giuseppe Esposito
BlockDriver->bdrv_io_unplug is categorized as IO callback, and it
currently doesn't run in a coroutine. We should let it take a graph
rdlock since the callback traverses the block nodes graph, which however
is only possible in a coroutine.
The only caller of this function is blk_io_unplug(), therefore make
blk_io_unplug() a co_wrapper, so that we're always running in a
coroutine where the lock can be taken.
Signed-off-by: Emanuele Giuseppe Esposito
Signed-off-by: Kevin Wolf
---
include/block/block-io.h | 3 +--
include/block/block_int-common.h | 2 +-
include/sysemu/block-backend-io.h | 4 +++-
block/block-backend.c | 4 ++--
block/io.c| 4 ++--
5 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/include/block/block-io.h b/include/block/block-io.h
index d96168375e..3bf201f7f4 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -218,8 +218,7 @@ void bdrv_coroutine_enter(BlockDriverState *bs, Coroutine
*co);
AioContext *child_of_bds_get_parent_aio_context(BdrvChild *c);
void coroutine_fn bdrv_co_io_plug(BlockDriverState *bs);
-
-void bdrv_io_unplug(BlockDriverState *bs);
+void coroutine_fn bdrv_co_io_unplug(BlockDriverState *bs);
bool coroutine_fn bdrv_co_can_store_new_dirty_bitmap(BlockDriverState *bs,
const char *name,
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 4a1c1e348a..adccb4e540 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -730,7 +730,7 @@ struct BlockDriver {
/* io queue for linux-aio */
void coroutine_fn (*bdrv_io_plug)(BlockDriverState *bs);
-void (*bdrv_io_unplug)(BlockDriverState *bs);
+void coroutine_fn (*bdrv_io_unplug)(BlockDriverState *bs);
/**
* bdrv_drain_begin is called if implemented in the beginning of a
diff --git a/include/sysemu/block-backend-io.h
b/include/sysemu/block-backend-io.h
index 70b73f7d11..d8cc8d74f5 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -76,7 +76,9 @@ int blk_get_max_hw_iov(BlockBackend *blk);
void coroutine_fn blk_co_io_plug(BlockBackend *blk);
void co_wrapper blk_io_plug(BlockBackend *blk);
-void blk_io_unplug(BlockBackend *blk);
+void coroutine_fn blk_co_io_unplug(BlockBackend *blk);
+void co_wrapper blk_io_unplug(BlockBackend *blk);
+
AioContext *blk_get_aio_context(BlockBackend *blk);
BlockAcctStats *blk_get_stats(BlockBackend *blk);
void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
diff --git a/block/block-backend.c b/block/block-backend.c
index f862fd1950..5a357f504a 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2325,13 +2325,13 @@ void coroutine_fn blk_co_io_plug(BlockBackend *blk)
}
}
-void blk_io_unplug(BlockBackend *blk)
+void coroutine_fn blk_co_io_unplug(BlockBackend *blk)
{
BlockDriverState *bs = blk_bs(blk);
IO_CODE();
if (bs) {
-bdrv_io_unplug(bs);
+bdrv_co_io_unplug(bs);
}
}
diff --git a/block/io.c b/block/io.c
index 3d27836420..c33672e90a 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3144,7 +3144,7 @@ void coroutine_fn bdrv_co_io_plug(BlockDriverState *bs)
}
}
-void bdrv_io_unplug(BlockDriverState *bs)
+void coroutine_fn bdrv_co_io_unplug(BlockDriverState *bs)
{
BdrvChild *child;
IO_CODE();
@@ -3158,7 +3158,7 @@ void bdrv_io_unplug(BlockDriverState *bs)
}
QLIST_FOREACH(child, &bs->children, next) {
-bdrv_io_unplug(child->bs);
+bdrv_co_io_unplug(child->bs);
}
}
--
2.38.1
[PATCH 13/14] block: Convert bdrv_debug_event to co_wrapper_mixed
From: Emanuele Giuseppe Esposito
bdrv_debug_event() is categorized as an I/O function, and it currently
doesn't run in a coroutine. We should let it take a graph rdlock since
it traverses the block nodes graph, which however is only possible in a
coroutine.
Therefore turn it into a co_wrapper_mixed to move the actual function
into a coroutine where the lock can be taken.
Signed-off-by: Emanuele Giuseppe Esposito
Signed-off-by: Kevin Wolf
---
include/block/block-io.h | 5 -
include/block/block_int-common.h | 3 ++-
block.c | 2 +-
block/io.c | 22 +++---
4 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/include/block/block-io.h b/include/block/block-io.h
index 7e76bb647a..9737fc63cb 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -188,7 +188,10 @@ void *qemu_try_blockalign0(BlockDriverState *bs, size_t
size);
void bdrv_enable_copy_on_read(BlockDriverState *bs);
void bdrv_disable_copy_on_read(BlockDriverState *bs);
-void bdrv_debug_event(BlockDriverState *bs, BlkdebugEvent event);
+void coroutine_fn bdrv_co_debug_event(BlockDriverState *bs,
+ BlkdebugEvent event);
+void co_wrapper_mixed bdrv_debug_event(BlockDriverState *bs,
+ BlkdebugEvent event);
#define BLKDBG_EVENT(child, evt) \
do { \
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 0de99682e2..90702c6dcf 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -728,7 +728,8 @@ struct BlockDriver {
int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_check)(
BlockDriverState *bs, BdrvCheckResult *result, BdrvCheckMode fix);
-void (*bdrv_debug_event)(BlockDriverState *bs, BlkdebugEvent event);
+void coroutine_fn (*bdrv_debug_event)(BlockDriverState *bs,
+ BlkdebugEvent event);
/* io queue for linux-aio */
void coroutine_fn (*bdrv_io_plug)(BlockDriverState *bs);
diff --git a/block.c b/block.c
index d5e660bc9d..d27105dd3b 100644
--- a/block.c
+++ b/block.c
@@ -6350,7 +6350,7 @@ BlockStatsSpecific
*bdrv_get_specific_stats(BlockDriverState *bs)
return drv->bdrv_get_specific_stats(bs);
}
-void bdrv_debug_event(BlockDriverState *bs, BlkdebugEvent event)
+void coroutine_fn bdrv_co_debug_event(BlockDriverState *bs, BlkdebugEvent
event)
{
IO_CODE();
if (!bs || !bs->drv || !bs->drv->bdrv_debug_event) {
diff --git a/block/io.c b/block/io.c
index 116d6cf10b..93ba851c8c 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1241,7 +1241,7 @@ static int coroutine_fn
bdrv_co_do_copy_on_readv(BdrvChild *child,
goto err;
}
-bdrv_debug_event(bs, BLKDBG_COR_WRITE);
+bdrv_co_debug_event(bs, BLKDBG_COR_WRITE);
if (drv->bdrv_co_pwrite_zeroes &&
buffer_is_zero(bounce_buffer, pnum)) {
/* FIXME: Should we (perhaps conditionally) be setting
@@ -1486,10 +1486,10 @@ static coroutine_fn int bdrv_padding_rmw_read(BdrvChild
*child,
qemu_iovec_init_buf(&local_qiov, pad->buf, bytes);
if (pad->head) {
-bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD);
+bdrv_co_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD);
}
if (pad->merge_reads && pad->tail) {
-bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
+bdrv_co_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
}
ret = bdrv_aligned_preadv(child, req, req->overlap_offset, bytes,
align, &local_qiov, 0, 0);
@@ -1497,10 +1497,10 @@ static coroutine_fn int bdrv_padding_rmw_read(BdrvChild
*child,
return ret;
}
if (pad->head) {
-bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD);
+bdrv_co_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD);
}
if (pad->merge_reads && pad->tail) {
-bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL);
+bdrv_co_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL);
}
if (pad->merge_reads) {
@@ -1511,7 +1511,7 @@ static coroutine_fn int bdrv_padding_rmw_read(BdrvChild
*child,
if (pad->tail) {
qemu_iovec_init_buf(&local_qiov, pad->tail_buf, align);
-bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
+bdrv_co_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
ret = bdrv_aligned_preadv(
child, req,
req->overlap_offset + req->overlap_bytes - align,
@@ -1519,7 +1519,7 @@ static coroutine_fn int bdrv_padding_rmw_read(BdrvChild
*child,
if (ret < 0) {
return ret;
}
-bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL);
+bdrv_co_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL);
}
zero_mem:
@@ -1921,16 +1921,16 @@ static int coroutine_fn bdrv
[PATCH 00/14] block: Move more functions to coroutines
This series converts some IO_CODE() functions to coroutine_fn because they access the graph and will need to hold the graph lock in the future. IO_CODE() functions can be called from iothreads, so taking the graph lock requires the function to run in coroutine context. Pretty much all of the changes in this series were posted by Emanuele before as part of "Protect the block layer with a rwlock: part 3". The major difference is that in the old version, the patches did two things at once: Converting functions to coroutine_fn, and adding the locking to them. This series does only the coroutine conversion. The locking part will be in another series which now comes with TSA annotations and makes the locking related changes big enough to have separate patches. Emanuele Giuseppe Esposito (14): block-coroutine-wrapper: support void functions block: Convert bdrv_io_plug() to co_wrapper block: Convert bdrv_io_unplug() to co_wrapper block: Rename refresh_total_sectors to bdrv_refresh_total_sectors block: Convert bdrv_refresh_total_sectors() to co_wrapper_mixed block-backend: use bdrv_getlength instead of blk_getlength block: use bdrv_co_refresh_total_sectors when possible block: Convert bdrv_get_allocated_file_size() to co_wrapper block: Convert bdrv_get_info() to co_wrapper_mixed block: Convert bdrv_is_inserted() to co_wrapper block: Convert bdrv_eject() to co_wrapper block: convert bdrv_lock_medium in co_wrapper block: Convert bdrv_debug_event to co_wrapper_mixed block: Rename newly converted BlockDriver IO coroutine functions include/block/block-io.h | 36 + include/block/block_int-common.h | 26 ++ include/block/block_int-io.h | 5 +- include/sysemu/block-backend-io.h | 31 --- block.c| 82 ++ block/blkdebug.c | 4 +- block/blkio.c | 6 +-- block/blklogwrites.c | 2 +- block/blkreplay.c | 2 +- block/blkverify.c | 2 +- block/block-backend.c | 36 ++--- block/commit.c | 4 +- block/copy-on-read.c | 12 ++--- block/crypto.c | 6 +-- block/curl.c | 8 +-- block/file-posix.c | 48 - block/file-win32.c | 12 ++--- block/filter-compress.c| 10 ++-- block/gluster.c| 16 +++--- block/io.c | 76 +-- block/iscsi.c | 8 +-- block/mirror.c | 6 +-- block/nbd.c| 6 +-- block/nfs.c| 2 +- block/null.c | 8 +-- block/nvme.c | 6 +-- block/preallocate.c| 2 +- block/qcow.c | 2 +- block/qcow2-refcount.c | 2 +- block/qcow2.c | 6 +-- block/qed.c| 4 +- block/quorum.c | 2 +- block/raw-format.c | 14 ++--- block/rbd.c| 4 +- block/replication.c| 2 +- block/ssh.c| 2 +- block/throttle.c | 2 +- block/vdi.c| 2 +- block/vhdx.c | 2 +- block/vmdk.c | 4 +- block/vpc.c| 2 +- blockdev.c | 8 ++- hw/scsi/scsi-disk.c| 5 ++ tests/unit/test-block-iothread.c | 3 ++ scripts/block-coroutine-wrapper.py | 20 ++-- block/meson.build | 1 + 46 files changed, 316 insertions(+), 233 deletions(-) -- 2.38.1
[PATCH 06/14] block-backend: use bdrv_getlength instead of blk_getlength
From: Emanuele Giuseppe Esposito
The only difference is that blk_ checks if the block is available,
but this check is already performed above in blk_check_byte_request().
This is in preparation for the graph rdlock, which will be taken
by both the callers of blk_check_byte_request() and blk_getlength().
Signed-off-by: Emanuele Giuseppe Esposito
Signed-off-by: Kevin Wolf
---
block/block-backend.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 0194d86113..5b8da86772 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1253,7 +1253,7 @@ static int blk_check_byte_request(BlockBackend *blk,
int64_t offset,
}
if (!blk->allow_write_beyond_eof) {
-len = blk_getlength(blk);
+len = bdrv_getlength(blk_bs(blk));
if (len < 0) {
return len;
}
--
2.38.1
[PATCH 11/14] block: Convert bdrv_eject() to co_wrapper
From: Emanuele Giuseppe Esposito
bdrv_eject() is categorized as an I/O function, and it currently
doesn't run in a coroutine. We should let it take a graph rdlock since
it traverses the block nodes graph, which however is only possible in a
coroutine.
The only caller of this function is blk_eject(). Therefore make
blk_eject() a co_wrapper, so that it always creates a new coroutine, and
then make bdrv_eject() coroutine_fn where the lock can be taken.
Signed-off-by: Emanuele Giuseppe Esposito
Signed-off-by: Kevin Wolf
---
include/block/block-io.h | 3 ++-
include/block/block_int-common.h | 2 +-
include/sysemu/block-backend-io.h | 4 +++-
block.c | 2 +-
block/block-backend.c | 4 ++--
block/copy-on-read.c | 2 +-
block/filter-compress.c | 2 +-
block/raw-format.c| 2 +-
8 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/include/block/block-io.h b/include/block/block-io.h
index e27dc9787b..f3d49ea05f 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -144,7 +144,8 @@ bool coroutine_fn bdrv_co_is_inserted(BlockDriverState *bs);
bool co_wrapper bdrv_is_inserted(BlockDriverState *bs);
void bdrv_lock_medium(BlockDriverState *bs, bool locked);
-void bdrv_eject(BlockDriverState *bs, bool eject_flag);
+void coroutine_fn bdrv_co_eject(BlockDriverState *bs, bool eject_flag);
+
const char *bdrv_get_format_name(BlockDriverState *bs);
bool bdrv_supports_compressed_writes(BlockDriverState *bs);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index b954b3cb36..fd31b37567 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -711,7 +711,7 @@ struct BlockDriver {
/* removable device specific */
bool coroutine_fn (*bdrv_is_inserted)(BlockDriverState *bs);
-void (*bdrv_eject)(BlockDriverState *bs, bool eject_flag);
+void coroutine_fn (*bdrv_eject)(BlockDriverState *bs, bool eject_flag);
void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
/* to control generic scsi devices */
diff --git a/include/sysemu/block-backend-io.h
b/include/sysemu/block-backend-io.h
index a1eac6c00a..00209625e1 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -59,7 +59,9 @@ bool co_wrapper_mixed blk_is_inserted(BlockBackend *blk);
bool blk_is_available(BlockBackend *blk);
void blk_lock_medium(BlockBackend *blk, bool locked);
-void blk_eject(BlockBackend *blk, bool eject_flag);
+
+void coroutine_fn blk_co_eject(BlockBackend *blk, bool eject_flag);
+void co_wrapper blk_eject(BlockBackend *blk, bool eject_flag);
int64_t coroutine_fn blk_co_getlength(BlockBackend *blk);
int64_t co_wrapper_mixed blk_getlength(BlockBackend *blk);
diff --git a/block.c b/block.c
index a7b9da6a7e..11647e49db 100644
--- a/block.c
+++ b/block.c
@@ -6820,7 +6820,7 @@ bool coroutine_fn bdrv_co_is_inserted(BlockDriverState
*bs)
/**
* If eject_flag is TRUE, eject the media. Otherwise, close the tray
*/
-void bdrv_eject(BlockDriverState *bs, bool eject_flag)
+void coroutine_fn bdrv_co_eject(BlockDriverState *bs, bool eject_flag)
{
BlockDriver *drv = bs->drv;
IO_CODE();
diff --git a/block/block-backend.c b/block/block-backend.c
index e4b54f7b0c..233ce6d05a 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2009,14 +2009,14 @@ void blk_lock_medium(BlockBackend *blk, bool locked)
}
}
-void blk_eject(BlockBackend *blk, bool eject_flag)
+void coroutine_fn blk_co_eject(BlockBackend *blk, bool eject_flag)
{
BlockDriverState *bs = blk_bs(blk);
char *id;
IO_CODE();
if (bs) {
-bdrv_eject(bs, eject_flag);
+bdrv_co_eject(bs, eject_flag);
}
/* Whether or not we ejected on the backend,
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 74f7727a02..76f884a6ae 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -218,7 +218,7 @@ static int coroutine_fn
cor_co_pwritev_compressed(BlockDriverState *bs,
static void cor_eject(BlockDriverState *bs, bool eject_flag)
{
-bdrv_eject(bs->file->bs, eject_flag);
+bdrv_co_eject(bs->file->bs, eject_flag);
}
diff --git a/block/filter-compress.c b/block/filter-compress.c
index 305716c86c..571e4684dd 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -118,7 +118,7 @@ static void compress_refresh_limits(BlockDriverState *bs,
Error **errp)
static void compress_eject(BlockDriverState *bs, bool eject_flag)
{
-bdrv_eject(bs->file->bs, eject_flag);
+bdrv_co_eject(bs->file->bs, eject_flag);
}
diff --git a/block/raw-format.c b/block/raw-format.c
index 39a2f20df1..efe1ef0265 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -405,7 +405,7 @@ static int coroutine_fn raw_co_truncate(BlockDriverState
*bs, int64_t offset,
static void raw_eject(BlockDriverState *bs, bool eject_flag)
{
-bdrv_eject(bs->file->bs
[PATCH 01/14] block-coroutine-wrapper: support void functions
From: Emanuele Giuseppe Esposito
Just omit the various 'return' when the return type is void.
Signed-off-by: Emanuele Giuseppe Esposito
Signed-off-by: Kevin Wolf
---
scripts/block-coroutine-wrapper.py | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/scripts/block-coroutine-wrapper.py
b/scripts/block-coroutine-wrapper.py
index 6e087fa0b7..0c5d7782b1 100644
--- a/scripts/block-coroutine-wrapper.py
+++ b/scripts/block-coroutine-wrapper.py
@@ -85,6 +85,16 @@ def __init__(self, return_type: str, name: str, args: str,
ctx = 'qemu_get_aio_context()'
self.ctx = ctx
+self.get_result = 's->ret = '
+self.ret = 'return s.ret;'
+self.co_ret = 'return '
+self.return_field = self.return_type + " ret;"
+if self.return_type == 'void':
+self.get_result = ''
+self.ret = ''
+self.co_ret = ''
+self.return_field = ''
+
def gen_list(self, format: str) -> str:
return ', '.join(format.format_map(arg.__dict__) for arg in self.args)
@@ -131,7 +141,7 @@ def create_mixed_wrapper(func: FuncDecl) -> str:
{{
if (qemu_in_coroutine()) {{
{graph_assume_lock}
-return {name}({ func.gen_list('{name}') });
+{func.co_ret}{name}({ func.gen_list('{name}') });
}} else {{
{struct_name} s = {{
.poll_state.ctx = {func.ctx},
@@ -143,7 +153,7 @@ def create_mixed_wrapper(func: FuncDecl) -> str:
s.poll_state.co = qemu_coroutine_create({name}_entry, &s);
bdrv_poll_co(&s.poll_state);
-return s.ret;
+{func.ret}
}}
}}"""
@@ -168,7 +178,7 @@ def create_co_wrapper(func: FuncDecl) -> str:
s.poll_state.co = qemu_coroutine_create({name}_entry, &s);
bdrv_poll_co(&s.poll_state);
-return s.ret;
+{func.ret}
}}"""
@@ -195,7 +205,7 @@ def gen_wrapper(func: FuncDecl) -> str:
typedef struct {struct_name} {{
BdrvPollCo poll_state;
-{func.return_type} ret;
+{func.return_field}
{ func.gen_block('{decl};') }
}} {struct_name};
@@ -204,7 +214,7 @@ def gen_wrapper(func: FuncDecl) -> str:
{struct_name} *s = opaque;
{graph_lock}
-s->ret = {name}({ func.gen_list('s->{name}') });
+{func.get_result}{name}({ func.gen_list('s->{name}') });
{graph_unlock}
s->poll_state.in_progress = false;
--
2.38.1
Re: [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state
On 13/12/2022 09.32, Philippe Mathieu-Daudé wrote:
On 13/12/22 09:03, Thomas Huth wrote:
On 13/12/2022 08.30, Philippe Mathieu-Daudé wrote:
On 13/12/22 01:14, Richard Henderson wrote:
On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:
The device endianness doesn't change during runtime.
What are you talking about? Of course it does.
The host CPU certainly does, but the virtio device doesn't... Does it?
This check only consider the device, not the CPU:
bool virtio_access_is_big_endian(VirtIODevice *vdev)
{
#if defined(LEGACY_VIRTIO_IS_BIENDIAN)
return virtio_is_big_endian(vdev);
#elif TARGET_BIG_ENDIAN
if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
/*Devices conforming to VIRTIO 1.0 or later are always LE.*/
return false;
}
return true;
Well, this part here means that the endianness can indeed change on the
device side during runtime. Depending on whether VIRTIO_F_VERSION_1 is
negotiated or not, the device is little or big endian. Happens on s390x
for example - for legacy virtio, big endian is used, and for modern
virtio, little endian is used instead.
virtio_is_big_endian() depends on vdev->device_endian which is set in:
1) virtio_init()
void virtio_init(VirtIODevice *vdev, uint16_t device_id,
size_t config_size)
{
vdev->device_endian = virtio_default_endian();
2) virtio_load()
int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
{
int i, ret;
int32_t config_len;
uint32_t num;
uint32_t features;
BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
/*
* We poison the endianness to ensure it does not get
* used before subsections have been loaded.
*/
vdev->device_endian = VIRTIO_DEVICE_ENDIAN_UNKNOWN;
if (vdev->device_endian == VIRTIO_DEVICE_ENDIAN_UNKNOWN) {
vdev->device_endian = virtio_default_endian();
}
3) virtio_reset()
void virtio_reset(void *opaque)
{
VirtIODevice *vdev = opaque;
VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
int i;
virtio_set_status(vdev, 0);
if (current_cpu) {
/* Guest initiated reset */
vdev->device_endian = virtio_current_cpu_endian();
This is where the virtio endianness depends on the CPU endianness. Looks
like it is fortunately only taken into account after a device reset, and not
for every access (as I originally thought).
Thomas
Re: [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state
On 13/12/22 09:03, Thomas Huth wrote:
On 13/12/2022 08.30, Philippe Mathieu-Daudé wrote:
On 13/12/22 01:14, Richard Henderson wrote:
On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:
The device endianness doesn't change during runtime.
What are you talking about? Of course it does.
The host CPU certainly does, but the virtio device doesn't... Does it?
This check only consider the device, not the CPU:
bool virtio_access_is_big_endian(VirtIODevice *vdev)
{
#if defined(LEGACY_VIRTIO_IS_BIENDIAN)
return virtio_is_big_endian(vdev);
#elif TARGET_BIG_ENDIAN
if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
/*Devices conforming to VIRTIO 1.0 or later are always LE.*/
return false;
}
return true;
Well, this part here means that the endianness can indeed change on the
device side during runtime. Depending on whether VIRTIO_F_VERSION_1 is
negotiated or not, the device is little or big endian. Happens on s390x
for example - for legacy virtio, big endian is used, and for modern
virtio, little endian is used instead.
virtio_is_big_endian() depends on vdev->device_endian which is set in:
1) virtio_init()
void virtio_init(VirtIODevice *vdev, uint16_t device_id,
size_t config_size)
{
vdev->device_endian = virtio_default_endian();
2) virtio_load()
int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
{
int i, ret;
int32_t config_len;
uint32_t num;
uint32_t features;
BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
/*
* We poison the endianness to ensure it does not get
* used before subsections have been loaded.
*/
vdev->device_endian = VIRTIO_DEVICE_ENDIAN_UNKNOWN;
if (vdev->device_endian == VIRTIO_DEVICE_ENDIAN_UNKNOWN) {
vdev->device_endian = virtio_default_endian();
}
3) virtio_reset()
void virtio_reset(void *opaque)
{
VirtIODevice *vdev = opaque;
VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
int i;
virtio_set_status(vdev, 0);
if (current_cpu) {
/* Guest initiated reset */
vdev->device_endian = virtio_current_cpu_endian();
} else {
/* System reset */
vdev->device_endian = virtio_default_endian();
}
So the current patch is not complete and should be:
-- >8 --
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 09b1a0e3d9..b02b9058f9 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2124,6 +2124,7 @@ void virtio_reset(void *opaque)
/* System reset */
vdev->device_endian = virtio_default_endian();
}
+vdev->access_is_big_endian = virtio_access_is_big_endian(vdev);
if (k->reset) {
k->reset(vdev);
@@ -3018,6 +3019,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f,
int version_id)
if (vdev->device_endian == VIRTIO_DEVICE_ENDIAN_UNKNOWN) {
vdev->device_endian = virtio_default_endian();
+vdev->access_is_big_endian = virtio_access_is_big_endian(vdev);
}
if (virtio_64bit_features_needed(vdev)) {
@@ -3193,6 +3195,7 @@ void virtio_init(VirtIODevice *vdev, uint16_t
device_id, size_t config_size)
vdev->vmstate = qdev_add_vm_change_state_handler(DEVICE(vdev),
virtio_vmstate_change, vdev);
vdev->device_endian = virtio_default_endian();
+vdev->access_is_big_endian = virtio_access_is_big_endian(vdev);
vdev->use_guest_notifier_mask = true;
}
---
Still, the result of virtio_access_is_big_endian() doesn't depend on
the CPU endianness in my analysis... What am I missing?
Thanks,
Phil.
Re: [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state
On 13/12/22 08:30, Philippe Mathieu-Daudé wrote:
On 13/12/22 01:14, Richard Henderson wrote:
On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:
The device endianness doesn't change during runtime.
What are you talking about? Of course it does.
The host CPU certainly does, but the virtio device doesn't... Does it?
This check only consider the device, not the CPU:
bool virtio_access_is_big_endian(VirtIODevice *vdev)
{
#if defined(LEGACY_VIRTIO_IS_BIENDIAN)
return virtio_is_big_endian(vdev);
#elif TARGET_BIG_ENDIAN
if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
/*Devices conforming to VIRTIO 1.0 or later are always LE.*/
return false;
}
return true;
#else
return false;
#endif
}
static inline bool virtio_is_big_endian(VirtIODevice *vdev)
{
if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN);
return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG;
}
/* Devices conforming to VIRTIO 1.0 or later are always LE. */
return false;
}
and once the features are negotiated it doesn't seem to change.
Per the spec, if the device changes its endianness, it must set the
VIRTIO_CONFIG_S_NEEDS_RESET bit:
3.2.1 Notification of Device Configuration Changes
For devices where the device-specific configuration information
can be changed, a configuration change notification is sent when
a device-specific configuration change occurs.
In addition, this notification is triggered by the device setting
DEVICE_NEEDS_RESET
However QEMU doesn't read this bit, only sets it:
hw/virtio/virtio.c:3551:vdev->status = vdev->status |
VIRTIO_CONFIG_S_NEEDS_RESET;
include/standard-headers/linux/virtio_config.h:44:#define
VIRTIO_CONFIG_S_NEEDS_RESET 0x40
I mean, it doesn't often in practice, because the Linux kernel is
compiled for one endianness and doesn't keep toggling state, but the
hooks that you're replacing test for the *current* endianness state of
the cpu. So this is a behaviour change.
I agree. Note however currently the CPU endianness is only checked once
upon virtio device reset (or from a migration stream):
void virtio_reset(void *opaque)
{
VirtIODevice *vdev = opaque;
VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
int i;
virtio_set_status(vdev, 0);
if (current_cpu) {
/* Guest initiated reset */
vdev->device_endian = virtio_current_cpu_endian();
} else {
/* System reset */
vdev->device_endian = virtio_default_endian();
}
bool cpu_virtio_is_big_endian(CPUState *cpu)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
if (cc->sysemu_ops->virtio_is_big_endian) {
return cc->sysemu_ops->virtio_is_big_endian(cpu);
}
return target_words_bigendian();
}
ARM being the single arch implementing a runtime endianness check:
static bool arm_cpu_virtio_is_big_endian(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
cpu_synchronize_state(cs);
return arm_cpu_data_is_big_endian(env);
}
virtio_reset() is only called by virtio_bus_reset().
$ git grep -w virtio_bus_reset
hw/s390x/virtio-ccw.c:256:virtio_bus_reset(&dev->bus);
hw/virtio/virtio-bus.c:102:void virtio_bus_reset(VirtioBusState *bus)
hw/virtio/virtio-mmio.c:75:virtio_bus_reset(&proxy->bus);
hw/virtio/virtio-pci.c:1998:virtio_bus_reset(bus);
So the result of virtio_access_is_big_endian() is only updated there,
after a virtio_bus_reset() call, and IIUC isn't dependent on the CPU
endianness state, thus can be cached in VirtIODevice. But maybe the
current implementation is incomplete and my change is simply making
it worst...
Re: [PATCH v4 0/2] Add OCP extended log to nvme QEMU
On Nov 25 10:48, Joel Granados wrote:
> The motivation and description are contained in the last patch in this set.
> Will copy paste it here for convenience:
>
> In order to evaluate write amplification factor (WAF) within the storage
> stack it is important to know the number of bytes written to the
> controller. The existing SMART log value of Data Units Written is too
> coarse (given in units of 500 Kb) and so we add the SMART health
> information extended from the OCP specification (given in units of bytes).
>
> To accommodate different vendor specific specifications like OCP, we add a
> multiplexing function (nvme_vendor_specific_log) which will route to the
> different log functions based on arguments and log ids. We only return the
> OCP extended smart log when the command is 0xC0 and ocp has been turned on
> in the args.
>
> Though we add the whole nvme smart log extended structure, we only
> populate
> the physical_media_units_{read,written}, log_page_version and
> log_page_uuid.
>
> V4 changes:
> 1. Fixed cpu_to_le64 instead of cpu_to_le32
> 2. Variable naming : uuid -> guid
> 3. Changed how the guid value appears in the code:
>Used to be:
> smart_l.log_page_uuid[0] = 0xA4F2BFEA2810AFC5;
> smart_l.log_page_uuid[1] = 0xAFD514C97C6F4F9C;
>
>Now is:
> static const uint8_t guid[16] = {
> 0xC5, 0xAF, 0x10, 0x28, 0xEA, 0xBF, 0xF2, 0xA4,
> 0x9C, 0x4F, 0x6F, 0x7C, 0xC9, 0x14, 0xD5, 0xAF
> };
>
>This is different from what @klaus suggested because I want to keep it
>consistent to what nvme-cli currently implements. I think here we can
>either change both nvme-cli and this patch or leave the order of the
>bytes as they are here. This all depends on how you interpret the Spec
>(which is ambiguous)
>
> V3 changes:
> 1. Corrected a bunch of checkpatch issues. Since I changed the first patch
>I did not include the reviewed-by.
> 2. Included some documentation in nvme.rst for the ocp argument
> 3. Squashed the ocp arg changes into the main patch.
> 4. Fixed several comments and an open parenthesis
> 5. Hex values are now in lower case.
> 6. Change the reserved format to rsvd
> 7. Made sure that NvmeCtrl is the first arg in all the functions.
> 8. Fixed comment on commit of main patch
>
> V2 changes:
> 1. I moved the ocp parameter from the namespace to the subsystem as it is
>defined there in the OCP specification
> 2. I now accumulate statistics from all namespaces and report them back on
>the extended log as per the spec.
> 3. I removed the default case in the switch in nvme_vendor_specific_log as
>it does not have any special function.
>
> Joel Granados (2):
> nvme: Move adjustment of data_units{read,written}
> nvme: Add physical writes/reads from OCP log
>
> docs/system/devices/nvme.rst | 7
> hw/nvme/ctrl.c | 73 +---
> hw/nvme/nvme.h | 1 +
> include/block/nvme.h | 36 ++
> 4 files changed, 111 insertions(+), 6 deletions(-)
>
> --
> 2.30.2
>
LGTM,
Reviewed-by: Klaus Jensen
signature.asc
Description: PGP signature
Re: [RFC PATCH-for-8.0 06/10] hw/virtio: Cache access_is_big_endian value in VirtIODevice state
On 13/12/2022 08.30, Philippe Mathieu-Daudé wrote:
On 13/12/22 01:14, Richard Henderson wrote:
On 12/12/22 17:05, Philippe Mathieu-Daudé wrote:
The device endianness doesn't change during runtime.
What are you talking about? Of course it does.
The host CPU certainly does, but the virtio device doesn't... Does it?
This check only consider the device, not the CPU:
bool virtio_access_is_big_endian(VirtIODevice *vdev)
{
#if defined(LEGACY_VIRTIO_IS_BIENDIAN)
return virtio_is_big_endian(vdev);
#elif TARGET_BIG_ENDIAN
if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
/*Devices conforming to VIRTIO 1.0 or later are always LE.*/
return false;
}
return true;
Well, this part here means that the endianness can indeed change on the
device side during runtime. Depending on whether VIRTIO_F_VERSION_1 is
negotiated or not, the device is little or big endian. Happens on s390x for
example - for legacy virtio, big endian is used, and for modern virtio,
little endian is used instead.
Thomas
