mbox series

[v4,0/5] coroutines: generate wrapper code

Message ID 20200525100801.13859-1-vsementsov@virtuozzo.com
Headers show
Series coroutines: generate wrapper code | expand

Message

Vladimir Sementsov-Ogievskiy May 25, 2020, 10:07 a.m. UTC 
Hi all!

The aim of the series is to reduce code-duplication and writing
parameters structure-packing by hand around coroutine function wrappers.

It's an alternative to "[PATCH v3] block: Factor out bdrv_run_co()"
patch.

Benefits:
 - no code duplication
 - less indirection

v4:
01: wording in commit message + Eric's r-b
02: add copyright header into new header
03: - add copyright headers into new headers (most funny is
                                              generated-co-wrapper.h)
    - fix Makefiles (hope this will help patchew, code builds for me
                     even without fixes, I don't know why)
    - fix extra new-line at the end of generated block/block-gen.c


For convenience I attach generated block/block-gen.c file below.

Vladimir Sementsov-Ogievskiy (5):
  block/io: refactor coroutine wrappers
  block: declare some coroutine functions in block/coroutines.h
  block: generate coroutine-wrapper code
  block: drop bdrv_prwv
  block/io: refactor save/load vmstate

 Makefile                             |   6 +
 Makefile.objs                        |   2 +-
 block/block-gen.h                    |  55 ++++
 block/coroutines.h                   |  66 +++++
 include/block/block.h                |  25 +-
 include/block/generated-co-wrapper.h |  35 +++
 block.c                              |  78 +-----
 block/io.c                           | 383 ++++-----------------------
 tests/test-bdrv-drain.c              |   2 +-
 block/Makefile.objs                  |   1 +
 scripts/coroutine-wrapper.py         | 168 ++++++++++++
 11 files changed, 401 insertions(+), 420 deletions(-)
 create mode 100644 block/block-gen.h
 create mode 100644 block/coroutines.h
 create mode 100644 include/block/generated-co-wrapper.h
 create mode 100755 scripts/coroutine-wrapper.py

=== Generated by these series block/block-gen.c ===
/*
 * File is generated by scripts/coroutine-wrapper.py
 */

#include "qemu/osdep.h"
#include "block/coroutines.h"
#include "block/block-gen.h"


/*
 * Wrappers for bdrv_co_truncate
 */

typedef struct BdrvCoTruncate {
    BdrvPollCo poll_state;
    BdrvChild *child;
    int64_t offset;
    bool exact;
    PreallocMode prealloc;
    BdrvRequestFlags flags;
    Error **errp;
} BdrvCoTruncate;

static void coroutine_fn bdrv_co_truncate_entry(void *opaque)
{
    BdrvCoTruncate *s = opaque;

    s->poll_state.ret = bdrv_co_truncate(s->child, s->offset, s->exact, s->prealloc, s->flags, s->errp);

    s->poll_state.in_progress = false;

    bdrv_poll_co__on_exit();
}

int bdrv_truncate(BdrvChild *child, int64_t offset, bool exact, PreallocMode prealloc, BdrvRequestFlags flags, Error **errp)
{
    if (qemu_in_coroutine()) {
        return bdrv_co_truncate(child, offset, exact, prealloc, flags, errp);
    } else {
        BdrvCoTruncate s = {
            .poll_state.bs = child->bs,
            .poll_state.in_progress = true,

            .child = child,
            .offset = offset,
            .exact = exact,
            .prealloc = prealloc,
            .flags = flags,
            .errp = errp,
        };

        s.poll_state.co = qemu_coroutine_create(bdrv_co_truncate_entry, &s);

        return bdrv_poll_co(&s.poll_state);
    }
}


/*
 * Wrappers for bdrv_co_check
 */

typedef struct BdrvCoCheck {
    BdrvPollCo poll_state;
    BlockDriverState *bs;
    BdrvCheckResult *res;
    BdrvCheckMode fix;
} BdrvCoCheck;

static void coroutine_fn bdrv_co_check_entry(void *opaque)
{
    BdrvCoCheck *s = opaque;

    s->poll_state.ret = bdrv_co_check(s->bs, s->res, s->fix);

    s->poll_state.in_progress = false;

    bdrv_poll_co__on_exit();
}

int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix)
{
    if (qemu_in_coroutine()) {
        return bdrv_co_check(bs, res, fix);
    } else {
        BdrvCoCheck s = {
            .poll_state.bs = bs,
            .poll_state.in_progress = true,

            .bs = bs,
            .res = res,
            .fix = fix,
        };

        s.poll_state.co = qemu_coroutine_create(bdrv_co_check_entry, &s);

        return bdrv_poll_co(&s.poll_state);
    }
}


/*
 * Wrappers for bdrv_co_invalidate_cache
 */

typedef struct BdrvCoInvalidateCache {
    BdrvPollCo poll_state;
    BlockDriverState *bs;
    Error **errp;
} BdrvCoInvalidateCache;

static void coroutine_fn bdrv_co_invalidate_cache_entry(void *opaque)
{
    BdrvCoInvalidateCache *s = opaque;

    bdrv_co_invalidate_cache(s->bs, s->errp);

    s->poll_state.in_progress = false;

    bdrv_poll_co__on_exit();
}

void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
{
    if (qemu_in_coroutine()) {
        bdrv_co_invalidate_cache(bs, errp);
    } else {
        BdrvCoInvalidateCache s = {
            .poll_state.bs = bs,
            .poll_state.in_progress = true,

            .bs = bs,
            .errp = errp,
        };

        s.poll_state.co = qemu_coroutine_create(bdrv_co_invalidate_cache_entry, &s);

        bdrv_poll_co(&s.poll_state);
    }
}


/*
 * Wrappers for bdrv_co_flush
 */

typedef struct BdrvCoFlush {
    BdrvPollCo poll_state;
    BlockDriverState *bs;
} BdrvCoFlush;

static void coroutine_fn bdrv_co_flush_entry(void *opaque)
{
    BdrvCoFlush *s = opaque;

    s->poll_state.ret = bdrv_co_flush(s->bs);

    s->poll_state.in_progress = false;

    bdrv_poll_co__on_exit();
}

int bdrv_flush(BlockDriverState *bs)
{
    if (qemu_in_coroutine()) {
        return bdrv_co_flush(bs);
    } else {
        BdrvCoFlush s = {
            .poll_state.bs = bs,
            .poll_state.in_progress = true,

            .bs = bs,
        };

        s.poll_state.co = qemu_coroutine_create(bdrv_co_flush_entry, &s);

        return bdrv_poll_co(&s.poll_state);
    }
}


/*
 * Wrappers for bdrv_co_pdiscard
 */

typedef struct BdrvCoPdiscard {
    BdrvPollCo poll_state;
    BdrvChild *child;
    int64_t offset;
    int64_t bytes;
} BdrvCoPdiscard;

static void coroutine_fn bdrv_co_pdiscard_entry(void *opaque)
{
    BdrvCoPdiscard *s = opaque;

    s->poll_state.ret = bdrv_co_pdiscard(s->child, s->offset, s->bytes);

    s->poll_state.in_progress = false;

    bdrv_poll_co__on_exit();
}

int bdrv_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes)
{
    if (qemu_in_coroutine()) {
        return bdrv_co_pdiscard(child, offset, bytes);
    } else {
        BdrvCoPdiscard s = {
            .poll_state.bs = child->bs,
            .poll_state.in_progress = true,

            .child = child,
            .offset = offset,
            .bytes = bytes,
        };

        s.poll_state.co = qemu_coroutine_create(bdrv_co_pdiscard_entry, &s);

        return bdrv_poll_co(&s.poll_state);
    }
}


/*
 * Wrappers for bdrv_co_readv_vmstate
 */

typedef struct BdrvCoReadvVmstate {
    BdrvPollCo poll_state;
    BlockDriverState *bs;
    QEMUIOVector *qiov;
    int64_t pos;
} BdrvCoReadvVmstate;

static void coroutine_fn bdrv_co_readv_vmstate_entry(void *opaque)
{
    BdrvCoReadvVmstate *s = opaque;

    s->poll_state.ret = bdrv_co_readv_vmstate(s->bs, s->qiov, s->pos);

    s->poll_state.in_progress = false;

    bdrv_poll_co__on_exit();
}

int bdrv_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
{
    if (qemu_in_coroutine()) {
        return bdrv_co_readv_vmstate(bs, qiov, pos);
    } else {
        BdrvCoReadvVmstate s = {
            .poll_state.bs = bs,
            .poll_state.in_progress = true,

            .bs = bs,
            .qiov = qiov,
            .pos = pos,
        };

        s.poll_state.co = qemu_coroutine_create(bdrv_co_readv_vmstate_entry, &s);

        return bdrv_poll_co(&s.poll_state);
    }
}


/*
 * Wrappers for bdrv_co_writev_vmstate
 */

typedef struct BdrvCoWritevVmstate {
    BdrvPollCo poll_state;
    BlockDriverState *bs;
    QEMUIOVector *qiov;
    int64_t pos;
} BdrvCoWritevVmstate;

static void coroutine_fn bdrv_co_writev_vmstate_entry(void *opaque)
{
    BdrvCoWritevVmstate *s = opaque;

    s->poll_state.ret = bdrv_co_writev_vmstate(s->bs, s->qiov, s->pos);

    s->poll_state.in_progress = false;

    bdrv_poll_co__on_exit();
}

int bdrv_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
{
    if (qemu_in_coroutine()) {
        return bdrv_co_writev_vmstate(bs, qiov, pos);
    } else {
        BdrvCoWritevVmstate s = {
            .poll_state.bs = bs,
            .poll_state.in_progress = true,

            .bs = bs,
            .qiov = qiov,
            .pos = pos,
        };

        s.poll_state.co = qemu_coroutine_create(bdrv_co_writev_vmstate_entry, &s);

        return bdrv_poll_co(&s.poll_state);
    }
}


/*
 * Wrappers for bdrv_co_preadv
 */

typedef struct BdrvCoPreadv {
    BdrvPollCo poll_state;
    BdrvChild *child;
    int64_t offset;
    unsigned int bytes;
    QEMUIOVector *qiov;
    BdrvRequestFlags flags;
} BdrvCoPreadv;

static void coroutine_fn bdrv_co_preadv_entry(void *opaque)
{
    BdrvCoPreadv *s = opaque;

    s->poll_state.ret = bdrv_co_preadv(s->child, s->offset, s->bytes, s->qiov, s->flags);

    s->poll_state.in_progress = false;

    bdrv_poll_co__on_exit();
}

int bdrv_preadv(BdrvChild *child, int64_t offset, unsigned int bytes, QEMUIOVector *qiov, BdrvRequestFlags flags)
{
    if (qemu_in_coroutine()) {
        return bdrv_co_preadv(child, offset, bytes, qiov, flags);
    } else {
        BdrvCoPreadv s = {
            .poll_state.bs = child->bs,
            .poll_state.in_progress = true,

            .child = child,
            .offset = offset,
            .bytes = bytes,
            .qiov = qiov,
            .flags = flags,
        };

        s.poll_state.co = qemu_coroutine_create(bdrv_co_preadv_entry, &s);

        return bdrv_poll_co(&s.poll_state);
    }
}


/*
 * Wrappers for bdrv_co_pwritev
 */

typedef struct BdrvCoPwritev {
    BdrvPollCo poll_state;
    BdrvChild *child;
    int64_t offset;
    unsigned int bytes;
    QEMUIOVector *qiov;
    BdrvRequestFlags flags;
} BdrvCoPwritev;

static void coroutine_fn bdrv_co_pwritev_entry(void *opaque)
{
    BdrvCoPwritev *s = opaque;

    s->poll_state.ret = bdrv_co_pwritev(s->child, s->offset, s->bytes, s->qiov, s->flags);

    s->poll_state.in_progress = false;

    bdrv_poll_co__on_exit();
}

int bdrv_pwritev(BdrvChild *child, int64_t offset, unsigned int bytes, QEMUIOVector *qiov, BdrvRequestFlags flags)
{
    if (qemu_in_coroutine()) {
        return bdrv_co_pwritev(child, offset, bytes, qiov, flags);
    } else {
        BdrvCoPwritev s = {
            .poll_state.bs = child->bs,
            .poll_state.in_progress = true,

            .child = child,
            .offset = offset,
            .bytes = bytes,
            .qiov = qiov,
            .flags = flags,
        };

        s.poll_state.co = qemu_coroutine_create(bdrv_co_pwritev_entry, &s);

        return bdrv_poll_co(&s.poll_state);
    }
}


/*
 * Wrappers for bdrv_co_common_block_status_above
 */

typedef struct BdrvCoCommonBlockStatusAbove {
    BdrvPollCo poll_state;
    BlockDriverState *bs;
    BlockDriverState *base;
    bool want_zero;
    int64_t offset;
    int64_t bytes;
    int64_t *pnum;
    int64_t *map;
    BlockDriverState **file;
} BdrvCoCommonBlockStatusAbove;

static void coroutine_fn bdrv_co_common_block_status_above_entry(void *opaque)
{
    BdrvCoCommonBlockStatusAbove *s = opaque;

    s->poll_state.ret = bdrv_co_common_block_status_above(s->bs, s->base, s->want_zero, s->offset, s->bytes, s->pnum, s->map, s->file);

    s->poll_state.in_progress = false;

    bdrv_poll_co__on_exit();
}

int bdrv_common_block_status_above(BlockDriverState *bs, BlockDriverState *base, bool want_zero, int64_t offset, int64_t bytes, int64_t *pnum, int64_t *map, BlockDriverState **file)
{
    if (qemu_in_coroutine()) {
        return bdrv_co_common_block_status_above(bs, base, want_zero, offset, bytes, pnum, map, file);
    } else {
        BdrvCoCommonBlockStatusAbove s = {
            .poll_state.bs = bs,
            .poll_state.in_progress = true,

            .bs = bs,
            .base = base,
            .want_zero = want_zero,
            .offset = offset,
            .bytes = bytes,
            .pnum = pnum,
            .map = map,
            .file = file,
        };

        s.poll_state.co = qemu_coroutine_create(bdrv_co_common_block_status_above_entry, &s);

        return bdrv_poll_co(&s.poll_state);
    }
}
=== End of generated block/block-gen.c ===

Comments

no-reply@patchew.org May 25, 2020, 1:14 p.m. UTC | #1 
Patchew URL: https://patchew.org/QEMU/20200525100801.13859-1-vsementsov@virtuozzo.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

block/vhdx-log.o: In function `vhdx_log_write_and_flush':
/tmp/qemu-test/src/block/vhdx-log.c:1049: undefined reference to `bdrv_flush'
/tmp/qemu-test/src/block/vhdx-log.c:1061: undefined reference to `bdrv_flush'
collect2: error: ld returned 1 exit status
make: *** [qemu-nbd] Error 1
make: *** Waiting for unfinished jobs....
  LINK    vhost-user-input
block.o: In function `bdrv_invalidate_cache_all':
---
block/vhdx-log.o: In function `vhdx_log_write_and_flush':
/tmp/qemu-test/src/block/vhdx-log.c:1049: undefined reference to `bdrv_flush'
/tmp/qemu-test/src/block/vhdx-log.c:1061: undefined reference to `bdrv_flush'
collect2: error: ld returned 1 exit status
make: *** [qemu-storage-daemon] Error 1
block.o: In function `bdrv_invalidate_cache_all':
/tmp/qemu-test/src/block.c:5697: undefined reference to `bdrv_invalidate_cache'
block.o: In function `bdrv_close':
---
block/vhdx-log.o: In function `vhdx_log_write_and_flush':
/tmp/qemu-test/src/block/vhdx-log.c:1049: undefined reference to `bdrv_flush'
/tmp/qemu-test/src/block/vhdx-log.c:1061: undefined reference to `bdrv_flush'
collect2: error: ld returned 1 exit status
make: *** [qemu-io] Error 1
  GEN     x86_64-softmmu/hmp-commands.h
  GEN     x86_64-softmmu/hmp-commands-info.h
  GEN     x86_64-softmmu/config-devices.h
---
../block/vhdx-log.o: In function `vhdx_log_write_and_flush':
/tmp/qemu-test/src/block/vhdx-log.c:1049: undefined reference to `bdrv_flush'
/tmp/qemu-test/src/block/vhdx-log.c:1061: undefined reference to `bdrv_flush'
collect2: error: ld returned 1 exit status
make[1]: *** [qemu-system-x86_64] Error 1
make: *** [x86_64-softmmu/all] Error 2
../blockdev.o: In function `external_snapshot_prepare':
/tmp/qemu-test/src/blockdev.c:1480: undefined reference to `bdrv_flush'
../block.o: In function `bdrv_invalidate_cache_all':
---
../block/vhdx-log.o: In function `vhdx_log_write_and_flush':
/tmp/qemu-test/src/block/vhdx-log.c:1049: undefined reference to `bdrv_flush'
/tmp/qemu-test/src/block/vhdx-log.c:1061: undefined reference to `bdrv_flush'
collect2: error: ld returned 1 exit status
make[1]: *** [qemu-system-aarch64] Error 1
make: *** [aarch64-softmmu/all] Error 2
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
    sys.exit(main())
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=cb38533df12a483595ddf084eb0a9493', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-ga_jl87d/src/docker-src.2020-05-25-09.11.03.18391:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=cb38533df12a483595ddf084eb0a9493
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-ga_jl87d/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    3m21.247s
user    0m8.473s


The full log is available at
http://patchew.org/logs/20200525100801.13859-1-vsementsov@virtuozzo.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Vladimir Sementsov-Ogievskiy May 25, 2020, 1:48 p.m. UTC | #2 
25.05.2020 16:14, no-reply@patchew.org wrote:
> Patchew URL:https://patchew.org/QEMU/20200525100801.13859-1-vsementsov@virtuozzo.com/
> 
> 
> 
> Hi,
> 
> This series failed the docker-quick@centos7 build test. Please find the testing commands and
> their output below. If you have Docker installed, you can probably reproduce it
> locally.
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> make docker-image-centos7 V=1 NETWORK=1
> time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
> === TEST SCRIPT END ===
> 
> block/vhdx-log.o: In function `vhdx_log_write_and_flush':
> /tmp/qemu-test/src/block/vhdx-log.c:1049: undefined reference to `bdrv_flush'
> /tmp/qemu-test/src/block/vhdx-log.c:1061: undefined reference to `bdrv_flush'
> collect2: error: ld returned 1 exit status
> make: *** [qemu-nbd] Error 1

Hmm. Who can help?

I assume, that this is because I've added block/block-gen.o into ./Makefile.objs, and not into block/Makefile.objs. I'll try it with next resend.
Eric Blake May 26, 2020, 8:42 p.m. UTC | #3 
On 5/25/20 8:48 AM, Vladimir Sementsov-Ogievskiy wrote:
> 25.05.2020 16:14, no-reply@patchew.org wrote:
>> Patchew 
>> URL:https://patchew.org/QEMU/20200525100801.13859-1-vsementsov@virtuozzo.com/ 
>>
>>
>>
>>
>> Hi,
>>
>> This series failed the docker-quick@centos7 build test. Please find 
>> the testing commands and
>> their output below. If you have Docker installed, you can probably 
>> reproduce it
>> locally.
>>
>> === TEST SCRIPT BEGIN ===
>> #!/bin/bash
>> make docker-image-centos7 V=1 NETWORK=1
>> time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
>> === TEST SCRIPT END ===
>>
>> block/vhdx-log.o: In function `vhdx_log_write_and_flush':
>> /tmp/qemu-test/src/block/vhdx-log.c:1049: undefined reference to 
>> `bdrv_flush'
>> /tmp/qemu-test/src/block/vhdx-log.c:1061: undefined reference to 
>> `bdrv_flush'
>> collect2: error: ld returned 1 exit status
>> make: *** [qemu-nbd] Error 1
> 
> Hmm. Who can help?
> 
> I assume, that this is because I've added block/block-gen.o into 
> ./Makefile.objs, and not into block/Makefile.objs. I'll try it with next 
> resend.

Are you doing in-tree or VPATH builds?  When I tried a VPATH build, I got:

$ make -C build block/block-gen.c V=1
make: Entering directory '/home/eblake/qemu/build'
...
cat include/block/block.h block/coroutines.h | 
/home/eblake/qemu/scripts/coroutine-wrapper.py > block/block-gen.c
cat: include/block/block.h: No such file or directory
cat: block/coroutines.h: No such file or directory
make: 'block/block-gen.c' is up to date.
make: Leaving directory '/home/eblake/qemu/build'

and a resulting block/block-gen.c that declares nothing but 3 #includes.