[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 17/36] qtest: Avoid dynamic JSON in
From: |
John Snow |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 17/36] qtest: Avoid dynamic JSON in ahci-test |
Date: |
Wed, 30 Nov 2016 16:02:37 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 |
On 11/30/2016 02:44 PM, Eric Blake wrote:
> As argued elsewhere, it's less code to maintain if we convert
> from a dynamic string passed to qobject_from_jsonv() to instead
> use a hand-built QDict.
>
> Rather than build up a QDict by manual qdict_put*() calls, we
> can let QAPI do the work for us. The result is more lines of
> code to initialize the QAPI struct, but the result will force us
> to track any changes to the qapi (whereas the dynamic JSON string
> would not detect qapi changes until runtime).
>
Benefit of doubt that you're right.
> Signed-off-by: Eric Blake <address@hidden>
> ---
> tests/ahci-test.c | 26 +++++++++++++++++++++-----
> 1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> index ef17629..dfa9c52 100644
> --- a/tests/ahci-test.c
> +++ b/tests/ahci-test.c
> @@ -32,6 +32,8 @@
>
> #include "qemu-common.h"
> #include "qemu/host-utils.h"
> +#include "qapi/qmp/qjson.h"
> +#include "qapi/qobject-output-visitor.h"
>
> #include "hw/pci/pci_ids.h"
> #include "hw/pci/pci_regs.h"
> @@ -1576,6 +1578,7 @@ static void test_atapi_tray(void)
> uint8_t port, sense, asc;
> uint64_t iso_size = ATAPI_SECTOR_SIZE;
> QDict *rsp;
> + QObject *args;
>
> fd = prepare_iso(iso_size, &tx, &iso);
> ahci = ahci_boot_and_enable("-drive if=none,id=drive0,file=%s,format=raw
> "
> @@ -1607,11 +1610,24 @@ static void test_atapi_tray(void)
> atapi_wait_tray(true);
>
> /* Re-insert media */
> - qmp_discard_response("{'execute': 'blockdev-add', "
> - "'arguments': {'node-name': 'node0', "
> - "'driver': 'raw', "
> - "'file': { 'driver': 'file', "
> - "'filename': %s }}}", iso);
> + {
> + BlockdevRef ref = {
> + .type = QTYPE_QDICT,
> + .u.definition = {
> + .driver = BLOCKDEV_DRIVER_FILE,
> + .u.file.filename = iso,
> + },
> + };
> + BlockdevOptions opts = {
> + .has_node_name = true,
> + .node_name = (char *)"node0",
> + .driver = BLOCKDEV_DRIVER_RAW,
> + .u.raw.file = &ref,
> + };
> + args = QAPI_TO_QOBJECT(BlockdevOptions, &opts, &error_abort);
> + }
> +
> + qmp_cmd_discard_response("blockdev-add", qobject_to_qdict(args));
> qmp_discard_response("{'execute': 'x-blockdev-insert-medium',"
> "'arguments': { 'device': 'drive0', "
> "'node-name': 'node0' }}");
>
I assume qmp_cmd_discard_response takes ownership of the object we just
built?
Assuming yes:
Reviewed-by: John Snow <address@hidden>