From nobody Sun May 5 10:18:16 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 1517301624289941.1778747429477; Tue, 30 Jan 2018 00:40:24 -0800 (PST) Received: from localhost ([::1]:49862 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1egRSp-0001QT-6Z for importer@patchew.org; Tue, 30 Jan 2018 03:40:23 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36736) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1egRRS-0000Xu-CU for qemu-devel@nongnu.org; Tue, 30 Jan 2018 03:39:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1egRRP-00037W-41 for qemu-devel@nongnu.org; Tue, 30 Jan 2018 03:38:58 -0500 Received: from mail-pg0-x242.google.com ([2607:f8b0:400e:c05::242]:34147) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1egRRC-0002yF-6k; Tue, 30 Jan 2018 03:38:42 -0500 Received: by mail-pg0-x242.google.com with SMTP id r19so6700502pgn.1; Tue, 30 Jan 2018 00:38:41 -0800 (PST) Received: from localhost ([122.194.13.238]) by smtp.gmail.com with ESMTPSA id n80sm39214273pfj.79.2018.01.30.00.38.37 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 30 Jan 2018 00:38:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:date:to:cc:subject:message-id:mime-version:content-disposition :user-agent; bh=PBPa8DcSKMDVSYLJR39bd15+CA7ZzahN0lYD7Dz68HM=; b=CJPA97dmcmpDapOyWBDpNrFUPOl20uT5fJDiHxkUWO3+VdnygH76ZbpUw3GyO6HNjF XhgEOjmYewylQ32biN+A4gOUSoePhVrkG35k4hzceIVk1ddUNCJEB8KUCV5fO1hI3c0x f7mVN04bwFEtp4hKRIafGbWx+dJPEgEMiYSau/WVm2n+7edi2XtI/u3GGW+wCEWDAS8d rOs5FO23dWGT8kvnEkGS0kbRLfmM5H5HoMKXc/o+Ev9WUKMDCEuH944D/oDcdDaJj5tf YQS3eGAGS6PcFe3tfMWk1HCT0KEO+hvOVIuhGJf8n4a45GpDH4isN9jQWfFRQpm8+iFE AlFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:date:to:cc:subject:message-id:mime-version :content-disposition:user-agent; bh=PBPa8DcSKMDVSYLJR39bd15+CA7ZzahN0lYD7Dz68HM=; b=obuUBy68ZRmTSAIPRWTlrR1ipeMeTY+9Vp/aQdr09JRCYKbxB98tso/rIzRF9B+6WI mfgyRMQH1g82wPwgpk3ZJ3QbZSPo5dpnJQ0wZkOQSvdUZ8nLwaFsPJ54ZJ3hiu10Y7xm E09OqscOWldH1Tl/RePXBdLYjl3s507QqVUhT5SflsZM7Tr4wUg4so/WFMECwtKlyc4M Qo7Bfz4/Rjq9kM/t2XNj3kkwslKY8s1LPxHnOUEjsmLkpwJ61lljV8nwZHsM70Hcc71n ys5oetQLvQIcQj2UQ9dpdyUxpHD0C5eyKmV+6jslT5m1ebu247yQzkLN2nY8A7TDLxcy Qrug== X-Gm-Message-State: AKwxyteQHj2zlV1xhb9Eok2/sDP6vPFsnQc2mVbFQb8aUXxXhIiTl4nB 5RdbyKWoTAO+pqiUmqwGEp0= X-Google-Smtp-Source: AH8x225jSyLNW+UiyPc6QJbhR+IpB7vGPhM78U5mSvpbaXEv0Mi5m2EawAMN+HSH31k4VZnQ9yc+tw== X-Received: by 10.98.48.68 with SMTP id w65mr28874646pfw.21.1517301521068; Tue, 30 Jan 2018 00:38:41 -0800 (PST) From: Liang Li X-Google-Original-From: Liang Li Date: Tue, 30 Jan 2018 16:38:04 +0800 To: "Dr. David Alan Gilbert" , Markus Armbruster , Jeff Cody , Kevin Wolf , Eric Blake , Paolo Bonzini , qemu-block@nongnu.org, qemu-devel@nongnu.org, John Snow Message-ID: <20180130083532.GA31511@localhost> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.7.2 (2016-11-26) X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:400e:c05::242 Subject: [Qemu-devel] [PATCH] block/mirror: change the semantic of 'force' of block-job-cancel 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: Liang Li , Huaitong Han 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 Content-Type: text/plain; charset="utf-8" When doing drive mirror to a low speed shared storage, if there was heavy BLK IO write workload in VM after the 'ready' event, drive mirror block job can't be canceled immediately, it would keep running until the heavy BLK IO workload stopped in the VM. Because libvirt depends on block-job-cancel for block live migration, the current block-job-cancel has the semantic to make sure data is in sync after the 'ready' event. This semantic can't meet some requirement, for example, people may use drive mirror for realtime backup while need the ability of block live migration. If drive mirror can't not be cancelled immediately, it means block live migration need to wait, because libvirt make use drive mirror to implement block live migration and only one drive mirror block job is allowed at the same time for a give block dev. We need a new interface for 'force cancel', which could quit block job immediately if don't care about whether data is in sync or not. 'force' is not used by libvirt currently, to make things simple, change it's semantic slightly, hope it will not break some use case which need its original semantic. Cc: Paolo Bonzini Cc: Jeff Cody Cc: Kevin Wolf Cc: Max Reitz Cc: Eric Blake Cc: John Snow Reported-by: Huaitong Han Signed-off-by: Huaitong Han Signed-off-by: Liang Li --- block/mirror.c | 9 +++------ blockdev.c | 4 ++-- blockjob.c | 11 ++++++----- hmp-commands.hx | 3 ++- include/block/blockjob.h | 9 ++++++++- qapi/block-core.json | 6 ++++-- tests/test-blockjob-txn.c | 8 ++++---- 7 files changed, 29 insertions(+), 21 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index c9badc1..c22dff9 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -869,11 +869,8 @@ static void coroutine_fn mirror_run(void *opaque) =20 ret =3D 0; trace_mirror_before_sleep(s, cnt, s->synced, delay_ns); - if (!s->synced) { - block_job_sleep_ns(&s->common, delay_ns); - if (block_job_is_cancelled(&s->common)) { - break; - } + if (block_job_is_cancelled(&s->common) && s->common.force) { + break; } else if (!should_complete) { delay_ns =3D (s->in_flight =3D=3D 0 && cnt =3D=3D 0 ? SLICE_TI= ME : 0); block_job_sleep_ns(&s->common, delay_ns); @@ -887,7 +884,7 @@ immediate_exit: * or it was cancelled prematurely so that we do not guarantee that * the target is a copy of the source. */ - assert(ret < 0 || (!s->synced && block_job_is_cancelled(&s->common= ))); + assert(ret < 0 || block_job_is_cancelled(&s->common)); assert(need_drain); mirror_wait_for_all_io(s); } diff --git a/blockdev.c b/blockdev.c index 8e977ee..039f156 100644 --- a/blockdev.c +++ b/blockdev.c @@ -145,7 +145,7 @@ void blockdev_mark_auto_del(BlockBackend *blk) aio_context_acquire(aio_context); =20 if (bs->job) { - block_job_cancel(bs->job); + block_job_cancel(bs->job, false); } =20 aio_context_release(aio_context); @@ -3802,7 +3802,7 @@ void qmp_block_job_cancel(const char *device, } =20 trace_qmp_block_job_cancel(job); - block_job_cancel(job); + block_job_cancel(job, force); out: aio_context_release(aio_context); } diff --git a/blockjob.c b/blockjob.c index f5cea84..0aacb50 100644 --- a/blockjob.c +++ b/blockjob.c @@ -365,7 +365,7 @@ static void block_job_completed_single(BlockJob *job) block_job_unref(job); } =20 -static void block_job_cancel_async(BlockJob *job) +static void block_job_cancel_async(BlockJob *job, bool force) { if (job->iostatus !=3D BLOCK_DEVICE_IO_STATUS_OK) { block_job_iostatus_reset(job); @@ -376,6 +376,7 @@ static void block_job_cancel_async(BlockJob *job) job->pause_count--; } job->cancelled =3D true; + job->force =3D force; } =20 static int block_job_finish_sync(BlockJob *job, @@ -437,7 +438,7 @@ static void block_job_completed_txn_abort(BlockJob *job) * on the caller, so leave it. */ QLIST_FOREACH(other_job, &txn->jobs, txn_list) { if (other_job !=3D job) { - block_job_cancel_async(other_job); + block_job_cancel_async(other_job, true); } } while (!QLIST_EMPTY(&txn->jobs)) { @@ -542,10 +543,10 @@ void block_job_user_resume(BlockJob *job) } } =20 -void block_job_cancel(BlockJob *job) +void block_job_cancel(BlockJob *job, bool force) { if (block_job_started(job)) { - block_job_cancel_async(job); + block_job_cancel_async(job, force); block_job_enter(job); } else { block_job_completed(job, -ECANCELED); @@ -557,7 +558,7 @@ void block_job_cancel(BlockJob *job) * function pointer casts there. */ static void block_job_cancel_err(BlockJob *job, Error **errp) { - block_job_cancel(job); + block_job_cancel(job, false); } =20 int block_job_cancel_sync(BlockJob *job) diff --git a/hmp-commands.hx b/hmp-commands.hx index 15620c9..c8bb414 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -106,7 +106,8 @@ ETEXI .args_type =3D "force:-f,device:B", .params =3D "[-f] device", .help =3D "stop an active background block operation (use -f" - "\n\t\t\t if the operation is currently paused)", + "\n\t\t\t if you want to abort the operation immedia= tely" + "\n\t\t\t instead of keep running until data is in s= ync )", .cmd =3D hmp_block_job_cancel, }, =20 diff --git a/include/block/blockjob.h b/include/block/blockjob.h index 00403d9..4a96c42 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -63,6 +63,12 @@ typedef struct BlockJob { bool cancelled; =20 /** + * Set to true if the job should be abort immediately without waiting + * for data is in sync. + */ + bool force; + + /** * Counter for pause request. If non-zero, the block job is either pau= sed, * or if busy =3D=3D true will pause itself as soon as possible. */ @@ -218,10 +224,11 @@ void block_job_start(BlockJob *job); /** * block_job_cancel: * @job: The job to be canceled. + * @force: Quit a job without waiting data is in sync. * * Asynchronously cancel the specified job. */ -void block_job_cancel(BlockJob *job); +void block_job_cancel(BlockJob *job, bool force); =20 /** * block_job_complete: diff --git a/qapi/block-core.json b/qapi/block-core.json index 8225308..7c4dbfe 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2098,8 +2098,10 @@ # the name of the parameter), but since QEMU 2.7 it can have # other values. # -# @force: whether to allow cancellation of a paused job (default -# false). Since 1.3. +# @force: #optional whether to allow cancellation a job without waiting da= ta is +# in sync, please not that since 2.12 it's semantic is not exactly= the +# same as before, from 1.3 to 2.11 it means whether to allow cance= llation +# of a paused job (default false). Since 1.3. # # Returns: Nothing on success # If no background operation is active on this device, DeviceNotA= ctive diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c index 3591c96..53daadb 100644 --- a/tests/test-blockjob-txn.c +++ b/tests/test-blockjob-txn.c @@ -125,7 +125,7 @@ static void test_single_job(int expected) block_job_start(job); =20 if (expected =3D=3D -ECANCELED) { - block_job_cancel(job); + block_job_cancel(job, false); } =20 while (result =3D=3D -EINPROGRESS) { @@ -173,10 +173,10 @@ static void test_pair_jobs(int expected1, int expecte= d2) block_job_txn_unref(txn); =20 if (expected1 =3D=3D -ECANCELED) { - block_job_cancel(job1); + block_job_cancel(job1, false); } if (expected2 =3D=3D -ECANCELED) { - block_job_cancel(job2); + block_job_cancel(job2, false); } =20 while (result1 =3D=3D -EINPROGRESS || result2 =3D=3D -EINPROGRESS) { @@ -231,7 +231,7 @@ static void test_pair_jobs_fail_cancel_race(void) block_job_start(job1); block_job_start(job2); =20 - block_job_cancel(job1); + block_job_cancel(job1, false); =20 /* Now make job2 finish before the main loop kicks jobs. This simulat= es * the race between a pending kick and another job completing. --=20 1.8.3.1