Re: [Qemu-block] [Qemu-devel] [PATCH v3 12/12] docs/interop/firmware.json: Prefer -machine to if=pflash
Michal Privoznik writes:
> On 3/8/19 2:14 PM, Markus Armbruster wrote:
>> The previous commit added a way to configure firmware with -blockdev
>> rather than -drive if=pflash. Document it as the preferred way.
>>
>> Signed-off-by: Markus Armbruster
>> ---
>> docs/interop/firmware.json | 20 ++--
>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
>> index 28f9bc1591..ff8c2ce5f2 100644
>> --- a/docs/interop/firmware.json
>> +++ b/docs/interop/firmware.json
>> @@ -212,9 +212,13 @@
>> #
>> # @executable: Identifies the firmware executable. The firmware
>> # executable may be shared by multiple virtual machine
>> -# definitions. The corresponding QEMU command line option
>> -# is "-drive
>> -#
>> if=pflash,unit=0,readonly=on,file=@executable.@filename,format=@executable.@format".
>> +# definitions. The preferred corresponding QEMU command
>> +# line options are
>> +# -drive
>> if=none,id=pflash0,readonly=on,file=@executable.@filename,format=@executable.@format
>> +# -machine pflash0=pflash0
>
> I have a question. How is libvirt supposed to query for this? How can
> it learn it can use this new, preferred command line?
You can use qom-list-properties to find out whether the machine has
property pflash0.
---> {"execute": "qom-list-properties", "arguments": {"typename":
"pc-q35-4.0-machine"}}
<--- {"return": [... {"name": "pflash0", ...} ...]}
---> {"execute": "qom-list-properties", "arguments": {"typename":
"isapc-machine"}}
<--- {"return": [... no such property ...]}
Re: [Qemu-block] [Qemu-devel] [PATCH] xen-block: Replace qdict_put_obj() by qdict_put() where appropriate
Kevin Wolf writes: > Am 13.03.2019 um 18:44 hat Markus Armbruster geschrieben: >> Patch created mechanically by rerunning: >> >> $ spatch --sp-file scripts/coccinelle/qobject.cocci \ >> --macro-file scripts/cocci-macro-file.h \ >> --dir hw/block --in-place >> >> Signed-off-by: Markus Armbruster > > Reviewed-by: Kevin Wolf Thanks! > Which tree should this go through? The Xen one? Fine with me. I could also include it in a "miscellaneous cleanup" pull request along with other cleanup patches I got in flight.
[Qemu-block] Combining synchronous and asynchronous IO
Hi, Our current AIO path does a great job at unloading the work from the VM, and combined with IOThreads provides a good performance in most scenarios. But it also comes with its costs, in both a longer execution path and the need of the intervention of the scheduler at various points. There's one particular workload that suffers from this cost, and that's when you have just 1 or 2 cores on the Guest issuing synchronous requests. This happens to be a pretty common workload for some DBs and, in a general sense, on small VMs. I did a quick'n'dirty implementation on top of virtio-blk to get some numbers. This comes from a VM with 4 CPUs running on an idle server, with a secondary virtio-blk disk backed by a null_blk device with a simulated latency of 30us. - Average latency (us) || AIO+iothread | SIO+iothread | | 1 job | 70 | 55 | | 2 jobs | 83 | 82 | | 4 jobs | 90 | 159 | In this case the intuition matches the reality, and synchronous IO wins when there's just 1 job issuing the requests, while it loses hard when the are 4. While my first thought was implementing this as a tunable, turns out we have a hint about the nature of the workload in the number of the requests in the VQ. So I updated the code to use SIO if there's just 1 request and AIO otherwise, with these results: --- || AIO+iothread | SIO+iothread | AIO+SIO+iothread | | 1 job | 70 | 55 |55| | 2 jobs | 83 | 82 |78| | 4 jobs | 90 | 159 |90| --- This data makes me think this is something worth pursuing, but I'd like to hear your opinion on it. Thanks, Sergio.
Re: [Qemu-block] [PULL 13/28] file-posix: Prepare permission code for fd switching
On Tue, 12 Mar 2019 at 17:30, Kevin Wolf wrote:
>
> In order to be able to dynamically reopen the file read-only or
> read-write, depending on the users that are attached, we need to be able
> to switch to a different file descriptor during the permission change.
>
> This interacts with reopen, which also creates a new file descriptor and
> performs permission changes internally. In this case, the permission
> change code must reuse the reopen file descriptor instead of creating a
> third one.
>
> In turn, reopen can drop its code to copy file locks to the new file
> descriptor because that is now done when applying the new permissions.
Hi -- Coverity doesn't like this function (CID 1399712).
I think this may be a false positive, but could you confirm?
> @@ -2696,12 +2695,78 @@ static QemuOptsList raw_create_opts = {
> static int raw_check_perm(BlockDriverState *bs, uint64_t perm, uint64_t
> shared,
>Error **errp)
> {
> -return raw_handle_perm_lock(bs, RAW_PL_PREPARE, perm, shared, errp);
> +BDRVRawState *s = bs->opaque;
> +BDRVRawReopenState *rs = NULL;
> +int open_flags;
> +int ret;
> +
> +if (s->perm_change_fd) {
> +/*
> + * In the context of reopen, this function may be called several
> times
> + * (directly and recursively while change permissions of the parent).
> + * This is even true for children that don't inherit from the
> original
> + * reopen node, so s->reopen_state is not set.
> + *
> + * Ignore all but the first call.
> + */
> +return 0;
> +}
> +
> +if (s->reopen_state) {
> +/* We already have a new file descriptor to set permissions for */
> +assert(s->reopen_state->perm == perm);
> +assert(s->reopen_state->shared_perm == shared);
> +rs = s->reopen_state->opaque;
> +s->perm_change_fd = rs->fd;
> +} else {
> +/* We may need a new fd if auto-read-only switches the mode */
> +ret = raw_reconfigure_getfd(bs, bs->open_flags, &open_flags,
> +false, errp);
Coverity says that raw_reconfigure_getfd() returns an fd in 'ret' here...
> +if (ret < 0) {
> +return ret;
> +} else if (ret != s->fd) {
> +s->perm_change_fd = ret;
> +}
> +}
> +
> +/* Prepare permissions on old fd to avoid conflicts between old and new,
> + * but keep everything locked that new will need. */
> +ret = raw_handle_perm_lock(bs, RAW_PL_PREPARE, perm, shared, errp);
...but this call overwrites that fd, so we might never close it.
I think the answer is that either:
* ret == s->fd and we'll close s->fd later
* or we save ret into s->perm_change_fd
and Coverity just isn't clever enough to realise that if
ret == s->fd then we haven't lost the handle.
Is that right? If so I'll mark it as a false-positive in the UI.
thanks
-- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 34/54] commit: Support multiple roots above top node
On Fri, 6 Oct 2017 at 17:20, Kevin Wolf wrote:
>
> This changes the commit block job to support operation in a graph where
> there is more than a single active layer that references the top node.
>
> This involves inserting the commit filter node not only on the path
> between the given active node and the top node, but between the top node
> and all of its parents.
>
> On completion, bdrv_drop_intermediate() must consider all parents for
> updating the backing file link. These parents may be backing files
> themselves and as such read-only; reopen them temporarily if necessary.
> Previously this was achieved by the bdrv_reopen() calls in the commit
> block job that made overlay_bs read-write for the whole duration of the
> block job, even though write access is only needed on completion.
>
> Now that we consider all parents, overlay_bs is meaningless. It is left
> in place in this commit, but we'll remove it soon.
>
> Signed-off-by: Kevin Wolf
Hi -- a recent change to the block layer has caused Coverity
to flag up a possible issue with this older commit: CID 1399710:
> @@ -3492,42 +3504,42 @@ int bdrv_drop_intermediate(BlockDriverState *active,
> BlockDriverState *top,
> goto exit;
> }
>
> -new_top_bs = bdrv_find_overlay(active, top);
> -
> -if (new_top_bs == NULL) {
> -/* we could not find the image above 'top', this is an error */
> -goto exit;
> -}
> -
> -/* special case of new_top_bs->backing->bs already pointing to base -
> nothing
> - * to do, no intermediate images */
> -if (backing_bs(new_top_bs) == base) {
> -ret = 0;
> -goto exit;
> -}
> -
> /* Make sure that base is in the backing chain of top */
> if (!bdrv_chain_contains(top, base)) {
> goto exit;
> }
>
> /* success - we can delete the intermediate states, and link top->base */
> -if (new_top_bs->backing->role->update_filename) {
> -backing_file_str = backing_file_str ? backing_file_str :
> base->filename;
> -ret = new_top_bs->backing->role->update_filename(new_top_bs->backing,
> - base,
> backing_file_str,
> - &local_err);
> -if (ret < 0) {
> -bdrv_set_backing_hd(new_top_bs, top, &error_abort);
> +backing_file_str = backing_file_str ? backing_file_str : base->filename;
> +
> +QLIST_FOREACH_SAFE(c, &top->parents, next_parent, next) {
> +/* Check whether we are allowed to switch c from top to base */
> +GSList *ignore_children = g_slist_prepend(NULL, c);
> +bdrv_check_update_perm(base, NULL, c->perm, c->shared_perm,
> + ignore_children, &local_err);
Here we don't check the return value from bdrv_check_update_perm(),
which we do in all four other places that we call it. I think this
is probably a false positive in that the function will only
return ret < 0 if it also returns a failure via local_err, but
it might be worth being consistent just to placate Coverity.
> +if (local_err) {
> +ret = -EPERM;
> +error_report_err(local_err);
> goto exit;
> }
> -}
Happy to just mark the issue as a false-positive in the Coverity
UI if you think that's the best resolution.
thanks
-- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH v3 12/12] docs/interop/firmware.json: Prefer -machine to if=pflash
On 3/8/19 2:14 PM, Markus Armbruster wrote: The previous commit added a way to configure firmware with -blockdev rather than -drive if=pflash. Document it as the preferred way. Signed-off-by: Markus Armbruster --- docs/interop/firmware.json | 20 ++-- 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json index 28f9bc1591..ff8c2ce5f2 100644 --- a/docs/interop/firmware.json +++ b/docs/interop/firmware.json @@ -212,9 +212,13 @@ # # @executable: Identifies the firmware executable. The firmware # executable may be shared by multiple virtual machine -# definitions. The corresponding QEMU command line option -# is "-drive -# if=pflash,unit=0,readonly=on,file=@executable.@filename,format=@executable.@format". +# definitions. The preferred corresponding QEMU command +# line options are +# -drive if=none,id=pflash0,readonly=on,file=@executable.@filename,format=@executable.@format +# -machine pflash0=pflash0 I have a question. How is libvirt supposed to query for this? How can it learn it can use this new, preferred command line? Thanks, Michal
Re: [Qemu-block] [PATCH] vmdk: Support version=3 in VMDK descriptor files
Am 14.03.2019 um 15:14 hat Sam Eiderman geschrieben: > Commit 509d39aa22909c0ed1aabf896865f19c81fb38a1 added support for read > only VMDKs of version 3. > > This commit fixes the probe function to correctly handle descriptors of > version 3. > > This commit has two effects: > 1. We no longer need to supply '-f vmdk' when pointing to descriptor >files of version 3 in qemu/qemu-img command line arguments. > 2. This fixes the scenario where a VMDK points to a parent version 3 >descriptor file which is being probed as "raw" instead of "vmdk". > > Reviewed-by: Arbel Moshe > Reviewed-by: Mark Kanda > Signed-off-by: Shmuel Eiderman Thanks, applied to the block branch. Kevin
Re: [Qemu-block] [PATCH] xen-block: Replace qdict_put_obj() by qdict_put() where appropriate
Am 13.03.2019 um 18:44 hat Markus Armbruster geschrieben: > Patch created mechanically by rerunning: > > $ spatch --sp-file scripts/coccinelle/qobject.cocci \ > --macro-file scripts/cocci-macro-file.h \ > --dir hw/block --in-place > > Signed-off-by: Markus Armbruster Reviewed-by: Kevin Wolf Which tree should this go through? The Xen one? Kevin
[Qemu-block] [PATCH] vmdk: Support version=3 in VMDK descriptor files
Commit 509d39aa22909c0ed1aabf896865f19c81fb38a1 added support for read
only VMDKs of version 3.
This commit fixes the probe function to correctly handle descriptors of
version 3.
This commit has two effects:
1. We no longer need to supply '-f vmdk' when pointing to descriptor
files of version 3 in qemu/qemu-img command line arguments.
2. This fixes the scenario where a VMDK points to a parent version 3
descriptor file which is being probed as "raw" instead of "vmdk".
Reviewed-by: Arbel Moshe
Reviewed-by: Mark Kanda
Signed-off-by: Shmuel Eiderman
---
block/vmdk.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index d8c0c50390..8dec6ef767 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -195,13 +195,15 @@ static int vmdk_probe(const uint8_t *buf, int buf_size,
const char *filename)
}
if (end - p >= strlen("version=X\n")) {
if (strncmp("version=1\n", p, strlen("version=1\n")) == 0 ||
-strncmp("version=2\n", p, strlen("version=2\n")) == 0) {
+strncmp("version=2\n", p, strlen("version=2\n")) == 0 ||
+strncmp("version=3\n", p, strlen("version=3\n")) == 0) {
return 100;
}
}
if (end - p >= strlen("version=X\r\n")) {
if (strncmp("version=1\r\n", p, strlen("version=1\r\n")) == 0
||
-strncmp("version=2\r\n", p, strlen("version=2\r\n")) == 0)
{
+strncmp("version=2\r\n", p, strlen("version=2\r\n")) == 0
||
+strncmp("version=3\r\n", p, strlen("version=3\r\n")) == 0)
{
return 100;
}
}
--
2.13.3
Re: [Qemu-block] [PATCH] block/io: fix bdrv_co_do_copy_on_readv error handling
14.03.2019 13:14, Vladimir Sementsov-Ogievskiy wrote:
> It's not safe to treat bdrv_is_allocated error as unallocated: if we
> mistake we may rewrite guest data.
... with same data, which is not so bad.
So, it's ok, I'm wrong, drop it.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> block/io.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/block/io.c b/block/io.c
> index 2ba603c7bc..dccad64d46 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1193,11 +1193,13 @@ static int coroutine_fn
> bdrv_co_do_copy_on_readv(BdrvChild *child,
> ret = bdrv_is_allocated(bs, cluster_offset,
> MIN(cluster_bytes, max_transfer), &pnum);
> if (ret < 0) {
> -/* Safe to treat errors in querying allocation as if
> - * unallocated; we'll probably fail again soon on the
> - * read, but at least that will set a decent errno.
> +/*
> + * We must fail here, and can't treat error as allocated or
> + * unallocated: if we mistake, it will lead to not done
> copy-on-read
> + * in first case and to rewriting guest data that is already in
> top
> + * layer in the second case.
>*/
> -pnum = MIN(cluster_bytes, max_transfer);
> +goto err;
> }
>
> /* Stop at EOF if the image ends in the middle of the cluster */
> @@ -1208,7 +1210,7 @@ static int coroutine_fn
> bdrv_co_do_copy_on_readv(BdrvChild *child,
>
> assert(skip_bytes < pnum);
>
> -if (ret <= 0) {
> +if (ret == 0) {
> /* Must copy-on-read; use the bounce buffer */
> pnum = MIN(pnum, MAX_BOUNCE_BUFFER);
> qemu_iovec_init_buf(&local_qiov, bounce_buffer, pnum);
>
--
Best regards,
Vladimir
Re: [Qemu-block] [Qemu-devel] [PULL 03/28] qapi: move to QOM path for x-block-latency-histogram-set
Am 14.03.2019 um 09:47 hat Vladimir Sementsov-Ogievskiy geschrieben: > 12.03.2019 20:30, Kevin Wolf wrote: > > From: Vladimir Sementsov-Ogievskiy > > > > Signed-off-by: Vladimir Sementsov-Ogievskiy > > Signed-off-by: Kevin Wolf > > Not critical, but it is v4, when in v5 description and examples were > fixed to be s/device/id. I'll send a follow-up. Oops, sorry. Thanks for noticing and sending a follow-up so quickly. Kevin
Re: [Qemu-block] [PATCH] qapi: fix block-latency-histogram-set description and examples
Am 14.03.2019 um 09:52 hat Vladimir Sementsov-Ogievskiy geschrieben: > There no @device parameter, only the @id one. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Thanks, applied to the block branch. Kevin
Re: [Qemu-block] [PATCH for-4.0] qapi: fix block-latency-histogram-set description and examples
On 3/14/19 3:52 AM, Vladimir Sementsov-Ogievskiy wrote: > There no @device parameter, only the @id one. > > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > qapi/block-core.json | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) Documentation fixes are appropriate during freeze. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-block] [PULL for-4.0 0/1] Block patches
On Wed, 13 Mar 2019 at 11:08, Stefan Hajnoczi wrote: > > The following changes since commit 3f3bbfc7cef4490c5ed5550766a81e7d18f08db1: > > Merge remote-tracking branch > 'remotes/huth-gitlab/tags/pull-request-2019-03-12' into staging (2019-03-12 > 21:06:26 +) > > are available in the Git repository at: > > git://github.com/stefanha/qemu.git tags/block-pull-request > > for you to fetch changes up to f357fcd890a8d6ced6d261338b859a41414561e9: > > file-posix: add drop-cache=on|off option (2019-03-13 10:54:55 +) > > > Pull request > > * Add 'drop-cache=on|off' option to file-posix.c. The default is on. >Disabling the option fixes a QEMU 3.0.0 performance regression when live >migrating on the same host with cache.direct=off. > > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0 for any user-visible changes. -- PMM
[Qemu-block] [PATCH] block/io: fix bdrv_co_do_copy_on_readv error handling
It's not safe to treat bdrv_is_allocated error as unallocated: if we
mistake we may rewrite guest data.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/io.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/block/io.c b/block/io.c
index 2ba603c7bc..dccad64d46 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1193,11 +1193,13 @@ static int coroutine_fn
bdrv_co_do_copy_on_readv(BdrvChild *child,
ret = bdrv_is_allocated(bs, cluster_offset,
MIN(cluster_bytes, max_transfer), &pnum);
if (ret < 0) {
-/* Safe to treat errors in querying allocation as if
- * unallocated; we'll probably fail again soon on the
- * read, but at least that will set a decent errno.
+/*
+ * We must fail here, and can't treat error as allocated or
+ * unallocated: if we mistake, it will lead to not done
copy-on-read
+ * in first case and to rewriting guest data that is already in top
+ * layer in the second case.
*/
-pnum = MIN(cluster_bytes, max_transfer);
+goto err;
}
/* Stop at EOF if the image ends in the middle of the cluster */
@@ -1208,7 +1210,7 @@ static int coroutine_fn
bdrv_co_do_copy_on_readv(BdrvChild *child,
assert(skip_bytes < pnum);
-if (ret <= 0) {
+if (ret == 0) {
/* Must copy-on-read; use the bounce buffer */
pnum = MIN(pnum, MAX_BOUNCE_BUFFER);
qemu_iovec_init_buf(&local_qiov, bounce_buffer, pnum);
--
2.18.0
Re: [Qemu-block] [PATCH] xen-block: Replace qdict_put_obj() by qdict_put() where appropriate
> -Original Message- > From: Markus Armbruster [mailto:arm...@redhat.com] > Sent: 13 March 2019 17:45 > To: qemu-de...@nongnu.org > Cc: sstabell...@kernel.org; Anthony Perard ; Paul > Durrant > ; xen-de...@lists.xenproject.org; > qemu-block@nongnu.org > Subject: [PATCH] xen-block: Replace qdict_put_obj() by qdict_put() where > appropriate > > Patch created mechanically by rerunning: > > $ spatch --sp-file scripts/coccinelle/qobject.cocci \ > --macro-file scripts/cocci-macro-file.h \ > --dir hw/block --in-place > > Signed-off-by: Markus Armbruster Acked-by: Paul Durrant > --- > hw/block/xen-block.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c > index 70fc2455e8..9c722b9b95 100644 > --- a/hw/block/xen-block.c > +++ b/hw/block/xen-block.c > @@ -771,7 +771,7 @@ static XenBlockDrive *xen_block_drive_create(const char > *id, > QDict *cache_qdict = qdict_new(); > > qdict_put_bool(cache_qdict, "direct", true); > -qdict_put_obj(file_layer, "cache", QOBJECT(cache_qdict)); > +qdict_put(file_layer, "cache", cache_qdict); > > qdict_put_str(file_layer, "aio", "native"); > } > @@ -796,7 +796,7 @@ static XenBlockDrive *xen_block_drive_create(const char > *id, > qdict_put_str(driver_layer, "driver", driver); > g_free(driver); > > -qdict_put_obj(driver_layer, "file", QOBJECT(file_layer)); > +qdict_put(driver_layer, "file", file_layer); > > g_assert(!drive->node_name); > drive->node_name = xen_block_blockdev_add(drive->id, driver_layer, > -- > 2.17.2
Re: [Qemu-block] [Qemu-devel] [PATCH] qcow2: Fix data file error condition in qcow2_co_create()
On Wed, Mar 13, 2019 at 03:25:19PM +0100, Kevin Wolf wrote: > We were trying to check whether bdrv_open_blockdev_ref() returned > success, but accidentally checked the wrong variable. Spotted by > Coverity (CID 1399703). > > Signed-off-by: Kevin Wolf > --- > block/qcow2.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Stefano Garzarella
Re: [Qemu-block] [Qemu-devel] [PULL 03/28] qapi: move to QOM path for x-block-latency-histogram-set
12.03.2019 20:30, Kevin Wolf wrote:
> From: Vladimir Sementsov-Ogievskiy
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> Signed-off-by: Kevin Wolf
Not critical, but it is v4, when in v5 description and examples were fixed to
be s/device/id.
I'll send a follow-up.
> ---
> qapi/block-core.json | 4 ++--
> blockdev.c | 12 ++--
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 919d0530b2..3f0a5cb1e8 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -550,7 +550,7 @@
> # If only @device parameter is specified, remove all present latency
> histograms
> # for the device. Otherwise, add/reset some of (or all) latency histograms.
> #
> -# @device: device name to set latency histogram for.
> +# @id: The name or QOM path of the guest device.
> #
> # @boundaries: list of interval boundary values (see description in
> # BlockLatencyHistogramInfo definition). If specified, all
> @@ -608,7 +608,7 @@
> # <- { "return": {} }
> ##
> { 'command': 'x-block-latency-histogram-set',
> - 'data': {'device': 'str',
> + 'data': {'id': 'str',
> '*boundaries': ['uint64'],
> '*boundaries-read': ['uint64'],
> '*boundaries-write': ['uint64'],
> diff --git a/blockdev.c b/blockdev.c
> index 871966ca13..850fb34c52 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -4453,21 +4453,21 @@ void qmp_x_blockdev_set_iothread(const char
> *node_name, StrOrNull *iothread,
> }
>
> void qmp_x_block_latency_histogram_set(
> -const char *device,
> +const char *id,
> bool has_boundaries, uint64List *boundaries,
> bool has_boundaries_read, uint64List *boundaries_read,
> bool has_boundaries_write, uint64List *boundaries_write,
> bool has_boundaries_flush, uint64List *boundaries_flush,
> Error **errp)
> {
> -BlockBackend *blk = blk_by_name(device);
> +BlockBackend *blk = qmp_get_blk(NULL, id, errp);
> BlockAcctStats *stats;
> int ret;
>
> if (!blk) {
> -error_setg(errp, "Device '%s' not found", device);
> return;
> }
> +
> stats = blk_get_stats(blk);
>
> if (!has_boundaries && !has_boundaries_read && !has_boundaries_write &&
> @@ -4482,7 +4482,7 @@ void qmp_x_block_latency_histogram_set(
> stats, BLOCK_ACCT_READ,
> has_boundaries_read ? boundaries_read : boundaries);
> if (ret) {
> -error_setg(errp, "Device '%s' set read boundaries fail", device);
> +error_setg(errp, "Device '%s' set read boundaries fail", id);
> return;
> }
> }
> @@ -4492,7 +4492,7 @@ void qmp_x_block_latency_histogram_set(
> stats, BLOCK_ACCT_WRITE,
> has_boundaries_write ? boundaries_write : boundaries);
> if (ret) {
> -error_setg(errp, "Device '%s' set write boundaries fail",
> device);
> +error_setg(errp, "Device '%s' set write boundaries fail", id);
> return;
> }
> }
> @@ -4502,7 +4502,7 @@ void qmp_x_block_latency_histogram_set(
> stats, BLOCK_ACCT_FLUSH,
> has_boundaries_flush ? boundaries_flush : boundaries);
> if (ret) {
> -error_setg(errp, "Device '%s' set flush boundaries fail",
> device);
> +error_setg(errp, "Device '%s' set flush boundaries fail", id);
> return;
> }
> }
>
--
Best regards,
Vladimir
[Qemu-block] [PATCH] qapi: fix block-latency-histogram-set description and examples
There no @device parameter, only the @id one.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
qapi/block-core.json | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index ca684a8a04..7c1d47365d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -565,7 +565,7 @@
#
# Manage read, write and flush latency histograms for the device.
#
-# If only @device parameter is specified, remove all present latency histograms
+# If only @id parameter is specified, remove all present latency histograms
# for the device. Otherwise, add/reset some of (or all) latency histograms.
#
# @id: The name or QOM path of the guest device.
@@ -597,7 +597,7 @@
# [0, 10), [10, 50), [50, 100), [100, +inf):
#
# -> { "execute": "block-latency-histogram-set",
-# "arguments": { "device": "drive0",
+# "arguments": { "id": "drive0",
# "boundaries": [10, 50, 100] } }
# <- { "return": {} }
#
@@ -605,7 +605,7 @@
# not changed (or not created):
#
# -> { "execute": "block-latency-histogram-set",
-# "arguments": { "device": "drive0",
+# "arguments": { "id": "drive0",
# "boundaries-write": [10, 50, 100] } }
# <- { "return": {} }
#
@@ -614,7 +614,7 @@
# write: [0, 1000), [1000, 5000), [5000, +inf)
#
# -> { "execute": "block-latency-histogram-set",
-# "arguments": { "device": "drive0",
+# "arguments": { "id": "drive0",
# "boundaries": [10, 50, 100],
# "boundaries-write": [1000, 5000] } }
# <- { "return": {} }
@@ -622,7 +622,7 @@
# Example: remove all latency histograms:
#
# -> { "execute": "block-latency-histogram-set",
-# "arguments": { "device": "drive0" } }
+# "arguments": { "id": "drive0" } }
# <- { "return": {} }
##
{ 'command': 'block-latency-histogram-set',
--
2.18.0
