From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from AM0PR83CU005.outbound.protection.outlook.com (mail-westeuropeazon11020132.outbound.protection.outlook.com [52.101.69.132]) by lore.virtuozzo.com (Postfix) with ESMTPS id CF3588015C for ; Wed, 16 Sep 2026 16:28:18 +0000 (UTC) ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=cxAJpsEClX4sbsvYKaGFQ0po6yuY2rj6O3gb+dGM6Bo6HAG3eYSx56UtURoVUEjGKSC+cMWtlMp1WIxdmbikYuJ3A0bJ/mKCkMR/kNRpmvMzSKl6yGhTlXrGqll9aEv/YEkAvjyU+aiD3mEMnHLFWqf2Hwf34//IBzo/IJY9MlsWEWI8AazCY5VKOCGCfFN3SGdsI7285f9TU8bmLoyxZSkVTNXghlzojYhja+BMrN1w38Incx8cRcmpSnA5p7WuApBpVq+MpXSoDdZ+ShHBPx5wou4rnaibgciTwFKPYFH133DmxJ5Wae6/MmWoj9Klx9livZY1C9NDW1WdL4A5LA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=3TCY9+1cFCW8bNWdiJnPMmg38f15dHQY1iOYEi5lMCk=; b=s6iwn9PiZfObb5Ok+HVQcc5dqkU2BjTGYXjpRL8MHjqUFqlFBBXnmhf9T/puULlRDDPX4++e1wEaqABTOYsg3R6bl+zPd/o7MiWUVjeDRIGXXf4YiMcbEGBUgWGeHSMry08x+5jkb7yIUF14hesdnhIeQIrfvKmN3Vnx66uqnYS5xqKixsluHedvoRG99xFG9iRy0ThQkQOMRUF7i9dn03BTrfxJ06C7m9pzTq5kCUWWGzEkNPEYAjgAkH+7d0wA3QFmmrZxs9/my7hkV8Rlxgq4Svg+whIa7AOiFMVQKethJ2fFyQlqE1iAZij3lf2NU0WfnUWdwjWswXHEQC8EEw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=virtuozzo.com; dmarc=pass action=none header.from=virtuozzo.com; dkim=pass header.d=virtuozzo.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=virtuozzo.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=3TCY9+1cFCW8bNWdiJnPMmg38f15dHQY1iOYEi5lMCk=; b=PkVlMIcqFfSaFeYiH9tepwCRhJhzVB+KwMXQoWeW5eZvEGSR9WJjCez1cNtwnAjnewYql541vFBOBEn8vSlaErggDKFam/KGa6ffz2h3wxe4+dymL5r64iEJT7ttROqLvqSu9i8664+Rbhn7N+7k/GVjosImuxYkB77iXaEWj8vsK3hor8yBrjCmP1NqtMtip+iFtgW0GCYyvN9Lpr2Imo5XJsot5PzFAbB3EAAHW04Vo/3efoPCO2oq9hiZNqiXoxJ9EfvI1LpocC02cNqdpXp/VhAUgaJPPdqTnDLJP95JCEZEqzsZI0s6czXd1vYAkdnvR0+EelHH+CPChXvCpg== Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=virtuozzo.com; Received: from AM9PR08MB5892.eurprd08.prod.outlook.com (2603:10a6:20b:2dd::16) by PAWPR08MB9760.eurprd08.prod.outlook.com (2603:10a6:102:2ea::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.21.428.9; Wed, 16 Sep 2026 16:28:15 +0000 Received: from AM9PR08MB5892.eurprd08.prod.outlook.com ([fe80::94bb:633f:1f55:4bbd]) by AM9PR08MB5892.eurprd08.prod.outlook.com ([fe80::94bb:633f:1f55:4bbd%6]) with mapi id 15.21.0451.004; Wed, 16 Sep 2026 16:28:15 +0000 Message-ID: <04bc7b35-899f-4e9b-8df5-2bc64051ddb2@virtuozzo.com> Date: Wed, 16 Sep 2026 18:28:14 +0200 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH hci-8.0 5/5] qcow2: repair a dirty image when it becomes writable #VSTOR-140073 To: Andrey Drobyshev , "Denis V. Lunev" , svt-core@virtuozzo.com References: <20260916152437.1787889-1-den@openvz.org> <20260916152437.1787889-6-den@openvz.org> Content-Language: en-US From: "Denis V. Lunev" In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: VI5PEPF0000098A.AUTP296.PROD.OUTLOOK.COM (2603:10a6:808:1::80d) To AM9PR08MB5892.eurprd08.prod.outlook.com (2603:10a6:20b:2dd::16) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: AM9PR08MB5892:EE_|PAWPR08MB9760:EE_ X-MS-Office365-Filtering-Correlation-Id: 6b87853a-91d4-49c2-7ed7-08df140f827e List-Id: svt-core@virtuozzo.com X-LD-Processed: 0bc7f26d-0264-416e-a6fc-8352af79c58f,ExtAddr X-MS-Exchange-Transport-Rules-Loop: 1 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|23010399003|366016|10070799003|376014|1800799024|56012099006|5023799004|3023799007|4143699003|10067099003|6133799003|5113699003|22082099003|18002099003; X-Microsoft-Antispam-Message-Info: XMKEQM2QVflk2Jv9jVeHad8VwrJaAiQt08skIOI8b65+wR/RdPbgQqT8V/EW9bwuV34kqe1SqIu5LnuD8w9ZV12jt0iH9mJ7fNaJqCmpkdH9209jVtTNWyRH02LD0oDmkx3VDRumeSweMSSjhBHjJ/dWvucVrEePzPWgFEbrByqsE8OlUzmRU7aojDay3kDAO2j9zVx6sVcMf3x2C3JuCm62WrS7sgk5Av1GY+DokG/ywG41jodY28DvCo88lKmeHEV5oOAVPrEydwd7MneLNTbymjfdMEuQlNK/CZFaKukKw+evH8Wi+mbT8mHPBBEjsVsHHOY1nQB9VSHJXSjhHwn0XfMvuM2+jBoRBGL8DfplLsGnFdEMGCMteyjfSjSrJWBjbhM1JiAKF0NJOheozWo8O9wfkiBbkSQo5xmMQJxHxagcwqkw6u2xMF+LsCoXOdTALikylOiafACKMudH3NCMdnB1SZQRgT4kBL5YPjMOg/GtwN5JmmqnhQwtd4yr6yHihtfkRuuNjdjdzjgf3W9sAk6LEDgCK1qQ6pbfg9cOJFWto6xdUfHLXENCkr11ADQcvjGF+21moMQk0vb63oTUorIwuUouzJgskvrJzwLEKBu06179+C0OppteXynym6YljE1+xlAh0S4x2ibQnS+OY6SoE4rHPb5rDsfSS6k= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:AM9PR08MB5892.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230040)(23010399003)(366016)(10070799003)(376014)(1800799024)(56012099006)(5023799004)(3023799007)(4143699003)(10067099003)(6133799003)(5113699003)(22082099003)(18002099003);DIR:OUT;SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 2 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?N3J4amp6TFVuK0t3bFpBSWZNZXFVRE4yV0x4OHViamJoUFU2SHRXaHF6Sjgr?= =?utf-8?B?R0VkQlFaR3NvODhYazVTWUVXdWxmRmJtQ3ZXWlJSQXZqcUJRL2dlNFg0RTg3?= =?utf-8?B?Vm5VM1A4VTlSOXJnUE5LU050Z2cwdHpZWTRTVFowRzByeTZSYnViS0t0QmtT?= =?utf-8?B?Z3AwODhkK0d4WXVXQVZXTituNlo0Z3paYlRycWF0ZWxDUHliV1lJWlU2dFBh?= =?utf-8?B?aU5jNWpsOTZLVDhLL2JpcEtDenNPREc0dGZMRWNNV09YWStHTUZkL2FVb1NX?= =?utf-8?B?VTQ1ZERwZ2VuVzkzc3I5YmNJUlBkZFlheFlRdm5QbHNzWi93Y0J2Qy9HZDNi?= =?utf-8?B?VVBlcFZ1TzBBNGNlZmdUaHgrMjJUSUYxYmNxRnJYOVNQcStra3lpbkovTTFC?= =?utf-8?B?R0d5d1FwN1I4K1BuZ2VtSjJxMmlycWdYSDdPeE1haW5MZXJTM0Jtc3hxVUVH?= =?utf-8?B?S1BhRUZXLzQyWk9qR1pyRVRZOWwxOHdqWW5RN1FiT0tqTnVLYkN5Mk9NVHpy?= =?utf-8?B?aEJQbDVnamVBRGFnNzBPb0NpTTVnM0ZMcFl3TWJPWlZ6NmI2di8zNnlCMWZF?= =?utf-8?B?QTBBRUpFOHVTUjVrbW1XdTlvbFpNSzlHa0x0bUhhbzZQSUlZTVhod3RxRWx1?= =?utf-8?B?a000V2Facjh3a3BHZ3RMUlk5UlIwOXdFWFgwSEpyUWQwRlBCaWkwWFg1NzFi?= =?utf-8?B?cjVaK0FwRHVIcmhqWThKSURpMUNSMUl0ZXlXRGRIT1BmSENsTWFlWVdNYnIx?= =?utf-8?B?bEtxK1hJbDgyVmgyTXM5VmJjWG13SWhCRnlHWDhuSVNTVXMyRzFpSHp1d1JY?= =?utf-8?B?S2xydmtRYmxaRlVmYVJLazlSbVR5QmZ4YXJOMVNUTDg4aEZtNmxOR3hrUS9B?= =?utf-8?B?QytjNGd0U2dxTjVZbWs2WW9rdmNWRGdNLzFpVVhrSGlqTUEyNlRrWEJ0eGhq?= =?utf-8?B?QiswdGlQeHE5TGhCdDQ3WEw0SEFCVjV1WDVhUUtQSWwrd2R2K2NLREdxZEp6?= =?utf-8?B?VmtTOFhtUkJlbC9yQTk5aFFOV2haem5LNk0rU0d1SnBPWWZ1YXJBY0ZaWUQ1?= =?utf-8?B?UnZpRCtEVm1rcURkWU8rYkxENWF1azZVRUNiSHF5Z2hncWRiNVBZcmZva3VR?= =?utf-8?B?V3hFVi9IZWY0WTJkM2M0U2kvMHVmMVFodEtsejNwcGd5ZFVnZ1luMHVSTW91?= =?utf-8?B?UFF1V1FpMlhWbWVISEZJd1Yva3dJckFLY0I5TzNaWklFcG96MFFGSURzMDdI?= =?utf-8?B?TDVXY3lpMHFCbVZiRHpadEo3Z0FKT2hnak9RZ1ZlZVl0Nm9DNzhhQjhUbjhn?= =?utf-8?B?TE9WYXBaV3Qyczk3NE5LOExITFR1UVM5OEQ2UlEvL1MxSEdhekF6TW1YSFZE?= =?utf-8?B?dlhndUNDWnFSZklYK0Q1TmNnaDQ0NTMwdkN2N1h6aENqWlcwenMrUDJ2Qm5p?= =?utf-8?B?ZmMvZ2RCbU1lblVLRHpYTzZQQ0lsV2lNWXMrUnRXN0FvV1FlQlJ4bERmbXdr?= =?utf-8?B?UExrZVFkbFhFTWRWZVRHeVJRWmYydnZ3ejVHU2ZTcWoxbjBIeXRmSzNjazQv?= =?utf-8?B?UGhVK3lKZm1TL0ltMXJTb0lmVmIzNlFEZkp6TE1lcEJ4eXlFZVRuRElNUHlQ?= =?utf-8?B?ODErMWZnNU1uSTZUMXN2OVZ6ZnhScFNuQjdHekhZVFVFSitGaXVONThvcThO?= =?utf-8?B?Ri9MK2w1enFSYk1HM2pHa1hYT3hja3A1NmphL3h2dHdRMWdoZXNSUml2YjM0?= =?utf-8?B?VjM5bUxnYVU5bkxHTDlDdXFXL3JESjQvc2g5MHA3dW5SaGZmdis1Zy9YMGZt?= =?utf-8?B?MG1MODRJNmNkRWErbHh2MmZ3OXJRaS9HR2xJNWlwY3RmRDd3UDFVS1R1R2tV?= =?utf-8?B?d3R6YlJDbFdYVnd6OUVVbjFJQWpVZnFZNWtOTVpMZTZFdW1hTjJyMnNCdXk0?= =?utf-8?B?U3BIZ0tuVU9MYnEvQSthTC9TMWtlcmdtU0Y5Sks0cVRTMTVVM1UwT1U2NWJG?= =?utf-8?B?RHNtbXNCelp3RFBXbzkySEw0QmFPQi9rcW12MnBNajVUK3RzM0tadlBVNWJu?= =?utf-8?B?dUlwZEVPVHRmdEkvdmdiSW1mcURqMHI1dHBndDcwUlBMU1FjZ1V6NDRSMDBL?= =?utf-8?B?Q1UzdkUvOUZGd1hoak14TnNVMWExdkx5MTd5OUFFM0s3UGhNK0pQa2o2cDBo?= =?utf-8?B?MDJMWGdUQkptUUNPeUtrZEc5VWpUaUczQzVxRFNkalc4dEREYk9haDQ4VTRx?= =?utf-8?B?QTZkOTZsOXFIL1NLU3B3Y3pndW9GT3BGQnZWdmpNZktKUHNQb2JIWmlxN2Vo?= =?utf-8?B?aHB5M000NktEdFhzQXgxVlkvUDBjUVgzV05QakcwdDZTZmRERkpGbDZxWm03?= =?utf-8?Q?K8DAq2l4yViGerv2LWqnUmT7ONJy96jf8uG9f7qExS6eS?= X-MS-Exchange-AntiSpam-MessageData-1: 8slGmec6ty8sHg== X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-Network-Message-Id: 6b87853a-91d4-49c2-7ed7-08df140f827e X-MS-Exchange-CrossTenant-AuthSource: AM9PR08MB5892.eurprd08.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Sep 2026 16:28:15.7415 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: DF+NSBOLQZl/tWChLu7v5EiqPkPMNOwNsHFxvNfFIim8KfFt/mElgAjc1eOd30YklQnCg7nsWXwEjr0cBruzpw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAWPR08MB9760 pls answer to mainstream first properly, I'll send there and here Den On 9/16/26 18:26, Andrey Drobyshev wrote: > Other commits appear to be fine, but this commit has several issues: > > - Major: a failed repair makes a readable image unreadable. > state->bs->drv = NULL at block/qcow2.c:2251 blocks reads as well as > writes. The base of a running chain, or the active layer in the > stream_start() case, answers every guest read with ENOMEDIUM from then > on, for an image whose L1/L2 metadata is intact and was serving reads a > moment earlier. Test 039's own expected output shows it: read failed: No > medium found. Only allocation has to be stopped. A per-node gate in > BDRVQcow2State that fails pwritev/pwrite_zeroes/pdiscard/truncate with > -EIO (and skips qcow2_mark_clean() in qcow2_inactivate() so the dirty > bit survives close) keeps the guest running and the job simply fails. > The current approach also makes query-named-block-nodes/query-block fail > wholesale while the node exists, and in the commit case the node cannot > be deleted until the VM is restarted. > > - Major: use-after-free through the cache-clean coroutine. With > bs->drv == NULL, bdrv_close() (block.c:5259) never calls qcow2_close(), > so cache_clean_timer_del_and_wait() never runs, and bdrv_close() then > frees bs->opaque. The coroutine at block/qcow2.c:895 is still sleeping > on s->cache_clean_timer_wake (600 s default on Linux) and wakes on freed > memory. Pre-existing for qcow2_signal_corruption(fatal), but this patch > adds a second path and is the right place to stop the coroutine before > dropping the driver. The same skipped close also leaks the L1 table, > refcount table, both caches, crypto and bitmap state. > > - Major: the check runs under bdrv_drain_all_begin(). > bdrv_reopen_queue() drains every node (block.c:4593), and bdrv_check() > polls from the main loop. A full refcount scan plus rebuild of a large > image stalls all guest I/O on every disk and the monitor for its whole > duration, as a side effect of > block-commit/block-stream/change-backing-file. The QAPI text discloses > it, but the cover letter should quantify it and argue why this beats > failing the command. > > - Medium (downstream only): the new condition ignores s->keep_dirty, > unlike qcow2_do_open() at block/qcow2.c:2025. A node opened with > __vz_keep-dirty=on, then reopened read-write, gets checked and its dirty > bit cleared, breaking that option's contract. Add !s->keep_dirty in the > hci branch. > > - Low: after the failure the node keeps BDRV_O_RDWR, BLK_PERM_WRITE on > bs->file and the write lock, so the qemu-img check -r the hint > recommends is impossible until the node is gone, which for a commit base > means VM shutdown. Say so in the hint or the docs. > > - Low: test 040 asserts exact error strings owned by other code ("Node > 'base' is busy: node is used as backing hd of 'mid'", "Block device base > is ejected"), which makes it brittle. > > On 9/16/26 6:24 PM, Denis V. Lunev wrote: >> From: Denis V. Lunev >> >> A dirty image must be repaired before anything allocates a cluster in >> it. qcow2_do_open() does that, but only for a node that is writable >> from the start. A node opened read-only skips it, and nothing revisits >> the question once that node becomes writable, which block-commit does >> routinely: commit_active_start() and commit_start() reopen the base >> read-write for the duration of the job. >> >> With lazy refcounts the on-disk refcount block then still accounts for >> the metadata clusters only, so the allocator restarts at the front of >> the image and hands out clusters that L2 entries point at. Two guest >> offsets end up sharing one host cluster. Nothing fails, the corrupt bit >> stays clear, and a clean close clears the dirty bit, so no later open >> repairs the image either. The bit also stays set for the whole writable >> session, so a node which is merely writable says nothing. >> >> Refusing the reopen instead is simpler and keeps it atomic, but it >> leaves nowhere to go: the base belongs to a chain the VM has open, so >> the qemu-img check -r such an error would ask for cannot take the write >> lock it needs. The repair does the trick in most cases anyway. >> >> Do the repair in qcow2_reopen_commit_post(), the earliest point where >> the node is writable. An inactive node is skipped: bdrv_activate() calls >> qcow2_do_open() again through qcow2_co_invalidate_cache(). >> >> commit_post cannot reject the reopen, so a failed repair takes the >> driver away from the node instead, which is what stops writes from >> aliasing live clusters. qcow2_signal_corruption() does that as well, >> but it also sends BLOCK_IMAGE_CORRUPTED and sets the corrupt bit, >> which qcow2_do_open() honours by refusing every later read-write open. >> The image is dirty and unrepaired, not corrupt, and qemu-img check -r >> still fixes it, so neither belongs here. Return the error and skip >> the bitmaps. >> >> Signed-off-by: Denis V. Lunev >> Reviewed-by: Andrey Drobyshev >> CC: Kevin Wolf >> CC: Hanna Reitz >> CC: Eric Blake >> CC: Markus Armbruster >> CC: Andrey Drobyshev >> Cc: qemu-stable@nongnu.org >> --- >> block/qcow2.c | 21 +++++++ >> qapi/block-core.json | 19 ++++++ >> tests/qemu-iotests/039 | 60 ++++++++++++++++++ >> tests/qemu-iotests/039.out | 36 +++++++++++ >> tests/qemu-iotests/040 | 123 +++++++++++++++++++++++++++++++++++++ >> tests/qemu-iotests/040.out | 4 +- >> 6 files changed, 261 insertions(+), 2 deletions(-) >> >> diff --git a/block/qcow2.c b/block/qcow2.c >> index ddad39a3084..be346f0339b 100644 >> --- a/block/qcow2.c >> +++ b/block/qcow2.c >> @@ -2235,8 +2235,29 @@ static void qcow2_reopen_commit(BDRVReopenState *state) >> >> static int qcow2_reopen_commit_post(BDRVReopenState *state, Error **errp) >> { >> + ERRP_GUARD(); >> + BDRVQcow2State *s = state->bs->opaque; >> + >> GRAPH_RDLOCK_GUARD_MAINLOOP(); >> >> + if (!bdrv_reopen_was_writable(state) && bdrv_is_writable(state->bs) && >> + (s->incompatible_features & QCOW2_INCOMPAT_DIRTY)) { >> + BdrvCheckResult result = {0}; >> + int ret; >> + >> + ret = bdrv_check(state->bs, &result, BDRV_FIX_ERRORS | BDRV_FIX_LEAKS); >> + if (ret < 0 || result.check_errors || !state->bs->drv) { >> + ret = ret < 0 ? ret : -EIO; >> + /* No write may reach an image whose refcounts are unaccounted */ >> + state->bs->drv = NULL; >> + error_setg_errno(errp, -ret, "Could not repair dirty image '%s'", >> + bdrv_get_device_or_node_name(state->bs)); >> + error_append_hint(errp, "The image is left dirty and this node " >> + "holds it open until the node is removed\n"); >> + return ret; >> + } >> + } >> + >> if (state->flags & BDRV_O_RDWR) { >> Error *local_err = NULL; >> >> diff --git a/qapi/block-core.json b/qapi/block-core.json >> index c036a3a8858..6fb8c31210a 100644 >> --- a/qapi/block-core.json >> +++ b/qapi/block-core.json >> @@ -1843,6 +1843,9 @@ >> # r/w -> r/o, if needed). The new backing file string is written into >> # the image file metadata, and the QEMU internal strings are updated. >> # >> +# A dirty qcow2 image is repaired during that reopen, which blocks >> +# other requests and can fail the command. >> +# >> # @image-node-name: The name of the block driver state node of the >> # image to modify. The "device" argument is used to verify >> # "image-node-name" is in the chain described by "device". >> @@ -1882,6 +1885,9 @@ >> # size to match the size of the smaller top, you can safely truncate >> # it yourself once the commit operation successfully completes. >> # >> +# The base is opened read-write. A dirty qcow2 base is repaired >> +# first, which blocks other requests and can fail the command. >> +# >> # @job-id: identifier for the newly-created block job. If omitted, >> # the device name will be used. (Since 2.7) >> # >> @@ -2919,6 +2925,9 @@ >> # On successful completion the image file is updated to drop the >> # backing file and the `BLOCK_JOB_COMPLETED` event is emitted. >> # >> +# The top image is opened read-write. A dirty qcow2 image is repaired >> +# first, which blocks other requests and can fail the command. >> +# >> # In case @device is a filter node, `block-stream` modifies the first >> # non-filter overlay node below it to point to the new backing node >> # instead of modifying @device itself. >> @@ -5070,6 +5079,16 @@ >> # transaction, so if one of them fails then the whole transaction is >> # cancelled. >> # >> +# An error is also returned when a device cannot be used once it has >> +# been reopened. Such a reopen is not undone, so an error does not >> +# always mean that nothing has changed. A node the driver gave up on >> +# serves nothing at all: it keeps its image open, makes >> +# `query-named-block-nodes` fail for as long as it is in the graph, >> +# and `blockdev-del` removes it only once nothing refers to it. >> +# >> +# Reopening a dirty qcow2 image read-write repairs it first, which >> +# blocks other requests and can fail the command. >> +# >> # The command receives a list of block devices to reopen. For each >> # one of them, the top-level @node-name option (from `BlockdevOptions`) >> # must be specified and is used to select the block device to be >> diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039 >> index 3d0c073d65d..3f37c36ca86 100755 >> --- a/tests/qemu-iotests/039 >> +++ b/tests/qemu-iotests/039 >> @@ -33,6 +33,7 @@ status=1 # failure is the default! >> _cleanup() >> { >> _cleanup_test_img >> + rm -f "$TEST_DIR/blkdebug.conf" >> } >> trap "_cleanup; exit \$status" 0 1 2 3 15 >> >> @@ -176,6 +177,65 @@ $QEMU_IO -c "write 0 512" "$TEST_IMG" | _filter_qemu_io >> # The dirty bit must not be set >> _qcow2_dump_header | grep incompatible_features >> >> +echo >> +echo "== Reopening a dirty image read/write should repair it ==" >> + >> +_make_test_img -o "compat=1.1,lazy_refcounts=on" $size >> + >> +_NO_VALGRIND \ >> +$QEMU_IO -c "write -P 0x5a 0 512" \ >> + -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \ >> + | _filter_qemu_io >> + >> +# The dirty bit must be set >> +_qcow2_dump_header | grep incompatible_features >> + >> +# Without the repair this write would alias the cluster at offset 0 >> +$QEMU_IO -r -c "reopen -w" \ >> + -c "write -P 0xb1 1M 512" \ >> + -c "read -P 0x5a 0 512" "$TEST_IMG" | _filter_qemu_io >> + >> +_check_test_img >> + >> +echo >> +echo "== A read/write reopen must not check the image ==" >> + >> +_make_test_img -o "compat=1.1,lazy_refcounts=on" $size >> + >> +_NO_VALGRIND \ >> +$QEMU_IO -c "write -P 0x5a 0 512" \ >> + -c "reopen -o l2-cache-size=1M" \ >> + -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \ >> + | _filter_qemu_io >> + >> +# The dirty bit must still be set, it belongs to the running session >> +_qcow2_dump_header | grep incompatible_features >> + >> +echo >> +echo "== A failed repair must fail the reopen ==" >> + >> +_make_test_img -o "compat=1.1,lazy_refcounts=on" $size >> + >> +_NO_VALGRIND \ >> +$QEMU_IO -c "write -P 0x5a 0 512" \ >> + -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \ >> + | _filter_qemu_io >> + >> +cat > "$TEST_DIR/blkdebug.conf" <> +[inject-error] >> +event = "none" >> +iotype = "write" >> +errno = "5" >> +EOF >> + >> +# The repair cannot write, so the reopen itself must report the failure >> +$QEMU_IO -r -c "reopen -w" -c "read -P 0x5a 0 512" \ >> + "blkdebug:$TEST_DIR/blkdebug.conf:$TEST_IMG" 2>&1 \ >> + | _filter_testdir | _filter_qemu_io | _filter_generated_node_ids >> + >> +# The corrupt bit needs a write of its own, so the image is only left dirty >> +_qcow2_dump_header | grep incompatible_features >> + >> echo >> echo "== Creating an image file with lazy_refcounts=off ==" >> >> diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out >> index ce8ee577216..cc6ca3ab95b 100644 >> --- a/tests/qemu-iotests/039.out >> +++ b/tests/qemu-iotests/039.out >> @@ -79,6 +79,42 @@ wrote 512/512 bytes at offset 0 >> 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> incompatible_features [] >> >> +== Reopening a dirty image read/write should repair it == >> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 >> +wrote 512/512 bytes at offset 0 >> +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> +./common.rc: Killed ( VALGRIND_QEMU="${VALGRIND_QEMU_IO}" _qemu_proc_exec "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" ) >> +incompatible_features [0] >> +ERROR cluster 5 refcount=0 reference=1 >> +Rebuilding refcount structure >> +Repairing cluster 1 refcount=1 reference=0 >> +Repairing cluster 2 refcount=1 reference=0 >> +wrote 512/512 bytes at offset 1048576 >> +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> +read 512/512 bytes at offset 0 >> +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> +No errors were found on the image. >> + >> +== A read/write reopen must not check the image == >> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 >> +wrote 512/512 bytes at offset 0 >> +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> +./common.rc: Killed ( VALGRIND_QEMU="${VALGRIND_QEMU_IO}" _qemu_proc_exec "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" ) >> +incompatible_features [0] >> + >> +== A failed repair must fail the reopen == >> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 >> +wrote 512/512 bytes at offset 0 >> +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> +./common.rc: Killed ( VALGRIND_QEMU="${VALGRIND_QEMU_IO}" _qemu_proc_exec "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" ) >> +ERROR cluster 5 refcount=0 reference=1 >> +Rebuilding refcount structure >> +qemu-io: ERROR writing refblock: Input/output error >> +qemu-io: Could not repair dirty image 'NODE_NAME': Input/output error >> +The image is left dirty and this node holds it open until the node is removed >> +read failed: No medium found >> +incompatible_features [0] >> + >> == Creating an image file with lazy_refcounts=off == >> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 >> wrote 512/512 bytes at offset 0 >> diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 >> index 5c18e413ec9..b25c76183a1 100755 >> --- a/tests/qemu-iotests/040 >> +++ b/tests/qemu-iotests/040 >> @@ -951,6 +951,129 @@ class TestCommitWithOverriddenBacking(iotests.QMPTestCase): >> self.vm.qmp('block-job-complete', device='commit') >> self.vm.event_wait('BLOCK_JOB_COMPLETED') >> >> +QCOW2_INCOMPAT_FEATURES_OFFSET = 72 >> +QCOW2_INCOMPAT_DIRTY = 1 << 0 >> + >> +image_size = 4 * 1024 * 1024 >> +dirty_base = os.path.join(iotests.test_dir, 'dirty-base.img') >> +mid = os.path.join(iotests.test_dir, 'dirty-mid.img') >> +top = os.path.join(iotests.test_dir, 'dirty-top.img') >> + >> + >> +class TestCommitDirtyBase(iotests.QMPTestCase): >> + def setUp(self) -> None: >> + if iotests.imgfmt != 'qcow2': >> + self.case_skip('the dirty bit is a qcow2 feature') >> + iotests.qemu_img_create('-f', iotests.imgfmt, '-o', >> + 'compat=1.1,lazy_refcounts=on', dirty_base, >> + str(image_size)) >> + # Killing the process leaves the refcounts of the written cluster stale >> + iotests.qemu_io_popen('-t', 'writethrough', >> + '-c', 'write -P 0x5a 0 512', >> + '-c', 'sigraise 9', dirty_base).communicate() >> + iotests.qemu_img_create('-f', iotests.imgfmt, '-b', dirty_base, >> + '-F', iotests.imgfmt, mid) >> + iotests.qemu_img_create('-f', iotests.imgfmt, '-b', mid, >> + '-F', iotests.imgfmt, top) >> + # The commit has to allocate for this, which is where the stale >> + # refcounts hand out the cluster holding the data written above >> + qemu_io('-c', 'write -P 0xb1 1M 512', mid) >> + >> + self.vm = iotests.VM() >> + self.vm.launch() >> + self.vm.cmd('blockdev-add', driver='file', filename=dirty_base, >> + node_name='base-file') >> + >> + self.assertTrue(self.is_dirty()) >> + >> + def tearDown(self) -> None: >> + if self.vm.is_running(): >> + self.vm.shutdown() >> + for image in (dirty_base, mid, top): >> + os.remove(image) >> + >> + def add_chain(self, base_file: str) -> None: >> + self.vm.cmd('blockdev-add', driver=iotests.imgfmt, file=base_file, >> + node_name='base', read_only=True) >> + self.vm.cmd('blockdev-add', driver='file', filename=mid, >> + node_name='mid-file') >> + self.vm.cmd('blockdev-add', driver=iotests.imgfmt, file='mid-file', >> + node_name='mid', backing='base') >> + self.vm.cmd('blockdev-add', driver='file', filename=top, >> + node_name='top-file') >> + self.vm.cmd('blockdev-add', driver=iotests.imgfmt, file='top-file', >> + node_name='top', backing='mid') >> + >> + def check_base(self) -> None: >> + result = iotests.qemu_img_check(dirty_base) >> + self.assertEqual(result['check-errors'], 0) >> + self.assertEqual(result.get('corruptions', 0), 0) >> + # Without the repair the commit would have aliased this cluster >> + qemu_io('-c', 'read -P 0x5a 0 512', '-c', 'read -P 0xb1 1M 512', >> + dirty_base) >> + >> + def is_dirty(self) -> bool: >> + with open(dirty_base, 'rb') as img: >> + img.seek(QCOW2_INCOMPAT_FEATURES_OFFSET) >> + features = struct.unpack('>Q', img.read(8))[0] >> + return bool(features & QCOW2_INCOMPAT_DIRTY) >> + >> + def test_commit_repairs_base(self) -> None: >> + self.add_chain('base-file') >> + >> + self.vm.cmd('block-commit', job_id='job0', device='top', >> + top_node='mid', base_node='base') >> + self.wait_until_completed(drive='job0') >> + >> + self.vm.shutdown() >> + self.assertFalse(self.is_dirty()) >> + self.check_base() >> + >> + def test_active_commit_repairs_base(self) -> None: >> + self.add_chain('base-file') >> + >> + # Without top-node the whole chain commits, through >> + # commit_active_start() rather than commit_start() >> + self.vm.cmd('block-commit', job_id='job0', device='top', >> + base_node='base') >> + self.complete_and_wait(drive='job0') >> + >> + self.vm.shutdown() >> + self.assertFalse(self.is_dirty()) >> + self.check_base() >> + >> + def test_failed_repair_fails_the_commit(self) -> None: >> + self.vm.cmd('blockdev-add', driver='blkdebug', image='base-file', >> + node_name='base-blkdebug', >> + inject_error=[{'event': 'none', 'iotype': 'write', >> + 'errno': 5}]) >> + self.add_chain('base-blkdebug') >> + >> + result = self.vm.qmp('block-commit', job_id='job0', device='top', >> + top_node='mid', base_node='base') >> + self.assert_qmp(result, 'error/class', 'GenericError') >> + self.assertIn("Could not repair dirty image 'base'", >> + result['error']['desc']) >> + >> + # The base is left in the graph, and nothing can be queried while >> + # it is there >> + result = self.vm.qmp('query-named-block-nodes', flat=True) >> + self.assert_qmp(result, 'error/desc', 'Block device base is ejected') >> + >> + # It only goes away once nothing refers to it >> + result = self.vm.qmp('blockdev-del', node_name='base') >> + self.assert_qmp(result, 'error/desc', >> + "Node 'base' is busy: node is used as backing hd of " >> + "'mid'") >> + >> + # Marking the image corrupt needs a write of its own, which fails too >> + self.assertTrue(self.is_dirty()) >> + >> + # Nothing can use the base any more, and that is what is reported >> + result = self.vm.qmp('block-commit', job_id='job1', device='top', >> + top_node='mid', base_node='base') >> + self.assert_qmp(result, 'error/desc', 'Device has no medium') >> + >> if __name__ == '__main__': >> iotests.main(supported_fmts=['qcow2', 'qed'], >> supported_protocols=['file']) >> diff --git a/tests/qemu-iotests/040.out b/tests/qemu-iotests/040.out >> index 1bb1dc5f0ea..f3cbf73a01d 100644 >> --- a/tests/qemu-iotests/040.out >> +++ b/tests/qemu-iotests/040.out >> @@ -1,5 +1,5 @@ >> -................................................................. >> +.................................................................... >> ---------------------------------------------------------------------- >> -Ran 65 tests >> +Ran 68 tests >> >> OK