From nobody Fri Oct 18 06:24:01 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) client-ip=8.43.85.245; envelope-from=devel-bounces@lists.libvirt.org; helo=lists.libvirt.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) smtp.mailfrom=devel-bounces@lists.libvirt.org; arc=fail (BodyHash is different from the expected one); dmarc=pass(p=reject dis=none) header.from=lists.libvirt.org Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [8.43.85.245]) by mx.zohomail.com with SMTPS id 171883012846176.08044815305573; Wed, 19 Jun 2024 13:48:48 -0700 (PDT) Received: by lists.libvirt.org (Postfix, from userid 996) id 27DDACDD; Wed, 19 Jun 2024 16:48:47 -0400 (EDT) Received: from lists.libvirt.org (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id ED9ECBCF; Wed, 19 Jun 2024 16:48:20 -0400 (EDT) Received: by lists.libvirt.org (Postfix, from userid 996) id BB212B6D; Wed, 19 Jun 2024 16:48:17 -0400 (EDT) Received: from EUR05-AM6-obe.outbound.protection.outlook.com (mail-am6eur05on2091.outbound.protection.outlook.com [40.107.22.91]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.libvirt.org (Postfix) with ESMTPS id A24289C2 for ; Wed, 19 Jun 2024 16:48:16 -0400 (EDT) Received: from AM9PR08MB6179.eurprd08.prod.outlook.com (2603:10a6:20b:2da::9) by VI1PR08MB10220.eurprd08.prod.outlook.com (2603:10a6:800:1bd::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7698.21; Wed, 19 Jun 2024 20:48:11 +0000 Received: from AM9PR08MB6179.eurprd08.prod.outlook.com ([fe80::1ef1:a219:c3f8:decd]) by AM9PR08MB6179.eurprd08.prod.outlook.com ([fe80::1ef1:a219:c3f8:decd%6]) with mapi id 15.20.7698.017; Wed, 19 Jun 2024 20:48:11 +0000 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on lists.libvirt.org X-Spam-Level: X-Spam-Status: No, score=0.2 required=5.0 tests=DKIM_INVALID,DKIM_SIGNED, FORGED_SPF_HELO,MAILING_LIST_MULTI,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.4 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=FS4hTLVgtODhWPQtBB2b1RnZlkbX/HvL7INCx9c5cBJ7hgu2ccJyuBDtWXrTk2JZcJe7Wb63YJYqpBHU5mxo3yvIRNLn/NZXMj/K2LDsDlB8u7MKE14rqDHS8jQpTHiBybhooAUvDxCdT+PDa+UmL9eRroRZwQZGZ68rOpMwTgH58j5tPPBvYdCaHYOIVTnJJf1f2fW72RpKTUJk2JP878ypXBicoAdW1+AQ1GePb/Sos90nOPIlMy43SpZM7JUpZ0VY+OzPiAIdh6DgmrJxYQ1nZ/evIJSEhdQ2JrB3qIQCaSf9amnTHbdpZbD5i2/ZORKu9MEtZpyjWchPD+niAw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; 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=iDHHUtGoiXizPVVqu0Syfk3TlJMszGCKaHl1+vJzAtk=; b=NBHBMvc8wyLshmxjwwU2tvyGAVzIH+cQAwoUNU4/5YQbcJIaqR32Lx8LzKoWGXepl9OHGKhQ0dE0bPD+zlSdr7eUZ0MztrIGjx/uwS/vDxLT1P0o0cpwSJVhWlVpR52rQJSKFf6Hiozdc2Iq4yYrmHCcK/rEySVdNpeJppPYPRzPv/ZREm7a035h52JXw3zx545MKzCNUnvwAQfpT2vhriHTrXKWWc8VRaEwDdsgqUbWM2eXDgkCxdRcpvvJUMSkgG7cCzPI0Xgoqnv0a8hGKlSkicE9P5on0Wfuis3YRfEQvGGM0KQnxYgrXhpIeZvt734l0k0c7O+W7HznsxZzEA== 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=iDHHUtGoiXizPVVqu0Syfk3TlJMszGCKaHl1+vJzAtk=; b=zPmvoPa2H/+lzWZWfakca9YjkN2rrRZTu38aCWzwuIdpnJfNfxgPvtxI95pr0VubBWeVMt5slGJWsfFL5h5TTMDSO4hWGCqbkFWKxqU+qFI61qcZDxsWof5CDa8vyPA4SrvpHqWPlCRmAZn/qLV1M5GplYu3lJ0N0YLzq8lOEjip03gq2bPmQuoFRX+zRRLRDjMc/OR9JJ8hZmvK/tSz7RHDVK7fbn2CN2WP+jCxTWWQPjK2gMW+VYXvCYL9cSzw+T+KZpG+W1/yXOGCAxaizGZClQ+gLY6akF6c9j1ZZiXJj5edIaciTbah//jjm17T4RO/TgzWyKmBBTNdZ+ARkg== Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=virtuozzo.com; Message-ID: <5ab0eb7d-a07f-40d0-bd56-facba3a8a1d9@virtuozzo.com> Date: Thu, 20 Jun 2024 04:48:05 +0800 User-Agent: Mozilla Thunderbird Subject: [PATCH] Add a check that vmdisk and disk variables are not NULL To: Peter Krempa , =?UTF-8?B?TWljaGFsIFByw612b3puw61r?= References: <20240429124326.118051-1-efim.shevrin@virtuozzo.com> <6bdac469-7de2-4c5c-8725-b1f25e169fa2@redhat.com> Content-Language: en-US In-Reply-To: X-ClientProxiedBy: FR0P281CA0236.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:b2::8) To AM9PR08MB6179.eurprd08.prod.outlook.com (2603:10a6:20b:2da::9) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: AM9PR08MB6179:EE_|VI1PR08MB10220:EE_ X-MS-Office365-Filtering-Correlation-Id: 804b7445-3d43-4eb6-ad62-08dc90a12210 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230037|1800799021|376011|366013; X-Microsoft-Antispam-Message-Info: =?utf-8?B?eXVKbDNYMWM4SnduVGJnRkxKaHZFaG5UbEEydmtxOXRObEM4MzRtUXZHdGdt?= =?utf-8?B?cmkwdjZhUy8yVlhOUS83UCtKT1BTL2NUTlMrVktsdzRBQkRtMHp2OFhOaUZp?= =?utf-8?B?YWl4UXBweHRSeDZZTXRNbXgrQjd1M01DLzFGaHRNMjdCam8zczFDbGl4b3BD?= =?utf-8?B?Vk9PK3NCSjNFOEhteFlnb3JKM0RHa2pac25WK1ZJZ3JONG85ZmVRMjd6UWNr?= =?utf-8?B?bExhbjNPOW1ONFFHVGtDUDV2M2xaKzlwQ0RmNnVSZFNXNXZzeGt2dE9kVkxB?= =?utf-8?B?Q1R0Zjh3MzdJeEUxdmNtRHdzczc2clZ6VWJEWGM2ME9PL21yR0wzUC94dEdE?= =?utf-8?B?Skd0VThYdURQb0pBUk1DOEFRRFQ0WWxjeVpGZkxlZnE1TkRZU0wxWjR1ZDBo?= =?utf-8?B?SWU5Wk02MzRrZ1VhNmJXeEczWFZ4WnB0YmNKSUF2QUk2b2x5eUppYUZLQ0kz?= =?utf-8?B?UEpiendFeXN3ZmVuMytna2tzVTdoaU1EU3RxbUNLYk1oQnhpcWx1ZHpoRTVE?= =?utf-8?B?NU4xaWMraHRWVW5oV1RmbXU5VnlEUWFGTE93cGlFZ0xuQS9Wc1k4WHJyQUgx?= =?utf-8?B?UHpqbVJST0RqbEtLcDMrMThhdVhkc3RhMzJKUEcrYWtGb3FDMnlaejRtdTZY?= =?utf-8?B?S0Z1QmVJNVdOejlPOGtKa3U0VUF2c3ZDNE9ZamJERThodlpSUktXSC9hVDdz?= =?utf-8?B?bnV2SnRaN2lnNFExZXViMjNpUERGRGI5UHhMYkMvb1RtQnlCZ1Q2RWY5amVR?= =?utf-8?B?WVE4UzNwcHB2bXRVL3paWE9xMWFteVBENllPOHpMNTZORnc3RTZ5SGhxNGxP?= =?utf-8?B?Tyt4UnZMdGlFOUFNSC9tTGlKbzVmOEtZcWJHbjYyTnlMelBDaW5Oak9JbHFO?= =?utf-8?B?WkZ3K1ZURTR4ekRMK2oyRWJpdk1VS0k1RlJvaXBJMkY3WVhKK0JHaXR0SlJZ?= =?utf-8?B?UTlMcFd3U3RSaVFvRUlySnpPSXhyNEF6YTRGTXRxQ242aFkzUzJnZVZjdnI5?= =?utf-8?B?dVpLakpJV05EUFpaYWNMcCs4M3ZZSXlraU9xQ296OWRWaFJGRFBGWHJWVXY4?= =?utf-8?B?WlZLNi9NU3lLSEhYSTdHRW5PZjRBSjl5RlM1RHJMZlRrTjZ2aWxrMjZ3UlVM?= =?utf-8?B?WFN5UmNVU1JQSzZmSlFqdkZxNzl4MkZ4cGNkQXhoaVcxcEkrbW0vcUZLQVdS?= =?utf-8?B?QjVSQ2ZUcXJaQzg3Nm16REgxWndpcEhoNWtBOXg4Skw1dVhjNDdiL09FVUJC?= =?utf-8?B?Y0N4U2hGcDI1YWsyOFRLakxRUm1uTnNNeHlBTWc0WjVBQzNWOExON2pFWVZv?= =?utf-8?B?K3V1L2t6Y2xMY2QwR0xVRWhicjFJUFhjbUYxcTBhTHcyL0VwWk55S0FkZG5Z?= =?utf-8?B?dVZRRGtqRzNiUGIvcnRVSWFFc0ZpZ0ZtdjhaZjhzc0c0VHNVOU1GZ1FISnV5?= =?utf-8?B?U1VTRDd3eU41bmNGYit0WVJLaVhqYzBLU2pzN2FGeFhmcG52MUltdmQ3dFpH?= =?utf-8?B?YW5MSkdydHBORHFzRlV5eVQvTTJYUU96ZHNLYUdtSEpvWVdaMUhjd2NuV2h2?= =?utf-8?B?RzF4Q3k4RzZlcU5jdkdaYWVJckhmNlNOZUlKdHIvMTFNU213Y2VsVjRiUjgx?= =?utf-8?B?c1c5a1RwdzJhTWk2TG1JWWlRcHhaRVlRQ0k0NU9JR2UyeXYwVDgxdSt2czBE?= =?utf-8?B?aVBDcUM1dk5LaEhKZExoVWFuaWt2U0M2eHlueDV3ZXdDZVRxbG9qSXgyYnJh?= =?utf-8?Q?PzPKBVYVT6DMyeFr4jFQm221AaqF3dGTSYK0iCX?= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:AM9PR08MB6179.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230037)(1800799021)(376011)(366013);DIR:OUT;SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dHpmOEVKamIvUDN6Ri9icS82cUZBZWx1MEVIYXR5ek55dFJ6VWdDZFVEZWZp?= =?utf-8?B?VTBCZ3pUdUZNbUNQSUVINExhL0lwNGkzRmxnWXBseEJqU2RCQ2hoQjhFaTVs?= =?utf-8?B?aDUyb1RpYjlsa1F0RVJKT0dpZVBjem4rR3JBb1E0RDQxcGZBVEdCMDE4MUI4?= =?utf-8?B?SFREU3pMeEN0clM1cEo3eGhFM0t0azBFeEpLcU9aMWxrTE9lYmtxWFNsb05F?= =?utf-8?B?Z1FLVWIvd0RuVkM0c0RPSnNZeEY0c0JDNE9lajRSNThFWnI3d3ZsZ3FIei85?= =?utf-8?B?Mkh5MG5ZdkJXYmVlYnZCMEU0eGVpclJxVHFoYTV2RDh6dTd1SVFZQjZWSkJ0?= =?utf-8?B?OWduZCtuZEJnU2FiQVNwWWU4WTVVM3JSTzR0di9LcHNtdkZ5ZEtSLysyNHJK?= =?utf-8?B?UGxqb1MrOGFWSVA4Zm5UYWVKK0FYdnBqY2YvSXNrZE5WQ1pZOHpQc3JuSHJx?= =?utf-8?B?cHRuVmxYaVdnaUZHNEx5MGZuREVnUS9TbnhTcW90QXJXaENqdzZsc0UyY1Fo?= =?utf-8?B?ZG5QbksxQzVVc3RxWVhvckU5WmhnWVBNSWU3K3VJTnNiTjJzelVCd3FPaFAr?= =?utf-8?B?NVNWWDdib09qU1JPWm1YQ2F6Zm11WUhmMW9Yc3NieDRjYXJXd2FxWEtrK08y?= =?utf-8?B?TEhEQnFJc1ZiSEVuQU12YWlRekMvOGI1bWw4K1RTb3pHejMzdDFidm83V2lN?= =?utf-8?B?NGlqZGgwSHpKY2RDVXJiZVdiNm5OUGlqNU9QeXQrSlVlREwybmpLQkFmcHdM?= =?utf-8?B?U1cydlRJcmVKeFp6Z3EzMzZZVHkxWWNxRmNWaXVqdHc4NThCN0VKZjhuQ1JP?= =?utf-8?B?L201Qi9PWUl0WXBteFIxM2cxOW1nTHBZYkk1TGVPQllxZHpaVDVKL0o0WDFJ?= =?utf-8?B?RFVsaTl6aDF2WGZ1QXpxZ1dqSi9wd1FOekRNZ3VRTDBkK1Z2ZkJuME9pS0FQ?= =?utf-8?B?Z1VOMUY1V0Y0blI3blJLWHRWZ0taMDczWnc1Y0V6LzNhNElQQUI3Q1JCQ1Az?= =?utf-8?B?cTZ3UFVaMkJ2M2xCai9zbzluQmZKWUszU2RwMHpFeEgwYXM0UnlaQ0dHRDlq?= =?utf-8?B?eU8wTk9FNTh2bEdHaS9FcWlNajZtOHJTejF1MjN1RWF4UUQxSjRIRmgrTGtV?= =?utf-8?B?cFVaYU9UYkVVYm5iUUo3NUVlYy8zREgvekxNM01nN0dwTElVYVpwR0czeVpm?= =?utf-8?B?cVd2dTFMcTFwc0tJYXVjdVJ2OU1UZW5hSFd6SXNHekNwZDJPY2VzdW9ralJz?= =?utf-8?B?eDlPay9nb0hwUlltS2t1UHJocFBHWUJnbW8yb0FrRzRsSTlhL3lNWDhEYjEy?= =?utf-8?B?S0ZLaFdJSmZmUFM0UDUwdzlwckowK1h0WDIwNzUwQnoyd3hxOGhDMUJHL1kz?= =?utf-8?B?TTIwVzhIRFVZSlVjelVvY3FQa0tHUFBiK1huajY1TFFnbkJhRDJ2Z24vVU9W?= =?utf-8?B?N0RDdEdIK1JGWW9qQWgwZ0VHU0J4YjVaaEdId0hXVmlaYVdoSVgwM0NUSE05?= =?utf-8?B?WGNVemFVMU81aWt3M0RFUHd0V1hnUFV1di9sZVZGRnpUbVVZa1FYUVJyNUNs?= =?utf-8?B?Zy9LMFAva1l3N1JyZGttU1BRY3drTGlwV3BpWUJMVVlCNk5HMXJ6TXlJalcr?= =?utf-8?B?R0NNWFJBRjBwTlovNlRtdDRyclMwUlN0UXVGbHpqZVFLWUFvLzFoTWU0MXpE?= =?utf-8?B?RmpweStVaS9qWGV2VlJBQkZ4ZGgvSnpjZDVSSXhvRW5vMHJwUVROazRMUzR2?= =?utf-8?B?bldyS3FBUm5xNkhoVEVEcURuOWdkNVhIdGFEUVd4Z0tCcXVRUmNyaEplV1RZ?= =?utf-8?B?YUNjUHV0Q0hFeEVBRUJNL1BnK0pQRWRBUGhPNFpEZjVZWFBlbXQyeW9adncy?= =?utf-8?B?TzNnVXJWSGx4K1h0MWI4VDRkbysyaVlzQlMrQUxyb2VNVkM1R1I4TkcxbG9q?= =?utf-8?B?TEpFR2lac0RLRWJoMmc2QWx5VEJTNWJtUy90SEpUWG0rSkN0OEV1MCtOekUx?= =?utf-8?B?enNPRktEUmJpZEFqSWxrWUJlelY2TU05S0pMUldiMHRtbi84N3gzMkxTVzdO?= =?utf-8?B?Vzc5d3lzaS96VlJIaU5HdVpYYzlCR1MzSEt5dCtDN2tDcklCeTBzSHBRbkdH?= =?utf-8?B?T29jWkxkQlplL2RGU1pXajYzUHh6RUVoaEoxYU5QTkd5Vk5nOXVjWHVvMVE2?= =?utf-8?B?b2c9PQ==?= X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-Network-Message-Id: 804b7445-3d43-4eb6-ad62-08dc90a12210 X-MS-Exchange-CrossTenant-AuthSource: AM9PR08MB6179.eurprd08.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Jun 2024 20:48:11.7248 (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: BLR7d/asuweKz7RVaMpd0wxL8z24hr/3q7xaKKBfiZRHbWWLfDe0Afe8wRGpPYwsHJq8tn7QTIRcEMWhdPBx/NKEoQr1GJHE+FEumToqiAs= X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR08MB10220 Content-Transfer-Encoding: quoted-printable Message-ID-Hash: BEBAPQRCNHCZQRY4MGZO7GFSVNIZQKH5 X-Message-ID-Hash: BEBAPQRCNHCZQRY4MGZO7GFSVNIZQKH5 X-MailFrom: efim.shevrin@virtuozzo.com X-Mailman-Rule-Hits: nonmember-moderation X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; header-match-devel.lists.libvirt.org-0 CC: "devel@lists.libvirt.org" , "den@openvz.org" X-Mailman-Version: 3.2.2 Precedence: list List-Id: Development discussions about the libvirt library & tools Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: From: Fima Shevrin via Devel Reply-To: Fima Shevrin X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZM-MESSAGEID: 1718830128761100001 Content-Type: text/plain; charset="utf-8"; format="flowed" Hello, Thank you for your comments and explanations about disk states and=20 snapshots in general. I agree that it is quite dangerous to always delete a snapshot in case=20 one of the variables is NULL. I modified the patch based on Michal's comment that=20 qemuSnapshotDeleteValidate should return an error if vmdisk or disk is NULL. I am attaching the patch and updated commit message. I apologize for some delay in our discussion. Fima Subject: [PATCH] Add a check that vmdisk and disk variables are not NULL Before deleting a snapshot, there is a validation process that involves checking that the disk from the VM config (vmdisk) and the disk from the snapshot config (disk) point to the same storage location. The vmdisk and disk variables are obtained by searching by disk name in the vm domain and snapshot domain, respectively. It is possible that the disks have been removed from the configs, then the result of a search for vmdisk and disk may be NULL. Thus, if a vmdisk or disk is not found in the appropriate domain, we interpret this as an error and return -1. Signed-off-by: Fima Shevrin --- =C2=A0src/qemu/qemu_snapshot.c | 2 +- =C2=A01 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 09ec959f10..7d2cde8e8f 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -3806,7 +3806,7 @@ qemuSnapshotDeleteValidate(virDomainObj *vm, =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 v= mdisk =3D qemuDomainDiskByName(vm->def, snapDisk->name); =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 d= isk =3D qemuDomainDiskByName(snapdef->parent.dom,=20 snapDisk->name); -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (!vi= rStorageSourceIsSameLocation(vmdisk->src, disk->src)) { +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (vmd= isk =3D=3D NULL || disk =3D=3D NULL ||=20 !virStorageSourceIsSameLocation(vmdisk->src, disk->src)) { =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0 virReportError(VIR_ERR_OPERATION_UNSUPPORTED, =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 _("disk image '%1$s' for internal=20 snapshot '%2$s' is not the same as disk image currently used by VM"), =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 snapDisk->name, snap->def->name); --=20 2.39.3 On 20.05.2024 11:17 PM, Peter Krempa wrote: > [You don't often get email from pkrempa@redhat.com. Learn why this is imp= ortant at https://aka.ms/LearnAboutSenderIdentification ] > > On Mon, May 20, 2024 at 14:48:47 +0000, Efim Shevrin via Devel wrote: >> Hello, >> >>> If vmdisk is NULL, shouldn't this function (qemuSnapshotDeleteValidate(= )) return an error? >> I think this qemuSnapshotDeleteValidate should not return an error. >> >> It seems to me that when vmdisk is NULL, this does not invalidate >> the snapshot itself, but indicates that the config has changed since >> the snapshot was done. And if the VM config has changed, this adds evid= ence that the snapshot should be deleted, >> because the snapshot does not reflect the real vm config. >> >> Since we do not have an analogue of the --force option for deleting a sn= apshot, in the case when qemuSnapshotDeleteValidate returns >> an error when vmdisk is NULL, we will never delete a snapshot which has = invalid disk. > Snapshot deletion does have something that can be considered force and > that is the '--metadata' option that removes just the snapshot > definition (metadata) and doesn't touch the disk images. > >>> Similarly, disk can be NULL too >> Thank you for the comment regarding the disk variable. I`ve reworked pat= ch. >> >> When creating a snapshot of a VM with multiple hard disks, >> the snapshot takes into account the presence of all disks >> in the system. If, over time, one of the disks is deleted, >> the snapshot will continue to store knowledge of the deleted disk. >> This results in the fact that at the moment of deleting the snapshot, >> at the validation stage, a disk from the snapshot will be searched which >> is not in the VM configuration. As a result, vmdisk variable will >> be equal to NULL. Dereferencing a null pointer at the time of calling >> virStorageSourceIsSameLocation(vmdisk->src, disk->src) >> will result in SIGSEGV. > Crashing is obviously not okay ... > >> Also, the disk variable can also be equal to NULL and this >> requires to check that disk !=3D NULL before calling the >> virStorageSourceIsSameLocation function to avoid SIGSEGV. > .. but going ahead with the snapshot deletion isn't always okay either. > > The disk isn't referenced by the VM so the disk state can't be merged, > while the state would be merged for any other disk. > > When reverting back to a previous snapshot, which is still referencing > the older state of the disk which was removed from the VM, the VM would > see that the image state of disks that were present at deletion would > contain the merged state, but only a partial state for the disk which > was later removed. >