Re: [Qemu-devel] Re: RFC: blockdev_add & friends, brief rationale, QMP docs

[Anthony Liguori <anthony@codemonkey.ws>]

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.

Regards,

Anthony Liguori

> Would it make s1ense to add a password field to blockdev_add/change to
> avoid this?
>
> Kevin
>
>