From patchwork Mon Dec 9 16:16:07 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1206398 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="WZAlwbKs"; dkim-atps=neutral Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 47WpKT6sPSz9sPK for ; Tue, 10 Dec 2019 03:18:53 +1100 (AEDT) Received: from localhost ([::1]:42184 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ieLkI-00007J-ML for incoming@patchwork.ozlabs.org; Mon, 09 Dec 2019 11:18:50 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:60656) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ieLjp-000076-4t for qemu-devel@nongnu.org; Mon, 09 Dec 2019 11:18:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ieLjn-0005mR-HD for qemu-devel@nongnu.org; Mon, 09 Dec 2019 11:18:20 -0500 Received: from us-smtp-1.mimecast.com ([207.211.31.81]:45429 helo=us-smtp-delivery-1.mimecast.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ieLjn-0005mF-DT for qemu-devel@nongnu.org; Mon, 09 Dec 2019 11:18:19 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1575908299; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=OYU2bFdV8HYp/Dkfz+XS+7aelHvN6sGn5+i1ejOekLo=; b=WZAlwbKsORpdENazYT8Ft1W5eGQ8k2ZEq2yWZ0dpdAILP+ahvHewSTCUM9e8Zd9+xfXf6I Xkpz54zO2fnKdXHagzwKEkOzfSNz7op4bFNTw3puDUa4axmqD8bshGyRBOP6yI4M/x0r6Q WSXfDvzPX42aASVxqlRiRmxCNf0cv64= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-315-dQ4xN_jfO_inN6XfBoJBqw-1; Mon, 09 Dec 2019 11:16:14 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id BA727802592; Mon, 9 Dec 2019 16:16:12 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-171.phx2.redhat.com [10.3.116.171]) by smtp.corp.redhat.com (Postfix) with ESMTP id 98FAC46; Mon, 9 Dec 2019 16:16:09 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Subject: [PATCH for 4.2-rc5 1/1] block/qcow2-bitmap: fix crash bug in qcow2_co_remove_persistent_dirty_bitmap Date: Mon, 9 Dec 2019 10:16:07 -0600 Message-Id: <20191209161607.20894-1-eblake@redhat.com> In-Reply-To: <20191209160015.20253-1-eblake@redhat.com> References: <20191209160015.20253-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-MC-Unique: dQ4xN_jfO_inN6XfBoJBqw-1 X-Mimecast-Spam-Score: 0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 207.211.31.81 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , peter.maydell@linaro.org, vsementsov@virtuozzo.com, "open list:qcow2" , Max Reitz , jsnow@redhat.com Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" From: Vladimir Sementsov-Ogievskiy Here is double bug: First, return error but not set errp. This may lead to: qmp block-dirty-bitmap-remove may report success when actually failed block-dirty-bitmap-remove used in a transaction will crash, as qmp_transaction will think that it returned success and will call block_dirty_bitmap_remove_commit which will crash, as state->bitmap is NULL Second (like in anecdote), this case is not an error at all. As it is documented in the comment above bdrv_co_remove_persistent_dirty_bitmap definition, absence of bitmap is not an error, and similar case handled at start of qcow2_co_remove_persistent_dirty_bitmap, it returns 0 when there is no bitmaps at all. But when there are some bitmaps, but not the requested one, it return error with errp unset. Fix that. Trigger: 1. create persistent bitmap A 2. shutdown vm (bitmap A is synced) 3. start vm 4. create persistent bitmap B 5. remove bitmap B - it fails (and crashes if in transaction) Potential workaround (rather invasive to ask clients to implement it): 1. create persistent bitmap A 2. shutdown vm 3. start vm 4. create persistent bitmap B 5. remember, that we want to remove bitmap B after vm shutdown ... some other operations ... 6. vm shutdown 7. start vm in stopped mode, and remove all bitmaps marked for removing 8. stop vm Fixes: b56a1e31759b750 Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id: <20191205193049.30666-1-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake Reviewed-by: John Snow [eblake: commit message tweaks] Signed-off-by: Eric Blake --- block/qcow2-bitmap.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index 8abaf632fc7d..c6c8ebbe89d4 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -1469,8 +1469,10 @@ int coroutine_fn qcow2_co_remove_persistent_dirty_bitmap(BlockDriverState *bs, Qcow2BitmapList *bm_list; if (s->nb_bitmaps == 0) { - /* Absence of the bitmap is not an error: see explanation above - * bdrv_remove_persistent_dirty_bitmap() definition. */ + /* + * Absence of the bitmap is not an error: see explanation above + * bdrv_co_remove_persistent_dirty_bitmap() definition. + */ return 0; } @@ -1485,7 +1487,8 @@ int coroutine_fn qcow2_co_remove_persistent_dirty_bitmap(BlockDriverState *bs, bm = find_bitmap_by_name(bm_list, name); if (bm == NULL) { - ret = -EINVAL; + /* Absence of the bitmap is not an error, see above. */ + ret = 0; goto out; }