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. > + > +-> { "execute": "blockdev-add", > + "arguments": { "options": { "id": "backend0", > + "node-name": "node0", Why is 'id' needed? 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> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature