[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 05/10] block: Accept node-name arguments for
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v5 05/10] block: Accept node-name arguments for block-commit |
Date: |
Tue, 17 Jun 2014 06:28:27 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 |
On 06/13/2014 12:52 PM, Jeff Cody wrote:
> This modifies the block operation block-commit so that it will
> accept node-name arguments for either 'top' or 'base' BDS.
>
> void qmp_block_commit(const char *device,
> bool has_base, const char *base,
> + bool has_base_node_name, const char *base_node_name,
> bool has_top, const char *top,
> + bool has_top_node_name, const char *top_node_name,
> bool has_speed, int64_t speed,
> Error **errp)
> {
> - BlockDriverState *bs;
> - BlockDriverState *base_bs, *top_bs;
> + BlockDriverState *bs = NULL;
> + BlockDriverState *base_bs = NULL;
> + BlockDriverState *top_bs = NULL;
> Error *local_err = NULL;
> /* This will be part of the QMP command, if/when the
> * BlockdevOnError change for blkmirror makes it in
> @@ -1940,20 +1943,33 @@ void qmp_block_commit(const char *device,
> /* drain all i/o before commits */
> bdrv_drain_all();
>
> - bs = bdrv_find(device);
> - if (!bs) {
> - error_set(errp, QERR_DEVICE_NOT_FOUND, device);
On IRC, we discussed that libvirt would like to rely on
QERR_DEVICE_NOT_FOUND (aka "class":"DeviceNotFound") as a witness probe
for whether qemu supports optional 'top' (which is a rough approximation
for whether qemu supports active commit, other than it mis-diagnoses
qemu 2.0; but libvirt probably wants to just require qemu 2.1 for active
commit anyways). Compare:
on 2.1: {"execute":"block-commit","arguments":{"device":"no-such"}}
{"error": {"class": "DeviceNotFound", "desc": "Device 'no-such' not found"}}
on 1.7: {"execute":"block-commit","arguments":{"device":"no-such"}}
{"error": {"class": "GenericError", "desc": "Parameter 'top' is missing"}}
It might be worth a comment in the code that the device not found error
from a failed bdrv_find() should occur as early as possible, at least in
the case where all optional parameters are omitted, to serve as a
libvirt probe point (it is okay for the code to have various
GenericError sanity checks occur before the bdrv_find, so long as those
checks can only be triggered when optional parameters are present - but
again, worth mentioning in a comment added to the code).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v5 01/10] block: Auto-generate node_names for each BDS entry, (continued)
- [Qemu-devel] [PATCH v5 01/10] block: Auto-generate node_names for each BDS entry, Jeff Cody, 2014/06/13
- [Qemu-devel] [PATCH v5 02/10] block: add helper function to determine if a BDS is in a chain, Jeff Cody, 2014/06/13
- [Qemu-devel] [PATCH v5 03/10] block: simplify bdrv_find_base() and bdrv_find_overlay(), Jeff Cody, 2014/06/13
- [Qemu-devel] [PATCH v5 04/10] block: make 'top' argument to block-commit optional, Jeff Cody, 2014/06/13
- [Qemu-devel] [PATCH v5 06/10] block: extend block-commit to accept a string for the backing file, Jeff Cody, 2014/06/13
- [Qemu-devel] [PATCH v5 05/10] block: Accept node-name arguments for block-commit, Jeff Cody, 2014/06/13
- Re: [Qemu-devel] [PATCH v5 05/10] block: Accept node-name arguments for block-commit,
Eric Blake <=
- [Qemu-devel] [PATCH v5 07/10] block: add ability for block-stream to use node-name, Jeff Cody, 2014/06/13
- [Qemu-devel] [PATCH v5 08/10] block: add backing-file option to block-stream, Jeff Cody, 2014/06/13
- [Qemu-devel] [PATCH v5 09/10] block: Add QMP documentation for block-stream, Jeff Cody, 2014/06/13
- [Qemu-devel] [PATCH v5 10/10] block: add QAPI command to allow live backing file change, Jeff Cody, 2014/06/13
- Re: [Qemu-devel] [PATCH v5 00/10] Modify block jobs to use node-names, Eric Blake, 2014/06/17