From nobody Sun Apr 28 19:19:02 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1529018568985378.2636353677567; Thu, 14 Jun 2018 16:22:48 -0700 (PDT) Received: from localhost ([::1]:43457 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fTbZg-0002QN-15 for importer@patchew.org; Thu, 14 Jun 2018 19:22:40 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47048) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fTbYV-0001o9-CP for qemu-devel@nongnu.org; Thu, 14 Jun 2018 19:21:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fTbYR-0005b1-C9 for qemu-devel@nongnu.org; Thu, 14 Jun 2018 19:21:27 -0400 Received: from mail-qt0-x243.google.com ([2607:f8b0:400d:c0d::243]:39430) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fTbYR-0005aL-4e for qemu-devel@nongnu.org; Thu, 14 Jun 2018 19:21:23 -0400 Received: by mail-qt0-x243.google.com with SMTP id p23-v6so7506320qtn.6 for ; Thu, 14 Jun 2018 16:21:22 -0700 (PDT) Received: from breakout.internal.digitalocean.com (97-120-136-47.ptld.qwest.net. [97.120.136.47]) by smtp.gmail.com with ESMTPSA id u31-v6sm5220750qtc.28.2018.06.14.16.21.21 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 14 Jun 2018 16:21:21 -0700 (PDT) Received: by breakout.internal.digitalocean.com (Postfix, from userid 1000) id 5FDF28A2A52; Thu, 14 Jun 2018 16:21:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=digitalocean.com; s=google; h=from:to:cc:subject:date:message-id; bh=PjQxGqVlTTtobQnnSUHVHKlO3AAG6heHWFpaJxbGDRo=; b=QxnkOR2qg0vELPk1pttugrAGQkJCd68yb6gUJ6EwGZgGcwkxVb37trIWiF3pkHaAIG u35F18dnSPurK9bJ7r1L2l1idZSPpv1HXY9RkJG7g4ruOs3XdNCXfy2cetnOklWcY+wt bPGzpBvOHYcJzhJwjrbgMDPHe0jDZx+eQECPk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=PjQxGqVlTTtobQnnSUHVHKlO3AAG6heHWFpaJxbGDRo=; b=cTHBzoVHx1yTh+SxoUhHEnENj3tWAjTJpGLuDyQBxN5VY4WxY7uuAldWEvw/NQviMt e5u1jSaRXmESRz67ND7QoAYeji3kVINApnx4bAGV0n7AXBjltMcjwPIWw6glH/r4FWYl 11KRTyMw9yIrskvGZHUfKCBtRRbG3eGPxVfLQqyQmklJ/vqjMA6htvlQ2Mla9YmOvm6z 2nmPhUnuxkA6xcjrzyuFC7IUeVnT9g9ZakJ+HId+4o2UiTCZ8Rv30v+CU8bJNGc8z9ef PPBuU9SUEr7nr6IE3WYlYiQTqQj6sCWTM3L55RYJylKpfJNuyED6v9rxPO3cYlCEArpq X19A== X-Gm-Message-State: APt69E3YpoJZLPKmd8I8+xnpOHua3MOlB+AZZjW3/KGNu7arVpuSWpzX t+rzGT04vwdk8ha73F3xyEL1wg== X-Google-Smtp-Source: ADUXVKLPCg4j9sHT5npJYeSFkKqTCVSo9/ZTHpofAXloz1y234k0FKFoc5vCknNAsQVrtcn+44yY1A== X-Received: by 2002:ac8:281:: with SMTP id p1-v6mr4433868qtg.274.1529018482264; Thu, 14 Jun 2018 16:21:22 -0700 (PDT) To: qemu-devel@nongnu.org Date: Thu, 14 Jun 2018 16:21:19 -0700 Message-Id: <20180614232119.31669-1-naravamudan@digitalocean.com> X-Mailer: git-send-email 2.17.1 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:400d:c0d::243 Subject: [Qemu-devel] [PATCH] [RFC] aio: properly bubble up errors from initialization X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Nishanth Aravamudan via Qemu-devel Reply-To: Nishanth Aravamudan Cc: Nishanth Aravamudan , qemu-block@nongnu.org Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZohoMail: RDKM_2 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" laio_init() can fail for a couple of reasons, which will lead to a NULL pointer dereference in laio_attach_aio_context(). To solve this, add a aio_linux_aio_setup() path which is called where aio_get_linux_aio() is called currently, but can propogate errors up. virtio-block and virtio-scsi call this new function before calling blk_io_plug() (which eventually calls aio_get_linux_aio). This is necessary because plug/unplug currently assume they do not fail. It is trivial to make qemu segfault in my testing. Set /proc/sys/fs/aio-max-nr to 0 and start a guest with aio=3Dnative,cache=3Ddirectsync. With this patch, the guest successfully starts (but obviously isn't using native AIO). Setting aio-max-nr back up to a reasonable value, AIO contexts are consumed normally. Signed-off-by: Nishanth Aravamudan --- block/block-backend.c | 10 ++++++++++ block/file-posix.c | 27 +++++++++++++++++++++++++++ block/io.c | 27 +++++++++++++++++++++++++++ block/linux-aio.c | 15 ++++++++++----- hw/block/virtio-blk.c | 4 ++++ hw/scsi/virtio-scsi.c | 4 ++++ include/block/aio.h | 3 +++ include/block/block.h | 1 + include/block/block_int.h | 1 + include/block/raw-aio.h | 2 +- include/sysemu/block-backend.h | 1 + stubs/linux-aio.c | 2 +- util/async.c | 14 +++++++++++--- 13 files changed, 101 insertions(+), 10 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index d55c328736..2a18bb2af4 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1946,6 +1946,16 @@ void blk_add_insert_bs_notifier(BlockBackend *blk, N= otifier *notify) notifier_list_add(&blk->insert_bs_notifiers, notify); } =20 +int blk_io_plug_setup(BlockBackend *blk) +{ + BlockDriverState *bs =3D blk_bs(blk); + + if (bs) { + return bdrv_io_plug_setup(bs); + } + return 0; +} + void blk_io_plug(BlockBackend *blk) { BlockDriverState *bs =3D blk_bs(blk); diff --git a/block/file-posix.c b/block/file-posix.c index 07bb061fe4..adaba7c366 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -1665,6 +1665,12 @@ static int coroutine_fn raw_co_prw(BlockDriverState = *bs, uint64_t offset, type |=3D QEMU_AIO_MISALIGNED; #ifdef CONFIG_LINUX_AIO } else if (s->use_linux_aio) { + int rc; + rc =3D aio_linux_aio_setup(bdrv_get_aio_context(bs)); + if (rc !=3D 0) { + s->use_linux_aio =3D 0; + return rc; + } LinuxAioState *aio =3D aio_get_linux_aio(bdrv_get_aio_context(= bs)); assert(qiov->size =3D=3D bytes); return laio_co_submit(bs, aio, s->fd, offset, qiov, type); @@ -1690,12 +1696,28 @@ static int coroutine_fn raw_co_pwritev(BlockDriverS= tate *bs, uint64_t offset, return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_WRITE); } =20 +static int raw_aio_plug_setup(BlockDriverState *bs) +{ + int rc =3D 0; +#ifdef CONFIG_LINUX_AIO + BDRVRawState *s =3D bs->opaque; + if (s->use_linux_aio) { + rc =3D aio_linux_aio_setup(bdrv_get_aio_context(bs)); + if (rc !=3D 0) { + s->use_linux_aio =3D 0; + } + } +#endif + return rc; +} + static void raw_aio_plug(BlockDriverState *bs) { #ifdef CONFIG_LINUX_AIO BDRVRawState *s =3D bs->opaque; if (s->use_linux_aio) { LinuxAioState *aio =3D aio_get_linux_aio(bdrv_get_aio_context(bs)); + assert(aio !=3D NULL); laio_io_plug(bs, aio); } #endif @@ -1707,6 +1729,7 @@ static void raw_aio_unplug(BlockDriverState *bs) BDRVRawState *s =3D bs->opaque; if (s->use_linux_aio) { LinuxAioState *aio =3D aio_get_linux_aio(bdrv_get_aio_context(bs)); + assert(aio !=3D NULL); laio_io_unplug(bs, aio); } #endif @@ -2599,6 +2622,7 @@ BlockDriver bdrv_file =3D { .bdrv_co_copy_range_from =3D raw_co_copy_range_from, .bdrv_co_copy_range_to =3D raw_co_copy_range_to, .bdrv_refresh_limits =3D raw_refresh_limits, + .bdrv_io_plug_setup =3D raw_aio_plug_setup, .bdrv_io_plug =3D raw_aio_plug, .bdrv_io_unplug =3D raw_aio_unplug, =20 @@ -3079,6 +3103,7 @@ static BlockDriver bdrv_host_device =3D { .bdrv_co_copy_range_from =3D raw_co_copy_range_from, .bdrv_co_copy_range_to =3D raw_co_copy_range_to, .bdrv_refresh_limits =3D raw_refresh_limits, + .bdrv_io_plug_setup =3D raw_aio_plug_setup, .bdrv_io_plug =3D raw_aio_plug, .bdrv_io_unplug =3D raw_aio_unplug, =20 @@ -3201,6 +3226,7 @@ static BlockDriver bdrv_host_cdrom =3D { .bdrv_co_pwritev =3D raw_co_pwritev, .bdrv_aio_flush =3D raw_aio_flush, .bdrv_refresh_limits =3D raw_refresh_limits, + .bdrv_io_plug_setup =3D raw_aio_plug_setup, .bdrv_io_plug =3D raw_aio_plug, .bdrv_io_unplug =3D raw_aio_unplug, =20 @@ -3331,6 +3357,7 @@ static BlockDriver bdrv_host_cdrom =3D { .bdrv_co_pwritev =3D raw_co_pwritev, .bdrv_aio_flush =3D raw_aio_flush, .bdrv_refresh_limits =3D raw_refresh_limits, + .bdrv_io_plug_setup =3D raw_aio_plug_setup, .bdrv_io_plug =3D raw_aio_plug, .bdrv_io_unplug =3D raw_aio_unplug, =20 diff --git a/block/io.c b/block/io.c index b7beaeeb9f..3c9711f6ed 100644 --- a/block/io.c +++ b/block/io.c @@ -2779,6 +2779,33 @@ void bdrv_add_before_write_notifier(BlockDriverState= *bs, notifier_with_return_list_add(&bs->before_write_notifiers, notifier); } =20 +int bdrv_io_plug_setup(BlockDriverState *bs) +{ + int rc; + BdrvChild *child; + + QLIST_FOREACH(child, &bs->children, next) { + // XXX: do we need to undo the successful setups if we fail midway? + rc =3D bdrv_io_plug_setup(child->bs); + if (rc !=3D 0) { + return rc; + } + } + + if (atomic_fetch_inc(&bs->io_plugged) =3D=3D 0) { + BlockDriver *drv =3D bs->drv; + if (drv && drv->bdrv_io_plug_setup) { + rc =3D drv->bdrv_io_plug_setup(bs); + // XXX: do we need to undo the successful setups if we fail mi= dway? + if (rc !=3D 0) { + return rc; + } + } + } + + return 0; +} + void bdrv_io_plug(BlockDriverState *bs) { BdrvChild *child; diff --git a/block/linux-aio.c b/block/linux-aio.c index 88b8d55ec7..4d799f85fe 100644 --- a/block/linux-aio.c +++ b/block/linux-aio.c @@ -470,28 +470,33 @@ void laio_attach_aio_context(LinuxAioState *s, AioCon= text *new_context) qemu_laio_poll_cb); } =20 -LinuxAioState *laio_init(void) +int laio_init(LinuxAioState **linux_aio) { + int rc; LinuxAioState *s; =20 s =3D g_malloc0(sizeof(*s)); - if (event_notifier_init(&s->e, false) < 0) { + rc =3D event_notifier_init(&s->e, false); + if (rc < 0) { goto out_free_state; } =20 - if (io_setup(MAX_EVENTS, &s->ctx) !=3D 0) { + rc =3D io_setup(MAX_EVENTS, &s->ctx); + if (rc !=3D 0) { goto out_close_efd; } =20 ioq_init(&s->io_q); =20 - return s; + *linux_aio =3D s; + return 0; =20 out_close_efd: event_notifier_cleanup(&s->e); out_free_state: g_free(s); - return NULL; + *linux_aio =3D NULL; + return rc; } =20 void laio_cleanup(LinuxAioState *s) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 50b5c869e3..a2cd008a4c 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -598,6 +598,9 @@ bool virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq) bool progress =3D false; =20 aio_context_acquire(blk_get_aio_context(s->blk)); + if (blk_io_plug_setup(s->blk) !=3D 0) { + goto error; + } blk_io_plug(s->blk); =20 do { @@ -620,6 +623,7 @@ bool virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq) } =20 blk_io_unplug(s->blk); +error: aio_context_release(blk_get_aio_context(s->blk)); return progress; } diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 3aa99717e2..7d17f082d5 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -569,6 +569,10 @@ static int virtio_scsi_handle_cmd_req_prepare(VirtIOSC= SI *s, VirtIOSCSIReq *req) return -ENOBUFS; } scsi_req_ref(req->sreq); + rc =3D blk_io_plug_setup(d->conf.blk); + if (rc !=3D 0) { + return rc; + } blk_io_plug(d->conf.blk); return 0; } diff --git a/include/block/aio.h b/include/block/aio.h index ae6f354e6c..64881c24b9 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -381,6 +381,9 @@ GSource *aio_get_g_source(AioContext *ctx); /* Return the ThreadPool bound to this AioContext */ struct ThreadPool *aio_get_thread_pool(AioContext *ctx); =20 +/* Setup the LinuxAioState bound to this AioContext */ +int aio_linux_aio_setup(AioContext *ctx); + /* Return the LinuxAioState bound to this AioContext */ struct LinuxAioState *aio_get_linux_aio(AioContext *ctx); =20 diff --git a/include/block/block.h b/include/block/block.h index e677080c4e..2974f64ad2 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -548,6 +548,7 @@ void bdrv_set_aio_context(BlockDriverState *bs, AioCont= ext *new_context); int bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz); int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo); =20 +int bdrv_io_plug_setup(BlockDriverState *bs); void bdrv_io_plug(BlockDriverState *bs); void bdrv_io_unplug(BlockDriverState *bs); =20 diff --git a/include/block/block_int.h b/include/block/block_int.h index 327e478a73..2e5ffbdc4f 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -388,6 +388,7 @@ struct BlockDriver { AioContext *new_context); =20 /* io queue for linux-aio */ + int (*bdrv_io_plug_setup)(BlockDriverState *bs); void (*bdrv_io_plug)(BlockDriverState *bs); void (*bdrv_io_unplug)(BlockDriverState *bs); =20 diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h index 0e717fd475..81b90e5fc6 100644 --- a/include/block/raw-aio.h +++ b/include/block/raw-aio.h @@ -43,7 +43,7 @@ /* linux-aio.c - Linux native implementation */ #ifdef CONFIG_LINUX_AIO typedef struct LinuxAioState LinuxAioState; -LinuxAioState *laio_init(void); +int laio_init(LinuxAioState **linux_aio); void laio_cleanup(LinuxAioState *s); int coroutine_fn laio_co_submit(BlockDriverState *bs, LinuxAioState *s, in= t fd, uint64_t offset, QEMUIOVector *qiov, int t= ype); diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 8d03d493c2..7a1093f8f0 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -197,6 +197,7 @@ void blk_remove_aio_context_notifier(BlockBackend *blk, void *opaque); void blk_add_remove_bs_notifier(BlockBackend *blk, Notifier *notify); void blk_add_insert_bs_notifier(BlockBackend *blk, Notifier *notify); +int blk_io_plug_setup(BlockBackend *blk); void blk_io_plug(BlockBackend *blk); void blk_io_unplug(BlockBackend *blk); BlockAcctStats *blk_get_stats(BlockBackend *blk); diff --git a/stubs/linux-aio.c b/stubs/linux-aio.c index ed47bd443c..88ab927e35 100644 --- a/stubs/linux-aio.c +++ b/stubs/linux-aio.c @@ -21,7 +21,7 @@ void laio_attach_aio_context(LinuxAioState *s, AioContext= *new_context) abort(); } =20 -LinuxAioState *laio_init(void) +int laio_init(LinuxAioState **linux_aio) { abort(); } diff --git a/util/async.c b/util/async.c index 03f62787f2..34534aae63 100644 --- a/util/async.c +++ b/util/async.c @@ -323,12 +323,20 @@ ThreadPool *aio_get_thread_pool(AioContext *ctx) } =20 #ifdef CONFIG_LINUX_AIO -LinuxAioState *aio_get_linux_aio(AioContext *ctx) +int aio_linux_aio_setup(AioContext *ctx) { + int rc =3D 0; if (!ctx->linux_aio) { - ctx->linux_aio =3D laio_init(); - laio_attach_aio_context(ctx->linux_aio, ctx); + rc =3D laio_init(&ctx->linux_aio); + if (rc =3D=3D 0) { + laio_attach_aio_context(ctx->linux_aio, ctx); + } } + return rc; +} + +LinuxAioState *aio_get_linux_aio(AioContext *ctx) +{ return ctx->linux_aio; } #endif --=20 2.17.1