[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RESEND 41/50] blockdev: Add blockdev-insert-medi
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH RESEND 41/50] blockdev: Add blockdev-insert-medium |
Date: |
Wed, 28 Jan 2015 13:18:21 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
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 <address@hidden>
> ---
> 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 <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH RESEND 38/50] blockdev: Add blockdev-open-tray, (continued)
- [Qemu-devel] [PATCH RESEND 33/50] blockdev: Respect NULL BDS in do_drive_del(), Max Reitz, 2015/01/27
- [Qemu-devel] [PATCH RESEND 39/50] blockdev: Add blockdev-close-tray, Max Reitz, 2015/01/27
- [Qemu-devel] [PATCH RESEND 35/50] blockdev: Pull out blockdev option extraction, Max Reitz, 2015/01/27
- [Qemu-devel] [PATCH RESEND 41/50] blockdev: Add blockdev-insert-medium, Max Reitz, 2015/01/27
- Re: [Qemu-devel] [PATCH RESEND 41/50] blockdev: Add blockdev-insert-medium,
Eric Blake <=
- [Qemu-devel] [PATCH RESEND 40/50] blockdev: Add blockdev-remove-medium, Max Reitz, 2015/01/27
- [Qemu-devel] [PATCH RESEND 46/50] hmp: Use blockdev-change-medium for change command, Max Reitz, 2015/01/27
- [Qemu-devel] [PATCH RESEND 45/50] qmp: Introduce blockdev-change-medium, Max Reitz, 2015/01/27
- [Qemu-devel] [PATCH RESEND 42/50] blockdev: Implement eject with basic operations, Max Reitz, 2015/01/27