On Fri, 28 May 2010 14:17:07 -0500
Anthony Liguori <anth...@codemonkey.ws> wrote:
> On 05/28/2010 02:13 PM, Kevin Wolf wrote:
> > 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?
> >
>
> If a password is needed, we should throw an error and let the QMP client
> set the password and try again.
It's what we do today, a password should be set with block_passwd before
issuing the change command. Otherwise an error is throw.
>
> Regards,
>
> Anthony Liguori
>
> > Would it make s1ense to add a password field to blockdev_add/change to
> > avoid this?
> >
> > Kevin
> >
> >
>
