From patchwork Tue Jan 30 08:38:04 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Liang Li X-Patchwork-Id: 10191429 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 54E8B60383 for ; Tue, 30 Jan 2018 08:40:05 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3EF092891F for ; Tue, 30 Jan 2018 08:40:05 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2FAE3289AA; Tue, 30 Jan 2018 08:40:05 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id E6CA02891F for ; Tue, 30 Jan 2018 08:40:03 +0000 (UTC) Received: from localhost ([::1]:49862 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1egRSU-0001QT-IS for patchwork-qemu-devel@patchwork.kernel.org; Tue, 30 Jan 2018 03:40:02 -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) 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) 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) 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+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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) ret = 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 = (s->in_flight == 0 && cnt == 0 ? SLICE_TIME : 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); if (bs->job) { - block_job_cancel(bs->job); + block_job_cancel(bs->job, false); } aio_context_release(aio_context); @@ -3802,7 +3802,7 @@ void qmp_block_job_cancel(const char *device, } 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); } -static void block_job_cancel_async(BlockJob *job) +static void block_job_cancel_async(BlockJob *job, bool force) { if (job->iostatus != 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 = true; + job->force = force; } 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 != 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) } } -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); } 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 = "force:-f,device:B", .params = "[-f] device", .help = "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 immediately" + "\n\t\t\t instead of keep running until data is in sync )", .cmd = hmp_block_job_cancel, }, 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; /** + * 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 paused, * or if busy == 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); /** * 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 data 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 cancellation +# of a paused job (default false). Since 1.3. # # Returns: Nothing on success # If no background operation is active on this device, DeviceNotActive 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); if (expected == -ECANCELED) { - block_job_cancel(job); + block_job_cancel(job, false); } while (result == -EINPROGRESS) { @@ -173,10 +173,10 @@ static void test_pair_jobs(int expected1, int expected2) block_job_txn_unref(txn); if (expected1 == -ECANCELED) { - block_job_cancel(job1); + block_job_cancel(job1, false); } if (expected2 == -ECANCELED) { - block_job_cancel(job2); + block_job_cancel(job2, false); } while (result1 == -EINPROGRESS || result2 == -EINPROGRESS) { @@ -231,7 +231,7 @@ static void test_pair_jobs_fail_cancel_race(void) block_job_start(job1); block_job_start(job2); - block_job_cancel(job1); + block_job_cancel(job1, false); /* Now make job2 finish before the main loop kicks jobs. This simulates * the race between a pending kick and another job completing.