From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60780) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uor10-0005zV-BG for qemu-devel@nongnu.org; Tue, 18 Jun 2013 04:11:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Uor0w-00017t-J7 for qemu-devel@nongnu.org; Tue, 18 Jun 2013 04:11:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:6300) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uor0w-000162-7U for qemu-devel@nongnu.org; Tue, 18 Jun 2013 04:11:42 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r5I8Bf1k018164 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 18 Jun 2013 04:11:41 -0400 Date: Tue, 18 Jun 2013 16:11:40 +0800 From: Fam Zheng Message-ID: <20130618081140.GA1498@localhost.nay.redhat.com> References: <51BEDCB9.5090905@redhat.com> <20130617135253.GB3994@dhcp-200-207.str.redhat.com> <51BF16B8.6040801@redhat.com> <20130617142605.GD3994@dhcp-200-207.str.redhat.com> <51BF213F.60601@redhat.com> <20130617151238.GF3994@dhcp-200-207.str.redhat.com> <20130618035854.GA17533@localhost.nay.redhat.com> <20130618063256.GB3640@dhcp-200-207.str.redhat.com> <20130618070023.GA1220@localhost.nay.redhat.com> <20130618075102.GC3640@dhcp-200-207.str.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130618075102.GC3640@dhcp-200-207.str.redhat.com> Subject: Re: [Qemu-devel] [PATCH] block: add 'backing' option to drive_add List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Paolo Bonzini , qemu-devel@nongnu.org, stefanha@redhat.com, armbru@redhat.com On Tue, 06/18 09:51, Kevin Wolf wrote: > Am 18.06.2013 um 09:00 hat Fam Zheng geschrieben: > > On Tue, 06/18 08:32, Kevin Wolf wrote: > > > Am 18.06.2013 um 05:58 hat Fam Zheng geschrieben: > > > > On Mon, 06/17 17:12, Kevin Wolf wrote: > > > > > Am 17.06.2013 um 16:46 hat Paolo Bonzini geschrieben: > > > > > > Il 17/06/2013 16:26, Kevin Wolf ha scritto: > > > > > > > Am 17.06.2013 um 16:01 hat Paolo Bonzini geschrieben: > > > > > > >> Il 17/06/2013 15:52, Kevin Wolf ha scritto: > > > > > > >>> It's not a new thought that we need to change the block layer so that a > > > > > > >>> BlockDriverState can't be "empty", but that one BlockDriverState always > > > > > > >>> refers to one image. If you change media, you attach a different > > > > > > >>> BlockDriverState to the device. Once you have this, you can start > > > > > > >>> refcounting BlockDriverStates, so that the backing file remains usable > > > > > > >>> while the guest device already uses a different image. > > > > > > >>> > > > > > > >>> Not that it's it easy to get there... > > > > > > >> > > > > > > >> I'm not sure that is safe to do. > > > > > > >> > > > > > > >> Consider the case where the guest switches from A to B during backup, > > > > > > >> and then from B to A. You get two BDS for the same file, which pretty > > > > > > >> much means havoc. > > > > > > > > > > > > > > Well, yes, it means that the management tool needs to know what it's > > > > > > > doing. It shouldn't create a second BDS for A, but reattach the still > > > > > > > existing one. In this case do you mean mgmt tool should give a name of drive instead of file path? I like this idea, and further more, why don't we make QEMU smarter to bdrv_find_by_filename() the existing BDS? > > > > > > > > > > > > How? That would require the management tool to know the full chain of > > > > > > BDSes that were opened in the past. > > > > > > > > > > They better know on which files they are operating. It's not like the > > > > > management could be unaware of running backup jobs or things like that. > > > > > > > > > > > > > Is there any case that QEMU needs to have two BDS pointing to the same > > > > file? > > > > > > No, I think there's no case where this would make sense. > > > > > > > If not, can we try to detect such case on opening and try to > > > > reuse the bs? > > > > > > We can't do it reliably, think about symlinks or even hard links, or > > > things like /dev/fdset/..., let alone remote protocols that refer to the > > > same image file etc. > > > > > > We can check the obvious cases and error out for them, but that's about > > > what we can do. I don't think we should try to fix things automagically > > > when we can't do it right. > > > > It's impossible to know a remote protocol points to the same image with > > local file path, that's not in QEMU's scope, but we have a good chance > > to detect (strcmp with existing bs->filename) and error out Paolo's > > A-B-A problem, don't we? > > Yes, catching 50% of the misuses is better than catching none. > > My point was that we shouldn't "try to reuse the bs" when we detect that > the file is already open, because that makes it a feature that users are > supposed to use and that doesn't work consistently across backends and > will therefore cause endless pain. OK. > > If we detect it (in order to protect the user from his own mistakes), we > must treat it as a misuse and return an error. > IIUC, block job is not supposed to affect the guest or the source image, so from user's PoV, switching to another image, then switching back seems reasonable, even when a block job runs in the background. As we know it's already open, could we reattach to it instead, as you suggested above? -- Fam