On 2015-01-28 at 15:18, Eric Blake wrote:
On 01/27/2015 12:46 PM, Max Reitz wrote:
And a helper function for that which directly takes a pointer to the BDS
to be inserted instead of its node-name (which will be used for
implementing 'change' using blockdev-insert-medium).
Signed-off-by: Max Reitz <mre...@redhat.com>
---
blockdev.c | 43 +++++++++++++++++++++++++++++++++++++++++++
qapi/block-core.json | 17 +++++++++++++++++
qmp-commands.hx | 38 ++++++++++++++++++++++++++++++++++++++
3 files changed, 98 insertions(+)
+static void qmp_blockdev_insert_anon_medium(const char *device,
+ BlockDriverState *bs, Error **errp)
+{
+ BlockBackend *blk;
+
+ blk = blk_by_name(device);
+ if (!blk) {
+ error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+ return;
+ }
+
+ if (!blk_dev_has_removable_media(blk)) {
+ error_setg(errp, "Device '%s' is not removable", device);
+ return;
+ }
+
+ if (!blk_dev_is_tray_open(blk)) {
+ error_setg(errp, "Tray of device '%s' is not open", device);
+ return;
+ }
+
+ if (blk_bs(blk)) {
+ error_setg(errp, "There already is a medium in device '%s'", device);
+ return;
+ }
Good, you didn't implement hot-swap semantics of replacing an existing
medium (that gets too confusing; so I _like_ that you force the user to
consider all the steps through multiple low-level commands).
+
+Example (1):
I'll quit pointing it out; but if you clean up the useless (1) in one
patch, do it across the series.
Yes, will do.
+
+-> { "execute": "blockdev-add",
+ "arguments": { "options": { "id": "backend0",
+ "node-name": "node0",
Why is 'id' needed?
Oops, because I forgot removing it after I added the functionality for
creating a BDS tree without a BB. I'll remove it, thanks for catching it.
Max
Isn't the point of this command sequence to create
a BDS tree that is NOT tied to a BB, and then use insert-medium to make
the association after the fact? We don't need to create a BB named
'backend0' if we are immediately going to reuse 'node0' in a different
BB (true, we have somewhat anticipated the idea of sharing BDS tree
among multiple BB, but haven't quite turned that on before now).
+ "driver": "raw",
+ "file": { "driver": "file",
+ "filename": "fedora.iso" } } } }
+
+<- { "return": {} }
+
+-> { "execute": "blockdev-insert-medium",
+ "arguments": { "device": "ide1-cd0",
+ "node-name": "node0" } }
+
+<- { "return": {} }
If you can either explain why you used 'id' in the example, or remove
that parameter, you can add:
Reviewed-by: Eric Blake <ebl...@redhat.com>
