From nobody Tue Apr 8 16:37:08 2025 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; 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; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1535726161602588.330565537334; Fri, 31 Aug 2018 07:36:01 -0700 (PDT) Received: from localhost ([::1]:53991 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkWm-0007BV-De for importer@patchew.org; Fri, 31 Aug 2018 10:36:00 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50667) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkQZ-0001A5-5g for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:29:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fvkMA-0000kY-0T for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:25:08 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:50280 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fvkM4-0000ip-CM; Fri, 31 Aug 2018 10:24:58 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0BDAB80825A9; Fri, 31 Aug 2018 14:24:51 +0000 (UTC) Received: from localhost (ovpn-117-235.ams2.redhat.com [10.36.117.235]) by smtp.corp.redhat.com (Postfix) with ESMTPS id B162663F57; Fri, 31 Aug 2018 14:24:50 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Fri, 31 Aug 2018 16:24:36 +0200 Message-Id: <20180831142446.22264-2-mreitz@redhat.com> In-Reply-To: <20180831142446.22264-1-mreitz@redhat.com> References: <20180831142446.22264-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 31 Aug 2018 14:24:51 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 31 Aug 2018 14:24:51 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mreitz@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PULL 01/11] file-posix: Skip effectiveless OFD lock operations 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: , Cc: Kevin Wolf , Peter Maydell , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RDMRC_1 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: Fam Zheng If we know we've already locked the bytes, don't do it again; similarly don't unlock a byte if we haven't locked it. This doesn't change the behavior, but fixes a corner case explained below. Libvirt had an error handling bug that an image can get its (ownership, file mode, SELinux) permissions changed (RHBZ 1584982) by mistake behind QEMU. Specifically, an image in use by Libvirt VM has: $ ls -lhZ b.img -rw-r--r--. qemu qemu system_u:object_r:svirt_image_t:s0:c600,c690 b.img Trying to attach it a second time won't work because of image locking. And after the error, it becomes: $ ls -lhZ b.img -rw-r--r--. root root system_u:object_r:virt_image_t:s0 b.img Then, we won't be able to do OFD lock operations with the existing fd. In other words, the code such as in blk_detach_dev: blk_set_perm(blk, 0, BLK_PERM_ALL, &error_abort); can abort() QEMU, out of environmental changes. This patch is an easy fix to this and the change is regardlessly reasonable, so do it. Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/file-posix.c | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/block/file-posix.c b/block/file-posix.c index fe83cbf0eb..73ae00c8c5 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -680,23 +680,42 @@ typedef enum { * file; if @unlock =3D=3D true, also unlock the unneeded bytes. * @shared_perm_lock_bits is the mask of all permissions that are NOT shar= ed. */ -static int raw_apply_lock_bytes(int fd, +static int raw_apply_lock_bytes(BDRVRawState *s, int fd, uint64_t perm_lock_bits, uint64_t shared_perm_lock_bits, bool unlock, Error **errp) { int ret; int i; + uint64_t locked_perm, locked_shared_perm; + + if (s) { + locked_perm =3D s->perm; + locked_shared_perm =3D ~s->shared_perm & BLK_PERM_ALL; + } else { + /* + * We don't have the previous bits, just lock/unlock for each of t= he + * requested bits. + */ + if (unlock) { + locked_perm =3D BLK_PERM_ALL; + locked_shared_perm =3D BLK_PERM_ALL; + } else { + locked_perm =3D 0; + locked_shared_perm =3D 0; + } + } =20 PERM_FOREACH(i) { int off =3D RAW_LOCK_PERM_BASE + i; - if (perm_lock_bits & (1ULL << i)) { + uint64_t bit =3D (1ULL << i); + if ((perm_lock_bits & bit) && !(locked_perm & bit)) { ret =3D qemu_lock_fd(fd, off, 1, false); if (ret) { error_setg(errp, "Failed to lock byte %d", off); return ret; } - } else if (unlock) { + } else if (unlock && (locked_perm & bit) && !(perm_lock_bits & bit= )) { ret =3D qemu_unlock_fd(fd, off, 1); if (ret) { error_setg(errp, "Failed to unlock byte %d", off); @@ -706,13 +725,15 @@ static int raw_apply_lock_bytes(int fd, } PERM_FOREACH(i) { int off =3D RAW_LOCK_SHARED_BASE + i; - if (shared_perm_lock_bits & (1ULL << i)) { + uint64_t bit =3D (1ULL << i); + if ((shared_perm_lock_bits & bit) && !(locked_shared_perm & bit)) { ret =3D qemu_lock_fd(fd, off, 1, false); if (ret) { error_setg(errp, "Failed to lock byte %d", off); return ret; } - } else if (unlock) { + } else if (unlock && (locked_shared_perm & bit) && + !(shared_perm_lock_bits & bit)) { ret =3D qemu_unlock_fd(fd, off, 1); if (ret) { error_setg(errp, "Failed to unlock byte %d", off); @@ -788,7 +809,7 @@ static int raw_handle_perm_lock(BlockDriverState *bs, =20 switch (op) { case RAW_PL_PREPARE: - ret =3D raw_apply_lock_bytes(s->lock_fd, s->perm | new_perm, + ret =3D raw_apply_lock_bytes(s, s->lock_fd, s->perm | new_perm, ~s->shared_perm | ~new_shared, false, errp); if (!ret) { @@ -800,7 +821,7 @@ static int raw_handle_perm_lock(BlockDriverState *bs, op =3D RAW_PL_ABORT; /* fall through to unlock bytes. */ case RAW_PL_ABORT: - raw_apply_lock_bytes(s->lock_fd, s->perm, ~s->shared_perm, + raw_apply_lock_bytes(s, s->lock_fd, s->perm, ~s->shared_perm, true, &local_err); if (local_err) { /* Theoretically the above call only unlocks bytes and it cann= ot @@ -810,7 +831,7 @@ static int raw_handle_perm_lock(BlockDriverState *bs, } break; case RAW_PL_COMMIT: - raw_apply_lock_bytes(s->lock_fd, new_perm, ~new_shared, + raw_apply_lock_bytes(s, s->lock_fd, new_perm, ~new_shared, true, &local_err); if (local_err) { /* Theoretically the above call only unlocks bytes and it cann= ot @@ -2209,7 +2230,7 @@ raw_co_create(BlockdevCreateOptions *options, Error *= *errp) shared =3D BLK_PERM_ALL & ~BLK_PERM_RESIZE; =20 /* Step one: Take locks */ - result =3D raw_apply_lock_bytes(fd, perm, ~shared, false, errp); + result =3D raw_apply_lock_bytes(NULL, fd, perm, ~shared, false, errp); if (result < 0) { goto out_close; } @@ -2250,7 +2271,7 @@ raw_co_create(BlockdevCreateOptions *options, Error *= *errp) } =20 out_unlock: - raw_apply_lock_bytes(fd, 0, 0, true, &local_err); + raw_apply_lock_bytes(NULL, fd, 0, 0, true, &local_err); if (local_err) { /* The above call should not fail, and if it does, that does * not mean the whole creation operation has failed. So --=20 2.17.1 From nobody Tue Apr 8 16:37:08 2025 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; 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; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1535725946710175.94419944602942; Fri, 31 Aug 2018 07:32:26 -0700 (PDT) Received: from localhost ([::1]:53968 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkTG-0002sv-4G for importer@patchew.org; Fri, 31 Aug 2018 10:32:22 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50714) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkQZ-0001As-6e for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:29:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fvkM8-0000k5-81 for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:25:01 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:47792 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fvkM2-0000jA-NT; Fri, 31 Aug 2018 10:24:55 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 474E040200A8; Fri, 31 Aug 2018 14:24:53 +0000 (UTC) Received: from localhost (ovpn-117-235.ams2.redhat.com [10.36.117.235]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CDD642166B41; Fri, 31 Aug 2018 14:24:52 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Fri, 31 Aug 2018 16:24:37 +0200 Message-Id: <20180831142446.22264-3-mreitz@redhat.com> In-Reply-To: <20180831142446.22264-1-mreitz@redhat.com> References: <20180831142446.22264-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Fri, 31 Aug 2018 14:24:53 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Fri, 31 Aug 2018 14:24:53 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mreitz@redhat.com' RCPT:'' Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PULL 02/11] tests: fix bdrv-drain leak 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: , Cc: Kevin Wolf , Peter Maydell , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RDMRC_1 RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" From: Marc-Andr=C3=A9 Lureau Spotted by ASAN: =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D =3D=3D5378=3D=3DERROR: LeakSanitizer: detected memory leaks Direct leak of 65536 byte(s) in 1 object(s) allocated from: #0 0x7f788f83bc48 in malloc (/lib64/libasan.so.5+0xeec48) #1 0x7f788c9923c5 in g_malloc (/lib64/libglib-2.0.so.0+0x523c5) #2 0x5622a1fe37bc in coroutine_trampoline /home/elmarco/src/qq/util/cor= outine-ucontext.c:116 #3 0x7f788a15d75f in __correctly_grouped_prefixwc (/lib64/libc.so.6+0x4= c75f) (Broken in commit 4c8158e359d.) Signed-off-by: Marc-Andr=C3=A9 Lureau Message-id: 20180809114417.28718-3-marcandre.lureau@redhat.com Signed-off-by: Max Reitz --- tests/test-bdrv-drain.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index 17bb8508ae..abc8bbe6f0 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -948,6 +948,7 @@ static void coroutine_fn test_co_delete_by_drain(void *= opaque) } =20 dbdd->done =3D true; + g_free(buffer); } =20 /** --=20 2.17.1 From nobody Tue Apr 8 16:37:08 2025 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; 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; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1535725992619258.39711195110215; Fri, 31 Aug 2018 07:33:12 -0700 (PDT) Received: from localhost ([::1]:53969 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkU3-0003k2-Dq for importer@patchew.org; Fri, 31 Aug 2018 10:33:11 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50527) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkQU-00014U-KQ for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:29:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fvkMG-0000mi-4o for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:25:10 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:48980 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fvkM6-0000jK-6y; Fri, 31 Aug 2018 10:24:58 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9F63940241D4; Fri, 31 Aug 2018 14:24:55 +0000 (UTC) Received: from localhost (ovpn-117-235.ams2.redhat.com [10.36.117.235]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 05F472166B41; Fri, 31 Aug 2018 14:24:54 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Fri, 31 Aug 2018 16:24:38 +0200 Message-Id: <20180831142446.22264-4-mreitz@redhat.com> In-Reply-To: <20180831142446.22264-1-mreitz@redhat.com> References: <20180831142446.22264-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Fri, 31 Aug 2018 14:24:55 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Fri, 31 Aug 2018 14:24:55 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mreitz@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PULL 03/11] jobs: change start callback to run callback 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: , Cc: Kevin Wolf , Peter Maydell , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RDMRC_1 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: John Snow Presently we codify the entry point for a job as the "start" callback, but a more apt name would be "run" to clarify the idea that when this function returns we consider the job to have "finished," except for any cleanup which occurs in separate callbacks later. As part of this clarification, change the signature to include an error object and a return code. The error ptr is not yet used, and the return code while captured, will be overwritten by actions in the job_completed function. Signed-off-by: John Snow Reviewed-by: Max Reitz Message-id: 20180830015734.19765-2-jsnow@redhat.com Reviewed-by: Jeff Cody Signed-off-by: Max Reitz --- include/qemu/job.h | 2 +- block/backup.c | 7 ++++--- block/commit.c | 7 ++++--- block/create.c | 8 +++++--- block/mirror.c | 10 ++++++---- block/stream.c | 7 ++++--- job.c | 6 +++--- tests/test-bdrv-drain.c | 7 ++++--- tests/test-blockjob-txn.c | 16 ++++++++-------- tests/test-blockjob.c | 7 ++++--- 10 files changed, 43 insertions(+), 34 deletions(-) diff --git a/include/qemu/job.h b/include/qemu/job.h index 18c9223e31..9cf463d228 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -169,7 +169,7 @@ struct JobDriver { JobType job_type; =20 /** Mandatory: Entrypoint for the Coroutine. */ - CoroutineEntry *start; + int coroutine_fn (*run)(Job *job, Error **errp); =20 /** * If the callback is not NULL, it will be invoked when the job transi= tions diff --git a/block/backup.c b/block/backup.c index 8630d32926..5d47781840 100644 --- a/block/backup.c +++ b/block/backup.c @@ -480,9 +480,9 @@ static void backup_incremental_init_copy_bitmap(BackupB= lockJob *job) bdrv_dirty_iter_free(dbi); } =20 -static void coroutine_fn backup_run(void *opaque) +static int coroutine_fn backup_run(Job *opaque_job, Error **errp) { - BackupBlockJob *job =3D opaque; + BackupBlockJob *job =3D container_of(opaque_job, BackupBlockJob, commo= n.job); BackupCompleteData *data; BlockDriverState *bs =3D blk_bs(job->common.blk); int64_t offset, nb_clusters; @@ -587,6 +587,7 @@ static void coroutine_fn backup_run(void *opaque) data =3D g_malloc(sizeof(*data)); data->ret =3D ret; job_defer_to_main_loop(&job->common.job, backup_complete, data); + return ret; } =20 static const BlockJobDriver backup_job_driver =3D { @@ -596,7 +597,7 @@ static const BlockJobDriver backup_job_driver =3D { .free =3D block_job_free, .user_resume =3D block_job_user_resume, .drain =3D block_job_drain, - .start =3D backup_run, + .run =3D backup_run, .commit =3D backup_commit, .abort =3D backup_abort, .clean =3D backup_clean, diff --git a/block/commit.c b/block/commit.c index eb414579bd..a0ea86ff64 100644 --- a/block/commit.c +++ b/block/commit.c @@ -134,9 +134,9 @@ static void commit_complete(Job *job, void *opaque) bdrv_unref(top); } =20 -static void coroutine_fn commit_run(void *opaque) +static int coroutine_fn commit_run(Job *job, Error **errp) { - CommitBlockJob *s =3D opaque; + CommitBlockJob *s =3D container_of(job, CommitBlockJob, common.job); CommitCompleteData *data; int64_t offset; uint64_t delay_ns =3D 0; @@ -213,6 +213,7 @@ out: data =3D g_malloc(sizeof(*data)); data->ret =3D ret; job_defer_to_main_loop(&s->common.job, commit_complete, data); + return ret; } =20 static const BlockJobDriver commit_job_driver =3D { @@ -222,7 +223,7 @@ static const BlockJobDriver commit_job_driver =3D { .free =3D block_job_free, .user_resume =3D block_job_user_resume, .drain =3D block_job_drain, - .start =3D commit_run, + .run =3D commit_run, }, }; =20 diff --git a/block/create.c b/block/create.c index 915cd41bcc..04733c3618 100644 --- a/block/create.c +++ b/block/create.c @@ -45,9 +45,9 @@ static void blockdev_create_complete(Job *job, void *opaq= ue) job_completed(job, s->ret, s->err); } =20 -static void coroutine_fn blockdev_create_run(void *opaque) +static int coroutine_fn blockdev_create_run(Job *job, Error **errp) { - BlockdevCreateJob *s =3D opaque; + BlockdevCreateJob *s =3D container_of(job, BlockdevCreateJob, common); =20 job_progress_set_remaining(&s->common, 1); s->ret =3D s->drv->bdrv_co_create(s->opts, &s->err); @@ -55,12 +55,14 @@ static void coroutine_fn blockdev_create_run(void *opaq= ue) =20 qapi_free_BlockdevCreateOptions(s->opts); job_defer_to_main_loop(&s->common, blockdev_create_complete, NULL); + + return s->ret; } =20 static const JobDriver blockdev_create_job_driver =3D { .instance_size =3D sizeof(BlockdevCreateJob), .job_type =3D JOB_TYPE_CREATE, - .start =3D blockdev_create_run, + .run =3D blockdev_create_run, }; =20 void qmp_blockdev_create(const char *job_id, BlockdevCreateOptions *option= s, diff --git a/block/mirror.c b/block/mirror.c index 6cc10df5c9..691763db41 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -812,9 +812,9 @@ static int mirror_flush(MirrorBlockJob *s) return ret; } =20 -static void coroutine_fn mirror_run(void *opaque) +static int coroutine_fn mirror_run(Job *job, Error **errp) { - MirrorBlockJob *s =3D opaque; + MirrorBlockJob *s =3D container_of(job, MirrorBlockJob, common.job); MirrorExitData *data; BlockDriverState *bs =3D s->mirror_top_bs->backing->bs; BlockDriverState *target_bs =3D blk_bs(s->target); @@ -1041,7 +1041,9 @@ immediate_exit: if (need_drain) { bdrv_drained_begin(bs); } + job_defer_to_main_loop(&s->common.job, mirror_exit, data); + return ret; } =20 static void mirror_complete(Job *job, Error **errp) @@ -1138,7 +1140,7 @@ static const BlockJobDriver mirror_job_driver =3D { .free =3D block_job_free, .user_resume =3D block_job_user_resume, .drain =3D block_job_drain, - .start =3D mirror_run, + .run =3D mirror_run, .pause =3D mirror_pause, .complete =3D mirror_complete, }, @@ -1154,7 +1156,7 @@ static const BlockJobDriver commit_active_job_driver = =3D { .free =3D block_job_free, .user_resume =3D block_job_user_resume, .drain =3D block_job_drain, - .start =3D mirror_run, + .run =3D mirror_run, .pause =3D mirror_pause, .complete =3D mirror_complete, }, diff --git a/block/stream.c b/block/stream.c index 9264b68a1e..b4b987df7e 100644 --- a/block/stream.c +++ b/block/stream.c @@ -97,9 +97,9 @@ out: g_free(data); } =20 -static void coroutine_fn stream_run(void *opaque) +static int coroutine_fn stream_run(Job *job, Error **errp) { - StreamBlockJob *s =3D opaque; + StreamBlockJob *s =3D container_of(job, StreamBlockJob, common.job); StreamCompleteData *data; BlockBackend *blk =3D s->common.blk; BlockDriverState *bs =3D blk_bs(blk); @@ -206,6 +206,7 @@ out: data =3D g_malloc(sizeof(*data)); data->ret =3D ret; job_defer_to_main_loop(&s->common.job, stream_complete, data); + return ret; } =20 static const BlockJobDriver stream_job_driver =3D { @@ -213,7 +214,7 @@ static const BlockJobDriver stream_job_driver =3D { .instance_size =3D sizeof(StreamBlockJob), .job_type =3D JOB_TYPE_STREAM, .free =3D block_job_free, - .start =3D stream_run, + .run =3D stream_run, .user_resume =3D block_job_user_resume, .drain =3D block_job_drain, }, diff --git a/job.c b/job.c index e36ebaafd8..76988f6678 100644 --- a/job.c +++ b/job.c @@ -544,16 +544,16 @@ static void coroutine_fn job_co_entry(void *opaque) { Job *job =3D opaque; =20 - assert(job && job->driver && job->driver->start); + assert(job && job->driver && job->driver->run); job_pause_point(job); - job->driver->start(job); + job->ret =3D job->driver->run(job, NULL); } =20 =20 void job_start(Job *job) { assert(job && !job_started(job) && job->paused && - job->driver && job->driver->start); + job->driver && job->driver->run); job->co =3D qemu_coroutine_create(job_co_entry, job); job->pause_count--; job->busy =3D true; diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index abc8bbe6f0..39ee723c8e 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -757,9 +757,9 @@ static void test_job_completed(Job *job, void *opaque) job_completed(job, 0, NULL); } =20 -static void coroutine_fn test_job_start(void *opaque) +static int coroutine_fn test_job_run(Job *job, Error **errp) { - TestBlockJob *s =3D opaque; + TestBlockJob *s =3D container_of(job, TestBlockJob, common.job); =20 job_transition_to_ready(&s->common.job); while (!s->should_complete) { @@ -771,6 +771,7 @@ static void coroutine_fn test_job_start(void *opaque) } =20 job_defer_to_main_loop(&s->common.job, test_job_completed, NULL); + return 0; } =20 static void test_job_complete(Job *job, Error **errp) @@ -785,7 +786,7 @@ BlockJobDriver test_job_driver =3D { .free =3D block_job_free, .user_resume =3D block_job_user_resume, .drain =3D block_job_drain, - .start =3D test_job_start, + .run =3D test_job_run, .complete =3D test_job_complete, }, }; diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c index 58d9b87fb2..3194924071 100644 --- a/tests/test-blockjob-txn.c +++ b/tests/test-blockjob-txn.c @@ -38,25 +38,25 @@ static void test_block_job_complete(Job *job, void *opa= que) bdrv_unref(bs); } =20 -static void coroutine_fn test_block_job_run(void *opaque) +static int coroutine_fn test_block_job_run(Job *job, Error **errp) { - TestBlockJob *s =3D opaque; - BlockJob *job =3D &s->common; + TestBlockJob *s =3D container_of(job, TestBlockJob, common.job); =20 while (s->iterations--) { if (s->use_timer) { - job_sleep_ns(&job->job, 0); + job_sleep_ns(job, 0); } else { - job_yield(&job->job); + job_yield(job); } =20 - if (job_is_cancelled(&job->job)) { + if (job_is_cancelled(job)) { break; } } =20 - job_defer_to_main_loop(&job->job, test_block_job_complete, + job_defer_to_main_loop(job, test_block_job_complete, (void *)(intptr_t)s->rc); + return s->rc; } =20 typedef struct { @@ -80,7 +80,7 @@ static const BlockJobDriver test_block_job_driver =3D { .free =3D block_job_free, .user_resume =3D block_job_user_resume, .drain =3D block_job_drain, - .start =3D test_block_job_run, + .run =3D test_block_job_run, }, }; =20 diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c index cb42f06e61..b0462bfdec 100644 --- a/tests/test-blockjob.c +++ b/tests/test-blockjob.c @@ -176,9 +176,9 @@ static void cancel_job_complete(Job *job, Error **errp) s->should_complete =3D true; } =20 -static void coroutine_fn cancel_job_start(void *opaque) +static int coroutine_fn cancel_job_run(Job *job, Error **errp) { - CancelJob *s =3D opaque; + CancelJob *s =3D container_of(job, CancelJob, common.job); =20 while (!s->should_complete) { if (job_is_cancelled(&s->common.job)) { @@ -194,6 +194,7 @@ static void coroutine_fn cancel_job_start(void *opaque) =20 defer: job_defer_to_main_loop(&s->common.job, cancel_job_completed, s); + return 0; } =20 static const BlockJobDriver test_cancel_driver =3D { @@ -202,7 +203,7 @@ static const BlockJobDriver test_cancel_driver =3D { .free =3D block_job_free, .user_resume =3D block_job_user_resume, .drain =3D block_job_drain, - .start =3D cancel_job_start, + .run =3D cancel_job_run, .complete =3D cancel_job_complete, }, }; --=20 2.17.1 From nobody Tue Apr 8 16:37:08 2025 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; 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; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 15357267366244.452631733464841; Fri, 31 Aug 2018 07:45:36 -0700 (PDT) Received: from localhost ([::1]:54059 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkg3-0001LE-Fn for importer@patchew.org; Fri, 31 Aug 2018 10:45:35 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50671) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkQV-0001AA-3w for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:29:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fvkMG-0000md-3v for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:25:10 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:36202 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fvkM7-0000jh-Js; Fri, 31 Aug 2018 10:25:00 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 53DB340216E7; Fri, 31 Aug 2018 14:24:58 +0000 (UTC) Received: from localhost (ovpn-117-235.ams2.redhat.com [10.36.117.235]) by smtp.corp.redhat.com (Postfix) with ESMTPS id AB51B202706B; Fri, 31 Aug 2018 14:24:57 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Fri, 31 Aug 2018 16:24:39 +0200 Message-Id: <20180831142446.22264-5-mreitz@redhat.com> In-Reply-To: <20180831142446.22264-1-mreitz@redhat.com> References: <20180831142446.22264-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Fri, 31 Aug 2018 14:24:58 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Fri, 31 Aug 2018 14:24:58 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mreitz@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PULL 04/11] jobs: canonize Error object 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: , Cc: Kevin Wolf , Peter Maydell , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RDMRC_1 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: John Snow Jobs presently use both an Error object in the case of the create job, and char strings in the case of generic errors elsewhere. Unify the two paths as just j->err, and remove the extra argument from job_completed. The integer error code for job_completed is kept for now, to be removed shortly in a separate patch. Signed-off-by: John Snow Message-id: 20180830015734.19765-3-jsnow@redhat.com [mreitz: Dropped a superfluous g_strdup()] Reviewed-by: Eric Blake Signed-off-by: Max Reitz --- include/qemu/job.h | 14 ++++++++------ block/backup.c | 2 +- block/commit.c | 2 +- block/create.c | 5 ++--- block/mirror.c | 2 +- block/stream.c | 2 +- job-qmp.c | 5 +++-- job.c | 18 ++++++------------ tests/test-bdrv-drain.c | 2 +- tests/test-blockjob-txn.c | 2 +- tests/test-blockjob.c | 2 +- 11 files changed, 26 insertions(+), 30 deletions(-) diff --git a/include/qemu/job.h b/include/qemu/job.h index 9cf463d228..e0e99870a1 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -124,12 +124,16 @@ typedef struct Job { /** Estimated progress_current value at the completion of the job */ int64_t progress_total; =20 - /** Error string for a failed job (NULL if, and only if, job->ret =3D= =3D 0) */ - char *error; - /** ret code passed to job_completed. */ int ret; =20 + /** + * Error object for a failed job. + * If job->ret is nonzero and an error object was not set, it will be = set + * to strerror(-job->ret) during job_completed. + */ + Error *err; + /** The completion function that will be called when the job completes= . */ BlockCompletionFunc *cb; =20 @@ -484,15 +488,13 @@ void job_transition_to_ready(Job *job); /** * @job: The job being completed. * @ret: The status code. - * @error: The error message for a failing job (only with @ret < 0). If @r= et is - * negative, but NULL is given for @error, strerror() is used. * * Marks @job as completed. If @ret is non-zero, the job transaction it is= part * of is aborted. If @ret is zero, the job moves into the WAITING state. I= f it * is the last job to complete in its transaction, all jobs in the transac= tion * move from WAITING to PENDING. */ -void job_completed(Job *job, int ret, Error *error); +void job_completed(Job *job, int ret); =20 /** Asynchronously complete the specified @job. */ void job_complete(Job *job, Error **errp); diff --git a/block/backup.c b/block/backup.c index 5d47781840..1e965d54e5 100644 --- a/block/backup.c +++ b/block/backup.c @@ -388,7 +388,7 @@ static void backup_complete(Job *job, void *opaque) { BackupCompleteData *data =3D opaque; =20 - job_completed(job, data->ret, NULL); + job_completed(job, data->ret); g_free(data); } =20 diff --git a/block/commit.c b/block/commit.c index a0ea86ff64..4a17bb73ec 100644 --- a/block/commit.c +++ b/block/commit.c @@ -117,7 +117,7 @@ static void commit_complete(Job *job, void *opaque) * bdrv_set_backing_hd() to fail. */ block_job_remove_all_bdrv(bjob); =20 - job_completed(job, ret, NULL); + job_completed(job, ret); g_free(data); =20 /* If bdrv_drop_intermediate() didn't already do that, remove the comm= it diff --git a/block/create.c b/block/create.c index 04733c3618..26a385c6c7 100644 --- a/block/create.c +++ b/block/create.c @@ -35,14 +35,13 @@ typedef struct BlockdevCreateJob { BlockDriver *drv; BlockdevCreateOptions *opts; int ret; - Error *err; } BlockdevCreateJob; =20 static void blockdev_create_complete(Job *job, void *opaque) { BlockdevCreateJob *s =3D container_of(job, BlockdevCreateJob, common); =20 - job_completed(job, s->ret, s->err); + job_completed(job, s->ret); } =20 static int coroutine_fn blockdev_create_run(Job *job, Error **errp) @@ -50,7 +49,7 @@ static int coroutine_fn blockdev_create_run(Job *job, Err= or **errp) BlockdevCreateJob *s =3D container_of(job, BlockdevCreateJob, common); =20 job_progress_set_remaining(&s->common, 1); - s->ret =3D s->drv->bdrv_co_create(s->opts, &s->err); + s->ret =3D s->drv->bdrv_co_create(s->opts, errp); job_progress_update(&s->common, 1); =20 qapi_free_BlockdevCreateOptions(s->opts); diff --git a/block/mirror.c b/block/mirror.c index 691763db41..be5dc6b7b0 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -710,7 +710,7 @@ static void mirror_exit(Job *job, void *opaque) blk_insert_bs(bjob->blk, mirror_top_bs, &error_abort); =20 bs_opaque->job =3D NULL; - job_completed(job, data->ret, NULL); + job_completed(job, data->ret); =20 g_free(data); bdrv_drained_end(src); diff --git a/block/stream.c b/block/stream.c index b4b987df7e..26a775386b 100644 --- a/block/stream.c +++ b/block/stream.c @@ -93,7 +93,7 @@ out: } =20 g_free(s->backing_file_str); - job_completed(job, data->ret, NULL); + job_completed(job, data->ret); g_free(data); } =20 diff --git a/job-qmp.c b/job-qmp.c index 410775df61..a969b2bbf0 100644 --- a/job-qmp.c +++ b/job-qmp.c @@ -146,8 +146,9 @@ static JobInfo *job_query_single(Job *job, Error **errp) .status =3D job->status, .current_progress =3D job->progress_current, .total_progress =3D job->progress_total, - .has_error =3D !!job->error, - .error =3D g_strdup(job->error), + .has_error =3D !!job->err, + .error =3D job->err ? \ + g_strdup(error_get_pretty(job->err)) : NULL, }; =20 return info; diff --git a/job.c b/job.c index 76988f6678..7b3721d2c7 100644 --- a/job.c +++ b/job.c @@ -369,7 +369,7 @@ void job_unref(Job *job) =20 QLIST_REMOVE(job, job_list); =20 - g_free(job->error); + error_free(job->err); g_free(job->id); g_free(job); } @@ -546,7 +546,7 @@ static void coroutine_fn job_co_entry(void *opaque) =20 assert(job && job->driver && job->driver->run); job_pause_point(job); - job->ret =3D job->driver->run(job, NULL); + job->ret =3D job->driver->run(job, &job->err); } =20 =20 @@ -666,8 +666,8 @@ static void job_update_rc(Job *job) job->ret =3D -ECANCELED; } if (job->ret) { - if (!job->error) { - job->error =3D g_strdup(strerror(-job->ret)); + if (!job->err) { + error_setg(&job->err, "%s", strerror(-job->ret)); } job_state_transition(job, JOB_STATUS_ABORTING); } @@ -865,17 +865,11 @@ static void job_completed_txn_success(Job *job) } } =20 -void job_completed(Job *job, int ret, Error *error) +void job_completed(Job *job, int ret) { assert(job && job->txn && !job_is_completed(job)); =20 job->ret =3D ret; - if (error) { - assert(job->ret < 0); - job->error =3D g_strdup(error_get_pretty(error)); - error_free(error); - } - job_update_rc(job); trace_job_completed(job, ret, job->ret); if (job->ret) { @@ -893,7 +887,7 @@ void job_cancel(Job *job, bool force) } job_cancel_async(job, force); if (!job_started(job)) { - job_completed(job, -ECANCELED, NULL); + job_completed(job, -ECANCELED); } else if (job->deferred_to_main_loop) { job_completed_txn_abort(job); } else { diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index 39ee723c8e..6bda4451c1 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -754,7 +754,7 @@ typedef struct TestBlockJob { =20 static void test_job_completed(Job *job, void *opaque) { - job_completed(job, 0, NULL); + job_completed(job, 0); } =20 static int coroutine_fn test_job_run(Job *job, Error **errp) diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c index 3194924071..82cedee78b 100644 --- a/tests/test-blockjob-txn.c +++ b/tests/test-blockjob-txn.c @@ -34,7 +34,7 @@ static void test_block_job_complete(Job *job, void *opaqu= e) rc =3D -ECANCELED; } =20 - job_completed(job, rc, NULL); + job_completed(job, rc); bdrv_unref(bs); } =20 diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c index b0462bfdec..408a226939 100644 --- a/tests/test-blockjob.c +++ b/tests/test-blockjob.c @@ -167,7 +167,7 @@ static void cancel_job_completed(Job *job, void *opaque) { CancelJob *s =3D opaque; s->completed =3D true; - job_completed(job, 0, NULL); + job_completed(job, 0); } =20 static void cancel_job_complete(Job *job, Error **errp) --=20 2.17.1 From nobody Tue Apr 8 16:37:08 2025 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; 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; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1535726335852492.3290310141324; Fri, 31 Aug 2018 07:38:55 -0700 (PDT) Received: from localhost ([::1]:54004 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkZa-0002gl-Nb for importer@patchew.org; Fri, 31 Aug 2018 10:38:54 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50667) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkQT-0001A5-28 for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:29:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fvkMJ-0000qZ-VS for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:25:15 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:50286 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fvkMA-0000kM-1C; Fri, 31 Aug 2018 10:25:02 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B6F528077102; Fri, 31 Aug 2018 14:25:00 +0000 (UTC) Received: from localhost (ovpn-117-235.ams2.redhat.com [10.36.117.235]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4F11C2027EA0; Fri, 31 Aug 2018 14:25:00 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Fri, 31 Aug 2018 16:24:40 +0200 Message-Id: <20180831142446.22264-6-mreitz@redhat.com> In-Reply-To: <20180831142446.22264-1-mreitz@redhat.com> References: <20180831142446.22264-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 31 Aug 2018 14:25:00 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 31 Aug 2018 14:25:00 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mreitz@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PULL 05/11] jobs: add exit shim 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: , Cc: Kevin Wolf , Peter Maydell , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RDMRC_1 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: John Snow All jobs do the same thing when they leave their running loop: - Store the return code in a structure - wait to receive this structure in the main thread - signal job completion via job_completed Few jobs do anything beyond exactly this. Consolidate this exit logic for a net reduction in SLOC. More seriously, when we utilize job_defer_to_main_loop_bh to call a function that calls job_completed, job_finalize_single will run in a context where it has recursively taken the aio_context lock, which can cause hangs if it puts down a reference that causes a flush. You can observe this in practice by looking at mirror_exit's careful placement of job_completed and bdrv_unref calls. If we centralize job exiting, we can signal job completion from outside of the aio_context, which should allow for job cleanup code to run with only one lock, which makes cleanup callbacks less tricky to write. Signed-off-by: John Snow Reviewed-by: Max Reitz Message-id: 20180830015734.19765-4-jsnow@redhat.com Reviewed-by: Jeff Cody Signed-off-by: Max Reitz --- include/qemu/job.h | 11 +++++++++++ job.c | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/include/qemu/job.h b/include/qemu/job.h index e0e99870a1..1144d671a1 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -208,6 +208,17 @@ struct JobDriver { */ void (*drain)(Job *job); =20 + /** + * If the callback is not NULL, exit will be invoked from the main thr= ead + * when the job's coroutine has finished, but before transactional + * convergence; before @prepare or @abort. + * + * FIXME TODO: This callback is only temporary to transition remaining= jobs + * to prepare/commit/abort/clean callbacks and will be removed before = 3.1. + * is released. + */ + void (*exit)(Job *job); + /** * If the callback is not NULL, prepare will be invoked when all the j= obs * belonging to the same transaction complete; or upon this job's comp= letion diff --git a/job.c b/job.c index 7b3721d2c7..5df7791ad8 100644 --- a/job.c +++ b/job.c @@ -535,6 +535,18 @@ void job_drain(Job *job) } } =20 +static void job_exit(void *opaque) +{ + Job *job =3D (Job *)opaque; + AioContext *aio_context =3D job->aio_context; + + if (job->driver->exit) { + aio_context_acquire(aio_context); + job->driver->exit(job); + aio_context_release(aio_context); + } + job_completed(job, job->ret); +} =20 /** * All jobs must allow a pause point before entering their job proper. This @@ -547,6 +559,12 @@ static void coroutine_fn job_co_entry(void *opaque) assert(job && job->driver && job->driver->run); job_pause_point(job); job->ret =3D job->driver->run(job, &job->err); + if (!job->deferred_to_main_loop) { + job->deferred_to_main_loop =3D true; + aio_bh_schedule_oneshot(qemu_get_aio_context(), + job_exit, + job); + } } =20 =20 --=20 2.17.1 From nobody Tue Apr 8 16:37:08 2025 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; 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; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1535726537030371.51776805937504; Fri, 31 Aug 2018 07:42:17 -0700 (PDT) Received: from localhost ([::1]:54031 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkcp-0005Mk-PU for importer@patchew.org; Fri, 31 Aug 2018 10:42:15 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50652) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkQN-00019c-9u for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:29:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fvkMN-0000rk-4Q for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:25:16 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:48986 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fvkME-0000kq-6A; Fri, 31 Aug 2018 10:25:08 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 39A6F406DE3D; Fri, 31 Aug 2018 14:25:03 +0000 (UTC) Received: from localhost (ovpn-117-235.ams2.redhat.com [10.36.117.235]) by smtp.corp.redhat.com (Postfix) with ESMTPS id BE78F1010422; Fri, 31 Aug 2018 14:25:02 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Fri, 31 Aug 2018 16:24:41 +0200 Message-Id: <20180831142446.22264-7-mreitz@redhat.com> In-Reply-To: <20180831142446.22264-1-mreitz@redhat.com> References: <20180831142446.22264-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Fri, 31 Aug 2018 14:25:03 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Fri, 31 Aug 2018 14:25:03 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mreitz@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PULL 06/11] block/commit: utilize job_exit shim 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: , Cc: Kevin Wolf , Peter Maydell , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RDMRC_1 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: John Snow Change the manual deferment to commit_complete into the implicit callback to job_exit, renaming commit_complete to commit_exit. This conversion does change the timing of when job_completed is called to after the bdrv_replace_node and bdrv_unref calls, which could have implications for bjob->blk which will now be put down after this cleanup. Kevin highlights that we did not take any permissions for that backend at job creation time, so it is safe to reorder these operations. Signed-off-by: John Snow Reviewed-by: Max Reitz Message-id: 20180830015734.19765-5-jsnow@redhat.com Reviewed-by: Jeff Cody Signed-off-by: Max Reitz --- block/commit.c | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/block/commit.c b/block/commit.c index 4a17bb73ec..da69165de3 100644 --- a/block/commit.c +++ b/block/commit.c @@ -68,19 +68,13 @@ static int coroutine_fn commit_populate(BlockBackend *b= s, BlockBackend *base, return 0; } =20 -typedef struct { - int ret; -} CommitCompleteData; - -static void commit_complete(Job *job, void *opaque) +static void commit_exit(Job *job) { CommitBlockJob *s =3D container_of(job, CommitBlockJob, common.job); BlockJob *bjob =3D &s->common; - CommitCompleteData *data =3D opaque; BlockDriverState *top =3D blk_bs(s->top); BlockDriverState *base =3D blk_bs(s->base); BlockDriverState *commit_top_bs =3D s->commit_top_bs; - int ret =3D data->ret; bool remove_commit_top_bs =3D false; =20 /* Make sure commit_top_bs and top stay around until bdrv_replace_node= () */ @@ -91,10 +85,10 @@ static void commit_complete(Job *job, void *opaque) * the normal backing chain can be restored. */ blk_unref(s->base); =20 - if (!job_is_cancelled(job) && ret =3D=3D 0) { + if (!job_is_cancelled(job) && job->ret =3D=3D 0) { /* success */ - ret =3D bdrv_drop_intermediate(s->commit_top_bs, base, - s->backing_file_str); + job->ret =3D bdrv_drop_intermediate(s->commit_top_bs, base, + s->backing_file_str); } else { /* XXX Can (or should) we somehow keep 'consistent read' blocked e= ven * after the failed/cancelled commit job is gone? If we already wr= ote @@ -117,9 +111,6 @@ static void commit_complete(Job *job, void *opaque) * bdrv_set_backing_hd() to fail. */ block_job_remove_all_bdrv(bjob); =20 - job_completed(job, ret); - g_free(data); - /* If bdrv_drop_intermediate() didn't already do that, remove the comm= it * filter driver from the backing chain. Do this as the final step so = that * the 'consistent read' permission can be granted. */ @@ -137,7 +128,6 @@ static void commit_complete(Job *job, void *opaque) static int coroutine_fn commit_run(Job *job, Error **errp) { CommitBlockJob *s =3D container_of(job, CommitBlockJob, common.job); - CommitCompleteData *data; int64_t offset; uint64_t delay_ns =3D 0; int ret =3D 0; @@ -210,9 +200,6 @@ static int coroutine_fn commit_run(Job *job, Error **er= rp) out: qemu_vfree(buf); =20 - data =3D g_malloc(sizeof(*data)); - data->ret =3D ret; - job_defer_to_main_loop(&s->common.job, commit_complete, data); return ret; } =20 @@ -224,6 +211,7 @@ static const BlockJobDriver commit_job_driver =3D { .user_resume =3D block_job_user_resume, .drain =3D block_job_drain, .run =3D commit_run, + .exit =3D commit_exit, }, }; =20 --=20 2.17.1 From nobody Tue Apr 8 16:37:08 2025 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; 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; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1535726358116903.9368311845168; Fri, 31 Aug 2018 07:39:18 -0700 (PDT) Received: from localhost ([::1]:54010 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkZx-0002zX-0I for importer@patchew.org; Fri, 31 Aug 2018 10:39:17 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50527) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkQP-00014U-1V for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:29:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fvkML-0000rG-OB for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:25:15 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:36214 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fvkMG-0000kv-5A; Fri, 31 Aug 2018 10:25:09 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 73E63401EF02; Fri, 31 Aug 2018 14:25:05 +0000 (UTC) Received: from localhost (ovpn-117-235.ams2.redhat.com [10.36.117.235]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0E36610D16BA; Fri, 31 Aug 2018 14:25:04 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Fri, 31 Aug 2018 16:24:42 +0200 Message-Id: <20180831142446.22264-8-mreitz@redhat.com> In-Reply-To: <20180831142446.22264-1-mreitz@redhat.com> References: <20180831142446.22264-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Fri, 31 Aug 2018 14:25:05 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Fri, 31 Aug 2018 14:25:05 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mreitz@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PULL 07/11] block/mirror: utilize job_exit shim 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: , Cc: Kevin Wolf , Peter Maydell , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RDMRC_1 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: John Snow Change the manual deferment to mirror_exit into the implicit callback to job_exit and the mirror_exit callback. This does change the order of some bdrv_unref calls and job_completed, but thanks to the new context in which we call .exit, this is safe to defer the possible flushing of any nodes to the job_finalize_single cleanup stage. Signed-off-by: John Snow Message-id: 20180830015734.19765-6-jsnow@redhat.com Reviewed-by: Max Reitz Reviewed-by: Jeff Cody Signed-off-by: Max Reitz --- block/mirror.c | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index be5dc6b7b0..b8941db6c1 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -607,26 +607,22 @@ static void mirror_wait_for_all_io(MirrorBlockJob *s) } } =20 -typedef struct { - int ret; -} MirrorExitData; - -static void mirror_exit(Job *job, void *opaque) +static void mirror_exit(Job *job) { MirrorBlockJob *s =3D container_of(job, MirrorBlockJob, common.job); BlockJob *bjob =3D &s->common; - MirrorExitData *data =3D opaque; MirrorBDSOpaque *bs_opaque =3D s->mirror_top_bs->opaque; AioContext *replace_aio_context =3D NULL; BlockDriverState *src =3D s->mirror_top_bs->backing->bs; BlockDriverState *target_bs =3D blk_bs(s->target); BlockDriverState *mirror_top_bs =3D s->mirror_top_bs; Error *local_err =3D NULL; + int ret =3D job->ret; =20 bdrv_release_dirty_bitmap(src, s->dirty_bitmap); =20 - /* Make sure that the source BDS doesn't go away before we called - * job_completed(). */ + /* Make sure that the source BDS doesn't go away during bdrv_replace_n= ode, + * before we can call bdrv_drained_end */ bdrv_ref(src); bdrv_ref(mirror_top_bs); bdrv_ref(target_bs); @@ -652,7 +648,7 @@ static void mirror_exit(Job *job, void *opaque) bdrv_set_backing_hd(target_bs, backing, &local_err); if (local_err) { error_report_err(local_err); - data->ret =3D -EPERM; + ret =3D -EPERM; } } } @@ -662,7 +658,7 @@ static void mirror_exit(Job *job, void *opaque) aio_context_acquire(replace_aio_context); } =20 - if (s->should_complete && data->ret =3D=3D 0) { + if (s->should_complete && ret =3D=3D 0) { BlockDriverState *to_replace =3D src; if (s->to_replace) { to_replace =3D s->to_replace; @@ -679,7 +675,7 @@ static void mirror_exit(Job *job, void *opaque) bdrv_drained_end(target_bs); if (local_err) { error_report_err(local_err); - data->ret =3D -EPERM; + ret =3D -EPERM; } } if (s->to_replace) { @@ -710,12 +706,12 @@ static void mirror_exit(Job *job, void *opaque) blk_insert_bs(bjob->blk, mirror_top_bs, &error_abort); =20 bs_opaque->job =3D NULL; - job_completed(job, data->ret); =20 - g_free(data); bdrv_drained_end(src); bdrv_unref(mirror_top_bs); bdrv_unref(src); + + job->ret =3D ret; } =20 static void mirror_throttle(MirrorBlockJob *s) @@ -815,7 +811,6 @@ static int mirror_flush(MirrorBlockJob *s) static int coroutine_fn mirror_run(Job *job, Error **errp) { MirrorBlockJob *s =3D container_of(job, MirrorBlockJob, common.job); - MirrorExitData *data; BlockDriverState *bs =3D s->mirror_top_bs->backing->bs; BlockDriverState *target_bs =3D blk_bs(s->target); bool need_drain =3D true; @@ -1035,14 +1030,10 @@ immediate_exit: g_free(s->in_flight_bitmap); bdrv_dirty_iter_free(s->dbi); =20 - data =3D g_malloc(sizeof(*data)); - data->ret =3D ret; - if (need_drain) { bdrv_drained_begin(bs); } =20 - job_defer_to_main_loop(&s->common.job, mirror_exit, data); return ret; } =20 @@ -1141,6 +1132,7 @@ static const BlockJobDriver mirror_job_driver =3D { .user_resume =3D block_job_user_resume, .drain =3D block_job_drain, .run =3D mirror_run, + .exit =3D mirror_exit, .pause =3D mirror_pause, .complete =3D mirror_complete, }, @@ -1157,6 +1149,7 @@ static const BlockJobDriver commit_active_job_driver = =3D { .user_resume =3D block_job_user_resume, .drain =3D block_job_drain, .run =3D mirror_run, + .exit =3D mirror_exit, .pause =3D mirror_pause, .complete =3D mirror_complete, }, --=20 2.17.1 From nobody Tue Apr 8 16:37:08 2025 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; 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; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1535726522108731.7071661161576; Fri, 31 Aug 2018 07:42:02 -0700 (PDT) Received: from localhost ([::1]:54027 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkcV-000584-U5 for importer@patchew.org; Fri, 31 Aug 2018 10:41:55 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50527) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkQT-00014U-3D for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:29:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fvkML-0000r6-N9 for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:25:15 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:50292 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fvkMG-0000lY-6T; Fri, 31 Aug 2018 10:25:10 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D27768077102; Fri, 31 Aug 2018 14:25:07 +0000 (UTC) Received: from localhost (ovpn-117-235.ams2.redhat.com [10.36.117.235]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4D60D7D4C3; Fri, 31 Aug 2018 14:25:07 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Fri, 31 Aug 2018 16:24:43 +0200 Message-Id: <20180831142446.22264-9-mreitz@redhat.com> In-Reply-To: <20180831142446.22264-1-mreitz@redhat.com> References: <20180831142446.22264-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 31 Aug 2018 14:25:07 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 31 Aug 2018 14:25:07 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mreitz@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PULL 08/11] jobs: utilize job_exit shim 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: , Cc: Kevin Wolf , Peter Maydell , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RDMRC_1 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: John Snow Utilize the job_exit shim by not calling job_defer_to_main_loop, and where applicable, converting the deferred callback into the job_exit callback. This converts backup, stream, create, and the unit tests all at once. Most of these jobs do not see any changes to the order in which they clean up their resources, except the test-blockjob-txn test, which now puts down its bs before job_completed is called. This is safe for the same reason the reordering in the mirror job is safe, because job_completed no longer runs under two locks, making the unref safe even if it causes a flush. Signed-off-by: John Snow Reviewed-by: Max Reitz Message-id: 20180830015734.19765-7-jsnow@redhat.com Signed-off-by: Max Reitz --- block/backup.c | 16 ---------------- block/create.c | 14 +++----------- block/stream.c | 22 +++++++--------------- tests/test-bdrv-drain.c | 6 ------ tests/test-blockjob-txn.c | 11 ++--------- tests/test-blockjob.c | 10 ++++------ 6 files changed, 16 insertions(+), 63 deletions(-) diff --git a/block/backup.c b/block/backup.c index 1e965d54e5..a67b7fa96b 100644 --- a/block/backup.c +++ b/block/backup.c @@ -380,18 +380,6 @@ static BlockErrorAction backup_error_action(BackupBloc= kJob *job, } } =20 -typedef struct { - int ret; -} BackupCompleteData; - -static void backup_complete(Job *job, void *opaque) -{ - BackupCompleteData *data =3D opaque; - - job_completed(job, data->ret); - g_free(data); -} - static bool coroutine_fn yield_and_check(BackupBlockJob *job) { uint64_t delay_ns; @@ -483,7 +471,6 @@ static void backup_incremental_init_copy_bitmap(BackupB= lockJob *job) static int coroutine_fn backup_run(Job *opaque_job, Error **errp) { BackupBlockJob *job =3D container_of(opaque_job, BackupBlockJob, commo= n.job); - BackupCompleteData *data; BlockDriverState *bs =3D blk_bs(job->common.blk); int64_t offset, nb_clusters; int ret =3D 0; @@ -584,9 +571,6 @@ static int coroutine_fn backup_run(Job *opaque_job, Err= or **errp) qemu_co_rwlock_unlock(&job->flush_rwlock); hbitmap_free(job->copy_bitmap); =20 - data =3D g_malloc(sizeof(*data)); - data->ret =3D ret; - job_defer_to_main_loop(&job->common.job, backup_complete, data); return ret; } =20 diff --git a/block/create.c b/block/create.c index 26a385c6c7..95341219ef 100644 --- a/block/create.c +++ b/block/create.c @@ -34,28 +34,20 @@ typedef struct BlockdevCreateJob { Job common; BlockDriver *drv; BlockdevCreateOptions *opts; - int ret; } BlockdevCreateJob; =20 -static void blockdev_create_complete(Job *job, void *opaque) -{ - BlockdevCreateJob *s =3D container_of(job, BlockdevCreateJob, common); - - job_completed(job, s->ret); -} - static int coroutine_fn blockdev_create_run(Job *job, Error **errp) { BlockdevCreateJob *s =3D container_of(job, BlockdevCreateJob, common); + int ret; =20 job_progress_set_remaining(&s->common, 1); - s->ret =3D s->drv->bdrv_co_create(s->opts, errp); + ret =3D s->drv->bdrv_co_create(s->opts, errp); job_progress_update(&s->common, 1); =20 qapi_free_BlockdevCreateOptions(s->opts); - job_defer_to_main_loop(&s->common, blockdev_create_complete, NULL); =20 - return s->ret; + return ret; } =20 static const JobDriver blockdev_create_job_driver =3D { diff --git a/block/stream.c b/block/stream.c index 26a775386b..67e1e72e23 100644 --- a/block/stream.c +++ b/block/stream.c @@ -54,20 +54,16 @@ static int coroutine_fn stream_populate(BlockBackend *b= lk, return blk_co_preadv(blk, offset, qiov.size, &qiov, BDRV_REQ_COPY_ON_R= EAD); } =20 -typedef struct { - int ret; -} StreamCompleteData; - -static void stream_complete(Job *job, void *opaque) +static void stream_exit(Job *job) { StreamBlockJob *s =3D container_of(job, StreamBlockJob, common.job); BlockJob *bjob =3D &s->common; - StreamCompleteData *data =3D opaque; BlockDriverState *bs =3D blk_bs(bjob->blk); BlockDriverState *base =3D s->base; Error *local_err =3D NULL; + int ret =3D job->ret; =20 - if (!job_is_cancelled(job) && bs->backing && data->ret =3D=3D 0) { + if (!job_is_cancelled(job) && bs->backing && ret =3D=3D 0) { const char *base_id =3D NULL, *base_fmt =3D NULL; if (base) { base_id =3D s->backing_file_str; @@ -75,11 +71,11 @@ static void stream_complete(Job *job, void *opaque) base_fmt =3D base->drv->format_name; } } - data->ret =3D bdrv_change_backing_file(bs, base_id, base_fmt); + ret =3D bdrv_change_backing_file(bs, base_id, base_fmt); bdrv_set_backing_hd(bs, base, &local_err); if (local_err) { error_report_err(local_err); - data->ret =3D -EPERM; + ret =3D -EPERM; goto out; } } @@ -93,14 +89,12 @@ out: } =20 g_free(s->backing_file_str); - job_completed(job, data->ret); - g_free(data); + job->ret =3D ret; } =20 static int coroutine_fn stream_run(Job *job, Error **errp) { StreamBlockJob *s =3D container_of(job, StreamBlockJob, common.job); - StreamCompleteData *data; BlockBackend *blk =3D s->common.blk; BlockDriverState *bs =3D blk_bs(blk); BlockDriverState *base =3D s->base; @@ -203,9 +197,6 @@ static int coroutine_fn stream_run(Job *job, Error **er= rp) =20 out: /* Modify backing chain and close BDSes in main loop */ - data =3D g_malloc(sizeof(*data)); - data->ret =3D ret; - job_defer_to_main_loop(&s->common.job, stream_complete, data); return ret; } =20 @@ -215,6 +206,7 @@ static const BlockJobDriver stream_job_driver =3D { .job_type =3D JOB_TYPE_STREAM, .free =3D block_job_free, .run =3D stream_run, + .exit =3D stream_exit, .user_resume =3D block_job_user_resume, .drain =3D block_job_drain, }, diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index 6bda4451c1..89ac15e88a 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -752,11 +752,6 @@ typedef struct TestBlockJob { bool should_complete; } TestBlockJob; =20 -static void test_job_completed(Job *job, void *opaque) -{ - job_completed(job, 0); -} - static int coroutine_fn test_job_run(Job *job, Error **errp) { TestBlockJob *s =3D container_of(job, TestBlockJob, common.job); @@ -770,7 +765,6 @@ static int coroutine_fn test_job_run(Job *job, Error **= errp) job_pause_point(&s->common.job); } =20 - job_defer_to_main_loop(&s->common.job, test_job_completed, NULL); return 0; } =20 diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c index 82cedee78b..ef29f35e44 100644 --- a/tests/test-blockjob-txn.c +++ b/tests/test-blockjob-txn.c @@ -24,17 +24,11 @@ typedef struct { int *result; } TestBlockJob; =20 -static void test_block_job_complete(Job *job, void *opaque) +static void test_block_job_exit(Job *job) { BlockJob *bjob =3D container_of(job, BlockJob, job); BlockDriverState *bs =3D blk_bs(bjob->blk); - int rc =3D (intptr_t)opaque; =20 - if (job_is_cancelled(job)) { - rc =3D -ECANCELED; - } - - job_completed(job, rc); bdrv_unref(bs); } =20 @@ -54,8 +48,6 @@ static int coroutine_fn test_block_job_run(Job *job, Erro= r **errp) } } =20 - job_defer_to_main_loop(job, test_block_job_complete, - (void *)(intptr_t)s->rc); return s->rc; } =20 @@ -81,6 +73,7 @@ static const BlockJobDriver test_block_job_driver =3D { .user_resume =3D block_job_user_resume, .drain =3D block_job_drain, .run =3D test_block_job_run, + .exit =3D test_block_job_exit, }, }; =20 diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c index 408a226939..ad4a65bc78 100644 --- a/tests/test-blockjob.c +++ b/tests/test-blockjob.c @@ -163,11 +163,10 @@ typedef struct CancelJob { bool completed; } CancelJob; =20 -static void cancel_job_completed(Job *job, void *opaque) +static void cancel_job_exit(Job *job) { - CancelJob *s =3D opaque; + CancelJob *s =3D container_of(job, CancelJob, common.job); s->completed =3D true; - job_completed(job, 0); } =20 static void cancel_job_complete(Job *job, Error **errp) @@ -182,7 +181,7 @@ static int coroutine_fn cancel_job_run(Job *job, Error = **errp) =20 while (!s->should_complete) { if (job_is_cancelled(&s->common.job)) { - goto defer; + return 0; } =20 if (!job_is_ready(&s->common.job) && s->should_converge) { @@ -192,8 +191,6 @@ static int coroutine_fn cancel_job_run(Job *job, Error = **errp) job_sleep_ns(&s->common.job, 100000); } =20 - defer: - job_defer_to_main_loop(&s->common.job, cancel_job_completed, s); return 0; } =20 @@ -204,6 +201,7 @@ static const BlockJobDriver test_cancel_driver =3D { .user_resume =3D block_job_user_resume, .drain =3D block_job_drain, .run =3D cancel_job_run, + .exit =3D cancel_job_exit, .complete =3D cancel_job_complete, }, }; --=20 2.17.1 From nobody Tue Apr 8 16:37:08 2025 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; 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; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 153572591918953.46972060912924; Fri, 31 Aug 2018 07:31:59 -0700 (PDT) Received: from localhost ([::1]:53967 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkSr-0002f2-Ij for importer@patchew.org; Fri, 31 Aug 2018 10:31:57 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50671) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkQN-0001AA-7L for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:29:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fvkMN-0000s1-Cg for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:25:16 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:36552 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fvkMI-0000pI-CX; Fri, 31 Aug 2018 10:25:12 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EFCB987A70; Fri, 31 Aug 2018 14:25:09 +0000 (UTC) Received: from localhost (ovpn-117-235.ams2.redhat.com [10.36.117.235]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9D64B63A62; Fri, 31 Aug 2018 14:25:09 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Fri, 31 Aug 2018 16:24:44 +0200 Message-Id: <20180831142446.22264-10-mreitz@redhat.com> In-Reply-To: <20180831142446.22264-1-mreitz@redhat.com> References: <20180831142446.22264-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Fri, 31 Aug 2018 14:25:10 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Fri, 31 Aug 2018 14:25:10 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mreitz@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PULL 09/11] block/backup: make function variables consistently named 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: , Cc: Kevin Wolf , Peter Maydell , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RDMRC_1 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: John Snow Rename opaque_job to job to be consistent with other job implementations. Rename 'job', the BackupBlockJob object, to 's' to also be consistent. Suggested-by: Eric Blake Signed-off-by: John Snow Reviewed-by: Max Reitz Message-id: 20180830015734.19765-8-jsnow@redhat.com Signed-off-by: Max Reitz --- block/backup.c | 62 +++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/block/backup.c b/block/backup.c index a67b7fa96b..4d084f6ca6 100644 --- a/block/backup.c +++ b/block/backup.c @@ -468,59 +468,59 @@ static void backup_incremental_init_copy_bitmap(Backu= pBlockJob *job) bdrv_dirty_iter_free(dbi); } =20 -static int coroutine_fn backup_run(Job *opaque_job, Error **errp) +static int coroutine_fn backup_run(Job *job, Error **errp) { - BackupBlockJob *job =3D container_of(opaque_job, BackupBlockJob, commo= n.job); - BlockDriverState *bs =3D blk_bs(job->common.blk); + BackupBlockJob *s =3D container_of(job, BackupBlockJob, common.job); + BlockDriverState *bs =3D blk_bs(s->common.blk); int64_t offset, nb_clusters; int ret =3D 0; =20 - QLIST_INIT(&job->inflight_reqs); - qemu_co_rwlock_init(&job->flush_rwlock); + QLIST_INIT(&s->inflight_reqs); + qemu_co_rwlock_init(&s->flush_rwlock); =20 - nb_clusters =3D DIV_ROUND_UP(job->len, job->cluster_size); - job_progress_set_remaining(&job->common.job, job->len); + nb_clusters =3D DIV_ROUND_UP(s->len, s->cluster_size); + job_progress_set_remaining(job, s->len); =20 - job->copy_bitmap =3D hbitmap_alloc(nb_clusters, 0); - if (job->sync_mode =3D=3D MIRROR_SYNC_MODE_INCREMENTAL) { - backup_incremental_init_copy_bitmap(job); + s->copy_bitmap =3D hbitmap_alloc(nb_clusters, 0); + if (s->sync_mode =3D=3D MIRROR_SYNC_MODE_INCREMENTAL) { + backup_incremental_init_copy_bitmap(s); } else { - hbitmap_set(job->copy_bitmap, 0, nb_clusters); + hbitmap_set(s->copy_bitmap, 0, nb_clusters); } =20 =20 - job->before_write.notify =3D backup_before_write_notify; - bdrv_add_before_write_notifier(bs, &job->before_write); + s->before_write.notify =3D backup_before_write_notify; + bdrv_add_before_write_notifier(bs, &s->before_write); =20 - if (job->sync_mode =3D=3D MIRROR_SYNC_MODE_NONE) { + if (s->sync_mode =3D=3D MIRROR_SYNC_MODE_NONE) { /* All bits are set in copy_bitmap to allow any cluster to be copi= ed. * This does not actually require them to be copied. */ - while (!job_is_cancelled(&job->common.job)) { + while (!job_is_cancelled(job)) { /* Yield until the job is cancelled. We just let our before_w= rite * notify callback service CoW requests. */ - job_yield(&job->common.job); + job_yield(job); } - } else if (job->sync_mode =3D=3D MIRROR_SYNC_MODE_INCREMENTAL) { - ret =3D backup_run_incremental(job); + } else if (s->sync_mode =3D=3D MIRROR_SYNC_MODE_INCREMENTAL) { + ret =3D backup_run_incremental(s); } else { /* Both FULL and TOP SYNC_MODE's require copying.. */ - for (offset =3D 0; offset < job->len; - offset +=3D job->cluster_size) { + for (offset =3D 0; offset < s->len; + offset +=3D s->cluster_size) { bool error_is_read; int alloced =3D 0; =20 - if (yield_and_check(job)) { + if (yield_and_check(s)) { break; } =20 - if (job->sync_mode =3D=3D MIRROR_SYNC_MODE_TOP) { + if (s->sync_mode =3D=3D MIRROR_SYNC_MODE_TOP) { int i; int64_t n; =20 /* Check to see if these blocks are already in the * backing file. */ =20 - for (i =3D 0; i < job->cluster_size;) { + for (i =3D 0; i < s->cluster_size;) { /* bdrv_is_allocated() only returns true/false based * on the first set of sectors it comes across that * are are all in the same state. @@ -529,7 +529,7 @@ static int coroutine_fn backup_run(Job *opaque_job, Err= or **errp) * needed but at some point that is always the case. */ alloced =3D bdrv_is_allocated(bs, offset + i, - job->cluster_size - i, &n); + s->cluster_size - i, &n); i +=3D n; =20 if (alloced || n =3D=3D 0) { @@ -547,29 +547,29 @@ static int coroutine_fn backup_run(Job *opaque_job, E= rror **errp) if (alloced < 0) { ret =3D alloced; } else { - ret =3D backup_do_cow(job, offset, job->cluster_size, + ret =3D backup_do_cow(s, offset, s->cluster_size, &error_is_read, false); } if (ret < 0) { /* Depending on error action, fail now or retry cluster */ BlockErrorAction action =3D - backup_error_action(job, error_is_read, -ret); + backup_error_action(s, error_is_read, -ret); if (action =3D=3D BLOCK_ERROR_ACTION_REPORT) { break; } else { - offset -=3D job->cluster_size; + offset -=3D s->cluster_size; continue; } } } } =20 - notifier_with_return_remove(&job->before_write); + notifier_with_return_remove(&s->before_write); =20 /* wait until pending backup_do_cow() calls have completed */ - qemu_co_rwlock_wrlock(&job->flush_rwlock); - qemu_co_rwlock_unlock(&job->flush_rwlock); - hbitmap_free(job->copy_bitmap); + qemu_co_rwlock_wrlock(&s->flush_rwlock); + qemu_co_rwlock_unlock(&s->flush_rwlock); + hbitmap_free(s->copy_bitmap); =20 return ret; } --=20 2.17.1 From nobody Tue Apr 8 16:37:08 2025 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; 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; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1535726166740532.5833412265866; Fri, 31 Aug 2018 07:36:06 -0700 (PDT) Received: from localhost ([::1]:53992 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkWr-0007HA-IW for importer@patchew.org; Fri, 31 Aug 2018 10:36:05 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50527) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkQN-00014U-6H for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:29:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fvkMN-0000sE-FU for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:25:16 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:50298 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fvkML-0000qw-M7; Fri, 31 Aug 2018 10:25:13 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8690D8077102; Fri, 31 Aug 2018 14:25:12 +0000 (UTC) Received: from localhost (ovpn-117-235.ams2.redhat.com [10.36.117.235]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 182372166B41; Fri, 31 Aug 2018 14:25:11 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Fri, 31 Aug 2018 16:24:45 +0200 Message-Id: <20180831142446.22264-11-mreitz@redhat.com> In-Reply-To: <20180831142446.22264-1-mreitz@redhat.com> References: <20180831142446.22264-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 31 Aug 2018 14:25:12 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 31 Aug 2018 14:25:12 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mreitz@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PULL 10/11] jobs: remove ret argument to job_completed; privatize it 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: , Cc: Kevin Wolf , Peter Maydell , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RDMRC_1 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: John Snow Jobs are now expected to return their retcode on the stack, from the .run callback, so we can remove that argument. job_cancel does not need to set -ECANCELED because job_completed will update the return code itself if the job was canceled. While we're here, make job_completed static to job.c and remove it from job.h; move the documentation of return code to the .run() callback and to the job->ret property, accordingly. Signed-off-by: John Snow Message-id: 20180830015734.19765-9-jsnow@redhat.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- include/qemu/job.h | 28 +++++++++++++++------------- job.c | 11 ++++++----- trace-events | 2 +- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/include/qemu/job.h b/include/qemu/job.h index 1144d671a1..23395c17fa 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -124,7 +124,11 @@ typedef struct Job { /** Estimated progress_current value at the completion of the job */ int64_t progress_total; =20 - /** ret code passed to job_completed. */ + /** + * Return code from @run and/or @prepare callback(s). + * Not final until the job has reached the CONCLUDED status. + * 0 on success, -errno on failure. + */ int ret; =20 /** @@ -172,7 +176,16 @@ struct JobDriver { /** Enum describing the operation */ JobType job_type; =20 - /** Mandatory: Entrypoint for the Coroutine. */ + /** + * Mandatory: Entrypoint for the Coroutine. + * + * This callback will be invoked when moving from CREATED to RUNNING. + * + * If this callback returns nonzero, the job transaction it is part of= is + * aborted. If it returns zero, the job moves into the WAITING state. = If it + * is the last job to complete in its transaction, all jobs in the + * transaction move from WAITING to PENDING. + */ int coroutine_fn (*run)(Job *job, Error **errp); =20 /** @@ -496,17 +509,6 @@ void job_early_fail(Job *job); /** Moves the @job from RUNNING to READY */ void job_transition_to_ready(Job *job); =20 -/** - * @job: The job being completed. - * @ret: The status code. - * - * Marks @job as completed. If @ret is non-zero, the job transaction it is= part - * of is aborted. If @ret is zero, the job moves into the WAITING state. I= f it - * is the last job to complete in its transaction, all jobs in the transac= tion - * move from WAITING to PENDING. - */ -void job_completed(Job *job, int ret); - /** Asynchronously complete the specified @job. */ void job_complete(Job *job, Error **errp); =20 diff --git a/job.c b/job.c index 5df7791ad8..37d828f964 100644 --- a/job.c +++ b/job.c @@ -535,6 +535,8 @@ void job_drain(Job *job) } } =20 +static void job_completed(Job *job); + static void job_exit(void *opaque) { Job *job =3D (Job *)opaque; @@ -545,7 +547,7 @@ static void job_exit(void *opaque) job->driver->exit(job); aio_context_release(aio_context); } - job_completed(job, job->ret); + job_completed(job); } =20 /** @@ -883,13 +885,12 @@ static void job_completed_txn_success(Job *job) } } =20 -void job_completed(Job *job, int ret) +static void job_completed(Job *job) { assert(job && job->txn && !job_is_completed(job)); =20 - job->ret =3D ret; job_update_rc(job); - trace_job_completed(job, ret, job->ret); + trace_job_completed(job, job->ret); if (job->ret) { job_completed_txn_abort(job); } else { @@ -905,7 +906,7 @@ void job_cancel(Job *job, bool force) } job_cancel_async(job, force); if (!job_started(job)) { - job_completed(job, -ECANCELED); + job_completed(job); } else if (job->deferred_to_main_loop) { job_completed_txn_abort(job); } else { diff --git a/trace-events b/trace-events index c445f54773..4fd2cb4b97 100644 --- a/trace-events +++ b/trace-events @@ -107,7 +107,7 @@ gdbstub_err_checksum_incorrect(uint8_t expected, uint8_= t got) "got command packe # job.c job_state_transition(void *job, int ret, const char *legal, const char *s= 0, const char *s1) "job %p (ret: %d) attempting %s transition (%s-->%s)" job_apply_verb(void *job, const char *state, const char *verb, const char = *legal) "job %p in state %s; applying verb %s (%s)" -job_completed(void *job, int ret, int jret) "job %p ret %d corrected ret %= d" +job_completed(void *job, int ret) "job %p ret %d" =20 # job-qmp.c qmp_job_cancel(void *job) "job %p" --=20 2.17.1 From nobody Tue Apr 8 16:37:08 2025 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; 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; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1535726135937504.50091138916184; Fri, 31 Aug 2018 07:35:35 -0700 (PDT) Received: from localhost ([::1]:53983 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkWM-0006tS-NP for importer@patchew.org; Fri, 31 Aug 2018 10:35:34 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50667) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fvkQL-0001A5-Al for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:29:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fvkMO-0000si-Hb for qemu-devel@nongnu.org; Fri, 31 Aug 2018 10:25:17 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:47804 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fvkMN-0000rf-6g; Fri, 31 Aug 2018 10:25:15 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9D9064023132; Fri, 31 Aug 2018 14:25:14 +0000 (UTC) Received: from localhost (ovpn-117-235.ams2.redhat.com [10.36.117.235]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 54CCA2027EA0; Fri, 31 Aug 2018 14:25:14 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Fri, 31 Aug 2018 16:24:46 +0200 Message-Id: <20180831142446.22264-12-mreitz@redhat.com> In-Reply-To: <20180831142446.22264-1-mreitz@redhat.com> References: <20180831142446.22264-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Fri, 31 Aug 2018 14:25:14 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Fri, 31 Aug 2018 14:25:14 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mreitz@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PULL 11/11] jobs: remove job_defer_to_main_loop 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: , Cc: Kevin Wolf , Peter Maydell , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RDMRC_1 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: John Snow Now that the job infrastructure is handling the job_completed call for all implemented jobs, we can remove the interface that allowed jobs to schedule their own completion. Signed-off-by: John Snow Reviewed-by: Max Reitz Message-id: 20180830015734.19765-10-jsnow@redhat.com Signed-off-by: Max Reitz --- include/qemu/job.h | 17 ----------------- job.c | 40 ++-------------------------------------- 2 files changed, 2 insertions(+), 55 deletions(-) diff --git a/include/qemu/job.h b/include/qemu/job.h index 23395c17fa..e0cff702b7 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -568,23 +568,6 @@ void job_finalize(Job *job, Error **errp); */ void job_dismiss(Job **job, Error **errp); =20 -typedef void JobDeferToMainLoopFn(Job *job, void *opaque); - -/** - * @job: The job - * @fn: The function to run in the main loop - * @opaque: The opaque value that is passed to @fn - * - * This function must be called by the main job coroutine just before it - * returns. @fn is executed in the main loop with the job AioContext acqu= ired. - * - * Block jobs must call bdrv_unref(), bdrv_close(), and anything that uses - * bdrv_drain_all() in the main loop. - * - * The @job AioContext is held while @fn executes. - */ -void job_defer_to_main_loop(Job *job, JobDeferToMainLoopFn *fn, void *opaq= ue); - /** * Synchronously finishes the given @job. If @finish is given, it is calle= d to * trigger completion or cancellation of the job. diff --git a/job.c b/job.c index 37d828f964..b960e72710 100644 --- a/job.c +++ b/job.c @@ -561,12 +561,8 @@ static void coroutine_fn job_co_entry(void *opaque) assert(job && job->driver && job->driver->run); job_pause_point(job); job->ret =3D job->driver->run(job, &job->err); - if (!job->deferred_to_main_loop) { - job->deferred_to_main_loop =3D true; - aio_bh_schedule_oneshot(qemu_get_aio_context(), - job_exit, - job); - } + job->deferred_to_main_loop =3D true; + aio_bh_schedule_oneshot(qemu_get_aio_context(), job_exit, job); } =20 =20 @@ -969,38 +965,6 @@ void job_complete(Job *job, Error **errp) job->driver->complete(job, errp); } =20 - -typedef struct { - Job *job; - JobDeferToMainLoopFn *fn; - void *opaque; -} JobDeferToMainLoopData; - -static void job_defer_to_main_loop_bh(void *opaque) -{ - JobDeferToMainLoopData *data =3D opaque; - Job *job =3D data->job; - AioContext *aio_context =3D job->aio_context; - - aio_context_acquire(aio_context); - data->fn(data->job, data->opaque); - aio_context_release(aio_context); - - g_free(data); -} - -void job_defer_to_main_loop(Job *job, JobDeferToMainLoopFn *fn, void *opaq= ue) -{ - JobDeferToMainLoopData *data =3D g_malloc(sizeof(*data)); - data->job =3D job; - data->fn =3D fn; - data->opaque =3D opaque; - job->deferred_to_main_loop =3D true; - - aio_bh_schedule_oneshot(qemu_get_aio_context(), - job_defer_to_main_loop_bh, data); -} - int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error *= *errp) { Error *local_err =3D NULL; --=20 2.17.1