From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3E1E5C64E7B for ; Wed, 2 Dec 2020 16:29:27 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 916F2208FE for ; Wed, 2 Dec 2020 16:29:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 916F2208FE Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:55674 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kkV0M-0004nD-O3 for qemu-devel@archiver.kernel.org; Wed, 02 Dec 2020 11:29:22 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:38362) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kkUzK-0004Hf-PB for qemu-devel@nongnu.org; Wed, 02 Dec 2020 11:28:18 -0500 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:20211) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1kkUzJ-0004lr-4W for qemu-devel@nongnu.org; Wed, 02 Dec 2020 11:28:18 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1606926494; 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: in-reply-to:in-reply-to:references:references; bh=1fdCv6syevGoD4Um3JJ/nqHPFRgb+c1he76UUkdyuVQ=; b=YZkU2Y2eqbapxFZsppuQ56SHnKcfJiFsf5WffudWP5bIYNLf0shc5Hu4H/g31GvwKCkz8/ AEwtB4vMpKe/19+UBOuBc5WvRLWWTNWn9xylioDcK79d6XiBeQpJECqK+JTCmvuSbf7Tiw vinxAqu87d92H1/WRVwbMWRq7G+fxgc= 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-15-p-D6BSRaOzut_S6jzNlUJQ-1; Wed, 02 Dec 2020 11:28:12 -0500 X-MC-Unique: p-D6BSRaOzut_S6jzNlUJQ-1 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 4EEA9100C667; Wed, 2 Dec 2020 16:28:11 +0000 (UTC) Received: from merkur.fritz.box (ovpn-113-199.ams2.redhat.com [10.36.113.199]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 22E4360855; Wed, 2 Dec 2020 16:28:09 +0000 (UTC) Date: Wed, 2 Dec 2020 17:28:08 +0100 From: Kevin Wolf To: Alberto Garcia Subject: Re: Plans to bring QMP 'x-blockdev-reopen' out of experimental? Message-ID: <20201202162808.GG16765@merkur.fritz.box> References: <20201006091001.GA64583@paraplu> <20201020082333.GB4452@merkur.fritz.box> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=kwolf@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Received-SPF: pass client-ip=216.205.24.124; envelope-from=kwolf@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -35 X-Spam_score: -3.6 X-Spam_bar: --- X-Spam_report: (-3.6 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-1.495, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action 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: mreitz@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org, Kashyap Chamarthy Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Am 02.12.2020 um 17:12 hat Alberto Garcia geschrieben: > On Tue 20 Oct 2020 10:23:33 AM CEST, Kevin Wolf wrote: > >> I forgot to add, we still don't support changing bs->file with this > >> command, so I guess that would be one blocker? > >> > >> There's no other way of inserting filter nodes, or is there? > > > > Not that I'm aware of. > > > > So yes, changing bs->file is the one thing I had in mind for > > implementing before we mark it stable. > > > > I'm not entirely sure if we should make some restrictions or allow > > arbitrary changes. If it's only about filters, we could check that the > > node returned by bdrv_skip_filters() stays the same. This would be > > almost certainly safe (if the chain is not frozen, of course). > > > > If people want to dynamically insert non-filters (maybe quorum?), it > > might be more restrictive than necessary, though. > > > > Other cases like inserting a qcow2 file in the chain where the old > > child becomes the backing file of the newly inserted node should in > > theory already be covered by blockdev-snapshot. > > Hi, > > I have been working a bit on this Oh, nice! And you might have mentioned this just in time to stop me from duplicating your work. There is a strong desire from libvirt to have a stable blockdev-reopen in QEMU 6.0. > and I have doubts about the following situation: let's say we have a > normal qcow2 image with two BDS for format (node-name "hd0") and > protocol ("hd0-file"): > > hd0 -> hd0-file > > { "execute": "blockdev-add", "arguments": > {'driver': 'file', 'node-name': 'hd0-file', 'filename': 'hd0.qcow2 }} > { "execute": "blockdev-add", "arguments": > {'driver': 'qcow2', 'node-name': 'hd0', 'file': 'hd0-file'}} > > Now we want to use x-blockdev-reopen to insert a throttle filter > between them, so the result would be: > > hd0 -> throttle -> hd0-file > > First we add the filter: > > { "execute": "object-add", "arguments": > { 'qom-type': 'throttle-group', 'id': 'group0', > 'props': { 'limits': { 'iops-total': 1000 } } } } > { "execute": "blockdev-add", "arguments": > { 'driver': 'throttle', 'node-name': 'throttle0', > 'throttle-group': 'group0', 'file': "hd0-file" } } > > And then we insert it: > > { "execute": "x-blockdev-reopen", "arguments": > {'driver': 'qcow2', 'node-name': 'hd0', 'file': 'throttle0'}} > > So x-blockdev-reopen sees that we want to replace the current bs->file > ("hd0-file") with a new one ("throttle0"). The problem here is that > throttle0 has hd0-file as its child, so when we check the permissions on > throttle0 (and its children) we get that hd0-file refuses because it's > already being used (although in in the process of being replaced) by > hd0: > > "Conflicts with use by hd0 as 'file', which does not allow 'write, resize' on hd0-file" > > And we would get a similar problem with the reverse operation (removing > the filter). This kind of situation isn't new, I believe some of the existing graph changes (iirc in the context of block jobs) can cause the same problem. This is essentially why some functions in the permission system take a GSList *ignore_children. So I believe the right thing to do here is telling the permission system that it needs to check the situation without the BdrvChild that links hd0 with hd0-file. I don't know the exact stack trace of your failure, so maybe this parameter isn't available yet in the place where you need it, but in the core functions it exists. Does this help or am I missing some details? Kevin