From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41014) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y1wVY-0004sI-40 for qemu-devel@nongnu.org; Fri, 19 Dec 2014 07:18:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y1wVR-0001Fp-TY for qemu-devel@nongnu.org; Fri, 19 Dec 2014 07:18:12 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49634) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y1wVR-0001FG-MR for qemu-devel@nongnu.org; Fri, 19 Dec 2014 07:18:05 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sBJCI4ZS010570 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Fri, 19 Dec 2014 07:18:04 -0500 Date: Fri, 19 Dec 2014 13:18:01 +0100 From: Kevin Wolf Message-ID: <20141219121801.GA3724@noname.str.redhat.com> References: <87vbltvpl0.fsf@blackfin.pond.sub.org> <87mw6l9e04.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87mw6l9e04.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] Review of ways to create BDSes (was: Review of monitor commands identifying BDS / BB by name) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Fam Zheng , qemu-devel@nongnu.org, Stefan Hajnoczi , mreitz@redhat.com Am 18.12.2014 um 16:25 hat Markus Armbruster geschrieben: > = Introduction = > > BDSes can be opened in various ways. Some of them don't provide for > user configuration. Some of them should. > > Example: -drive format=qcow2,file=foo.qcow2 where foo.qcow2 has a raw > backing file foo.raw. This creates the the following tree of BDSes: > > (qcow2,foo.qcow2) > / \ > (file,foo.qcow2) (raw,foo.raw) > | > (file,foo.raw) > > Before Kevin added driver-specific options, -drive let you configure > basically just the root. Configuration for the others was inferred from > the root's configuration and the images. Driver-specific options let > you configure all the nodes. Defaults are still inferred as before. > > Example: blockdev-snapshot-sync provides only a small subset of the full > configuration options for the BDS it creates. Could be fixed by > duplicating the full options, i.e. blockdev-add's. But a command that > just snapshots and leaves BDS creation to blockdev-add would be cleaner. > > This is a systematic review of all the ways you can open BDSes in qemu > proper, i.e. not in qemu-{img,io,nbd}. I tracked them down by following > the call chains leading to bdrv_open(). Possibly also interesting for a followup: All the ways you can reopen BDSes with different options/flags. > = QMP Commands = > > * block-commit > > I figure this clones the @base BDS initially, and replaces it by the > clone finally. Is support for user configuration for this clone > wanted? I don't think such a clone exists. Data is directly committed to @base. The command looks to me as if it could be okay. > * blockdev-add > > Boils down to a bdrv_open(), which is discussed in the next section. > > * blockdev-snapshot-sync > > Creates a new image and a new BDS with only a few configuration > options: @snapshot-file (the filename), @snapshot-node-name, > @format=qcow2, @mode. Conflates three operations: create image, open > image, snapshot. I guess we want to replace it by a basic snapshot > operation that can be used with with blockdev-add and some command to > create images. Yes. We should have called this one drive-snapshot, it fits better in the drive-* family of commands. We can call the real blockdev-style command blockdev-snapshot - it is still synchronous technically, but it doesn't do anything like bdrv_open() that could be blocking. > * change > > Closes, then opens a BDS with just two configuration options: @target > (the filename) and @arg (the format). Needs replacing. Max (added to CC) is working on it. > * drive-backup > > Similar to blockdev-snapshot-sync, except the filename is called > @target, and the node name can't be configured. I guess we want to > replace it by a basic backup operation. > > * drive-mirror > > Similar to blockdev-snapshot-sync, except the filename is called > @target, and the node name @node-name. I guess we want to replace it > by a basic mirror operation. Yes. We called these drive-* instead of blockdev-* intentionally, so that the latter names would be free for operations working on existing BDSes. > * transaction > > This is a wrapper around a list of transaction-capable commands. > Thus, nothing new here. > > > = Generic block layer = > > bdrv_open() opens a BDS and possibly children "file" and "backing" > according to its configuration. > > Subtypes of BlockdevOptionsGenericFormat have configuration for "file". > > Subtypes of BlockdevOptionsGenericCOWFormat additionally have > configuration for "backing" (defaults to "infer from COW image"). > > bdrv_open() can additionally splice in a QCOW2 BDS to implement > snapshot=on. No way to configure, but that's okay, because it's a > convenience feature, and to configure you simply do the splicing > explicitly. > > > = Block driver methods = > > == bdrv_create() == > > The BDSes created here are all internal temporaries of the method, hence > user configuration isn't needed. Correct? Filenames ought to be enough for everyone. Not. But at the moment all the callers can't deal with non-filename specifications of the image location, so that's a larger problem. > == bdrv_file_open() == > > * quorum_open() > > Creates / connects to its children according to configuration in > BlockdevOptionsQuorum. > > * blkdebug_open() > > Creates / connects to its children according to configuration in > BlockdevOptionsBlkdebug. > > * blkverify_open() > > Creates / connects to its children according to configuration in > BlockdevOptionsBlkverify. > > * vvfat's enable_write_target() > > You don't want to know. This would have been an interesting one for a change. ;-) > == bdrv_open() == > > * vmdk_open() > > Creates BDSes for its extents, configuration inferred from images. > Looks like this needs work. Very much so. > = Xen = > > blk_connect() calls bdrv_open() under a /* setup via xenbus */ heading. > Looks like backward compatibility crap to me. Are you sure? But Xen is another "you don't want to know" for me. Kevin