From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=41476 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OI50T-0008Mq-Rh for qemu-devel@nongnu.org; Fri, 28 May 2010 15:14:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OI50R-00062I-AJ for qemu-devel@nongnu.org; Fri, 28 May 2010 15:14:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57702) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OI50R-000620-3S for qemu-devel@nongnu.org; Fri, 28 May 2010 15:14:07 -0400 Received: from int-mx08.intmail.prod.int.phx2.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o4SJE5GD018405 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 28 May 2010 15:14:06 -0400 Message-ID: <4C0015E6.50805@redhat.com> Date: Fri, 28 May 2010 21:13:42 +0200 From: Kevin Wolf MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: RFC: blockdev_add & friends, brief rationale, QMP docs List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Avi Kivity , Gerd Hoffmann , qemu-devel@nongnu.org, Luiz Capitulino Am 28.05.2010 20:21, schrieb Markus Armbruster: > I'd like to give posting documentation of new QMP commands for review > before posting code a try. But first let me explain briefly why we need > new commands. > > We want a clean separation between host part (blockdev_add) and guest > part (device_add). Existing -drive and drive_add don't provide that; > they were designed to specify both parts together. Moreover, drive_add > is limited to adding virtio drives (with pci_add's help) and SCSI > drives. > > Support for defining just a host part for use with -device and was > grafted onto -drive (if=none), but it's a mess. Some parts are > redundant, other parts are broken. > > For instance, unit, bus, index, addr are redundant: -device does not use > them, it provides its own parameters to specify bus and bus-specific > address. > > The checks whether rerror, werror, readonly, cyls, heads, secs are sane > for a particular guest driver are broken. The checks are in the -drive > code, which used to know the guest driver, but doesn't with if=none. > > Additionally, removable media are flawed. Many parameters set with > -drive silently revert to defaults on media change. > > My proposed solution is a new option -blockdev and monitor command > blockdev_add. These specify only the host drive. Guest drive > properties are left to -device / device_add. We keep -drive for > backwards compatibility and command line convenience. Except we get rid > of if=none (may need a grace period). > > New monitor command blockdev_del works regardless of how the host block > device was created. > > New monitor command blockdev_change provides full control over the host > part, unlike the existing change command. > > Summary of the host / guest split: > > -drive options host or guest? > bus, unit, if, index, addr guest, already covered by qdev > cyls, heads, secs, trans guest, new qdev properties > (but defaults depend on image) > media guest > snapshot, file, cache, aio, format host, blockdev_add options > rerror, werror host, guest drivers will reject > values they don't support > serial guest, new qdev properties > readonly both host & guest, qdev will refuse > to connect readonly host to read/ > write guest > > QMP command docs: > > blockdev_add > ------------ > > Add host block device. > > Arguments: > > - "id": the host block device's ID, must be unique (json-string) > - "file": the disk image file to use (json-string, optional) > - "format": disk format (json-string, optional) > - Possible values: "raw", "qcow2", ... > - "aio": host AIO (json-string, optional) > - Possible values: "threads" (default), "native" > - "cache": host cache usage (json-string, optional) > - Possible values: "writethrough" (default), "writeback", "unsafe", > "none" > - "readonly": open image read-only (json-bool, optional, default false) > - "rerror": what to do on read error (json-string, optional) > - Possible values: "report" (default), "ignore", "stop" > - "werror": what to do on write error (json-string, optional) > - Possible values: "enospc" (default), "report", "ignore", "stop" > - "snapshot": enable snapshot (json-bool, optional, default false) > > Example: > > -> { "execute": "blockdev_add", > "arguments": { "format": "raw", "id": "blk1", > "file": "fedora.img" } } > <- { "return": {} } > > Notes: > > (1) If argument "file" is missing, all other optional arguments must be > missing as well. This defines a block device with no media > inserted. > > (2) It's possible to list supported disk formats by running QEMU with > arguments "-blockdev_add \?". -blockdev without _add you probably mean, if it's a command line option. Maybe one more thing to consider is encrypted images. With "change" in the user monitor you're automatically prompted for the password. I'm not sure how this is supposed to work with QMP. From the do_change_block code it looks as if you'd get an error and had to send a block_set_passwd as a response to that. In the meantime the image would be kind of half-open? What do devices do with it until the key is provided? Would it make sense to add a password field to blockdev_add/change to avoid this? Kevin