error: error_is_set() is finally unused; remove

Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
char: Explain qmp_chardev_add()'s unusual error handling
2014-05-21 11:57:58 +02:00 · 2014-05-21 11:57:58 +02:00 · 2014-05-21 11:57:58 +02:00 · 2014-05-21 11:57:57 +02:00 · 2014-05-21 11:57:57 +02:00 · 2014-05-19 14:10:01 +01:00
38 changed files with 1105 additions and 616 deletions
--- a/6
+++ b/6
@@ -659,6 +659,12 @@ S: Supported
 F: hw/block/nvme*
 F: tests/nvme-test.c

+megasas
+M: Hannes Reinecke <hare@suse.de>
+S: Supported
+F: hw/scsi/megasas.c
+F: hw/scsi/mfi.h
+
 Xilinx EDK
 M: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
 M: Edgar E. Iglesias <edgar.iglesias@gmail.com>
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -30,6 +30,8 @@
 #include "qemu-common.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
+#include "qemu/bitops.h"
+#include "qemu/bitmap.h"
 #include "block/block_int.h"
 #include "trace.h"
 #include "block/scsi.h"
@@ -59,6 +61,8 @@ typedef struct IscsiLun {
    struct scsi_inquiry_logical_block_provisioning lbp;
    struct scsi_inquiry_block_limits bl;
    unsigned char *zeroblock;
+    unsigned long *allocationmap;
+    int cluster_sectors;
 } IscsiLun;

 typedef struct IscsiTask {
@@ -92,6 +96,15 @@ typedef struct IscsiAIOCB {
 #define MAX_NOP_FAILURES 3
 #define ISCSI_CMD_RETRIES 5

+/* this threshhold is a trade-off knob to choose between
+ * the potential additional overhead of an extra GET_LBA_STATUS request
+ * vs. unnecessarily reading a lot of zero sectors over the wire.
+ * If a read request is greater or equal than ISCSI_CHECKALLOC_THRES
+ * sectors we check the allocation status of the area covered by the
+ * request first if the allocationmap indicates that the area might be
+ * unallocated. */
+#define ISCSI_CHECKALLOC_THRES 64
+
 static void
 iscsi_bh_cb(void *p)
 {
@@ -273,6 +286,32 @@ static bool is_request_lun_aligned(int64_t sector_num, int nb_sectors,
    return 1;
 }

+static void iscsi_allocationmap_set(IscsiLun *iscsilun, int64_t sector_num,
+                                    int nb_sectors)
+{
+    if (iscsilun->allocationmap == NULL) {
+        return;
+    }
+    bitmap_set(iscsilun->allocationmap,
+               sector_num / iscsilun->cluster_sectors,
+               DIV_ROUND_UP(nb_sectors, iscsilun->cluster_sectors));
+}
+
+static void iscsi_allocationmap_clear(IscsiLun *iscsilun, int64_t sector_num,
+                                      int nb_sectors)
+{
+    int64_t cluster_num, nb_clusters;
+    if (iscsilun->allocationmap == NULL) {
+        return;
+    }
+    cluster_num = DIV_ROUND_UP(sector_num, iscsilun->cluster_sectors);
+    nb_clusters = (sector_num + nb_sectors) / iscsilun->cluster_sectors
+                  - cluster_num;
+    if (nb_clusters > 0) {
+        bitmap_clear(iscsilun->allocationmap, cluster_num, nb_clusters);
+    }
+}
+
 static int coroutine_fn iscsi_co_writev(BlockDriverState *bs,
                                        int64_t sector_num, int nb_sectors,
                                        QEMUIOVector *iov)
@@ -336,9 +375,127 @@ retry:
        return -EIO;
    }

+    iscsi_allocationmap_set(iscsilun, sector_num, nb_sectors);
+
    return 0;
 }

+
+static bool iscsi_allocationmap_is_allocated(IscsiLun *iscsilun,
+                                             int64_t sector_num, int nb_sectors)
+{
+    unsigned long size;
+    if (iscsilun->allocationmap == NULL) {
+        return true;
+    }
+    size = DIV_ROUND_UP(sector_num + nb_sectors, iscsilun->cluster_sectors);
+    return !(find_next_bit(iscsilun->allocationmap, size,
+                           sector_num / iscsilun->cluster_sectors) == size);
+}
+
+
+#if defined(LIBISCSI_FEATURE_IOVECTOR)
+
+static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
+                                                  int64_t sector_num,
+                                                  int nb_sectors, int *pnum)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct scsi_get_lba_status *lbas = NULL;
+    struct scsi_lba_status_descriptor *lbasd = NULL;
+    struct IscsiTask iTask;
+    int64_t ret;
+
+    iscsi_co_init_iscsitask(iscsilun, &iTask);
+
+    if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
+        ret = -EINVAL;
+        goto out;
+    }
+
+    /* default to all sectors allocated */
+    ret = BDRV_BLOCK_DATA;
+    ret |= (sector_num << BDRV_SECTOR_BITS) | BDRV_BLOCK_OFFSET_VALID;
+    *pnum = nb_sectors;
+
+    /* LUN does not support logical block provisioning */
+    if (iscsilun->lbpme == 0) {
+        goto out;
+    }
+
+retry:
+    if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun,
+                                  sector_qemu2lun(sector_num, iscsilun),
+                                  8 + 16, iscsi_co_generic_cb,
+                                  &iTask) == NULL) {
+        ret = -ENOMEM;
+        goto out;
+    }
+
+    while (!iTask.complete) {
+        iscsi_set_events(iscsilun);
+        qemu_coroutine_yield();
+    }
+
+    if (iTask.do_retry) {
+        if (iTask.task != NULL) {
+            scsi_free_scsi_task(iTask.task);
+            iTask.task = NULL;
+        }
+        iTask.complete = 0;
+        goto retry;
+    }
+
+    if (iTask.status != SCSI_STATUS_GOOD) {
+        /* in case the get_lba_status_callout fails (i.e.
+         * because the device is busy or the cmd is not
+         * supported) we pretend all blocks are allocated
+         * for backwards compatibility */
+        goto out;
+    }
+
+    lbas = scsi_datain_unmarshall(iTask.task);
+    if (lbas == NULL) {
+        ret = -EIO;
+        goto out;
+    }
+
+    lbasd = &lbas->descriptors[0];
+
+    if (sector_qemu2lun(sector_num, iscsilun) != lbasd->lba) {
+        ret = -EIO;
+        goto out;
+    }
+
+    *pnum = sector_lun2qemu(lbasd->num_blocks, iscsilun);
+
+    if (lbasd->provisioning == SCSI_PROVISIONING_TYPE_DEALLOCATED ||
+        lbasd->provisioning == SCSI_PROVISIONING_TYPE_ANCHORED) {
+        ret &= ~BDRV_BLOCK_DATA;
+        if (iscsilun->lbprz) {
+            ret |= BDRV_BLOCK_ZERO;
+        }
+    }
+
+    if (ret & BDRV_BLOCK_ZERO) {
+        iscsi_allocationmap_clear(iscsilun, sector_num, *pnum);
+    } else {
+        iscsi_allocationmap_set(iscsilun, sector_num, *pnum);
+    }
+
+    if (*pnum > nb_sectors) {
+        *pnum = nb_sectors;
+    }
+out:
+    if (iTask.task != NULL) {
+        scsi_free_scsi_task(iTask.task);
+    }
+    return ret;
+}
+
+#endif /* LIBISCSI_FEATURE_IOVECTOR */
+
+
 static int coroutine_fn iscsi_co_readv(BlockDriverState *bs,
                                       int64_t sector_num, int nb_sectors,
                                       QEMUIOVector *iov)
@@ -355,6 +512,22 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs,
        return -EINVAL;
    }

+#if defined(LIBISCSI_FEATURE_IOVECTOR)
+    if (iscsilun->lbprz && nb_sectors >= ISCSI_CHECKALLOC_THRES &&
+        !iscsi_allocationmap_is_allocated(iscsilun, sector_num, nb_sectors)) {
+        int64_t ret;
+        int pnum;
+        ret = iscsi_co_get_block_status(bs, sector_num, INT_MAX, &pnum);
+        if (ret < 0) {
+            return ret;
+        }
+        if (ret & BDRV_BLOCK_ZERO && pnum >= nb_sectors) {
+            qemu_iovec_memset(iov, 0, 0x00, iov->size);
+            return 0;
+        }
+    }
+#endif
+
    lba = sector_qemu2lun(sector_num, iscsilun);
    num_sectors = sector_qemu2lun(nb_sectors, iscsilun);

@@ -643,101 +816,6 @@ iscsi_getlength(BlockDriverState *bs)
    return len;
 }

-#if defined(LIBISCSI_FEATURE_IOVECTOR)
-
-static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
-                                                  int64_t sector_num,
-                                                  int nb_sectors, int *pnum)
-{
-    IscsiLun *iscsilun = bs->opaque;
-    struct scsi_get_lba_status *lbas = NULL;
-    struct scsi_lba_status_descriptor *lbasd = NULL;
-    struct IscsiTask iTask;
-    int64_t ret;
-
-    iscsi_co_init_iscsitask(iscsilun, &iTask);
-
-    if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
-        ret = -EINVAL;
-        goto out;
-    }
-
-    /* default to all sectors allocated */
-    ret = BDRV_BLOCK_DATA;
-    ret |= (sector_num << BDRV_SECTOR_BITS) | BDRV_BLOCK_OFFSET_VALID;
-    *pnum = nb_sectors;
-
-    /* LUN does not support logical block provisioning */
-    if (iscsilun->lbpme == 0) {
-        goto out;
-    }
-
-retry:
-    if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun,
-                                  sector_qemu2lun(sector_num, iscsilun),
-                                  8 + 16, iscsi_co_generic_cb,
-                                  &iTask) == NULL) {
-        ret = -ENOMEM;
-        goto out;
-    }
-
-    while (!iTask.complete) {
-        iscsi_set_events(iscsilun);
-        qemu_coroutine_yield();
-    }
-
-    if (iTask.do_retry) {
-        if (iTask.task != NULL) {
-            scsi_free_scsi_task(iTask.task);
-            iTask.task = NULL;
-        }
-        iTask.complete = 0;
-        goto retry;
-    }
-
-    if (iTask.status != SCSI_STATUS_GOOD) {
-        /* in case the get_lba_status_callout fails (i.e.
-         * because the device is busy or the cmd is not
-         * supported) we pretend all blocks are allocated
-         * for backwards compatibility */
-        goto out;
-    }
-
-    lbas = scsi_datain_unmarshall(iTask.task);
-    if (lbas == NULL) {
-        ret = -EIO;
-        goto out;
-    }
-
-    lbasd = &lbas->descriptors[0];
-
-    if (sector_qemu2lun(sector_num, iscsilun) != lbasd->lba) {
-        ret = -EIO;
-        goto out;
-    }
-
-    *pnum = sector_lun2qemu(lbasd->num_blocks, iscsilun);
-    if (*pnum > nb_sectors) {
-        *pnum = nb_sectors;
-    }
-
-    if (lbasd->provisioning == SCSI_PROVISIONING_TYPE_DEALLOCATED ||
-        lbasd->provisioning == SCSI_PROVISIONING_TYPE_ANCHORED) {
-        ret &= ~BDRV_BLOCK_DATA;
-        if (iscsilun->lbprz) {
-            ret |= BDRV_BLOCK_ZERO;
-        }
-    }
-
-out:
-    if (iTask.task != NULL) {
-        scsi_free_scsi_task(iTask.task);
-    }
-    return ret;
-}
-
-#endif /* LIBISCSI_FEATURE_IOVECTOR */
-
 static int
 coroutine_fn iscsi_co_discard(BlockDriverState *bs, int64_t sector_num,
                                   int nb_sectors)
@@ -791,6 +869,8 @@ retry:
        return -EIO;
    }

+    iscsi_allocationmap_clear(iscsilun, sector_num, nb_sectors);
+
    return 0;
 }

@@ -809,13 +889,14 @@ coroutine_fn iscsi_co_write_zeroes(BlockDriverState *bs, int64_t sector_num,
        return -EINVAL;
    }

-    if (!(flags & BDRV_REQ_MAY_UNMAP) && !iscsilun->has_write_same) {
-        /* WRITE SAME without UNMAP is not supported by the target */
-        return -ENOTSUP;
+    if ((flags & BDRV_REQ_MAY_UNMAP) && !iscsilun->lbp.lbpws) {
+        /* WRITE SAME with UNMAP is not supported by the target,
+         * fall back and try WRITE SAME without UNMAP */
+        flags &= ~BDRV_REQ_MAY_UNMAP;
    }

-    if ((flags & BDRV_REQ_MAY_UNMAP) && !iscsilun->lbp.lbpws) {
-        /* WRITE SAME with UNMAP is not supported by the target */
+    if (!(flags & BDRV_REQ_MAY_UNMAP) && !iscsilun->has_write_same) {
+        /* WRITE SAME without UNMAP is not supported by the target */
        return -ENOTSUP;
    }

@@ -864,6 +945,12 @@ retry:
        return -EIO;
    }

+    if (flags & BDRV_REQ_MAY_UNMAP) {
+        iscsi_allocationmap_clear(iscsilun, sector_num, nb_sectors);
+    } else {
+        iscsi_allocationmap_set(iscsilun, sector_num, nb_sectors);
+    }
+
    return 0;
 }

@@ -1295,6 +1382,22 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
    timer_mod(iscsilun->nop_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + NOP_INTERVAL);
 #endif

+    /* Guess the internal cluster (page) size of the iscsi target by the means
+     * of opt_unmap_gran. Transfer the unmap granularity only if it has a
+     * reasonable size */
+    if (iscsilun->bl.opt_unmap_gran * iscsilun->block_size >= 4 * 1024 &&
+        iscsilun->bl.opt_unmap_gran * iscsilun->block_size <= 16 * 1024 * 1024) {
+        iscsilun->cluster_sectors = (iscsilun->bl.opt_unmap_gran *
+                                     iscsilun->block_size) >> BDRV_SECTOR_BITS;
+#if defined(LIBISCSI_FEATURE_IOVECTOR)
+        if (iscsilun->lbprz && !(bs->open_flags & BDRV_O_NOCACHE)) {
+            iscsilun->allocationmap =
+                bitmap_new(DIV_ROUND_UP(bs->total_sectors,
+                                        iscsilun->cluster_sectors));
+        }
+#endif
+    }
+
 out:
    qemu_opts_del(opts);
    if (initiator_name != NULL) {
@@ -1328,6 +1431,7 @@ static void iscsi_close(BlockDriverState *bs)
    qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), NULL, NULL, NULL);
    iscsi_destroy_context(iscsi);
    g_free(iscsilun->zeroblock);
+    g_free(iscsilun->allocationmap);
    memset(iscsilun, 0, sizeof(IscsiLun));
 }

@@ -1388,6 +1492,13 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t offset)
        return -EINVAL;
    }

+    if (iscsilun->allocationmap != NULL) {
+        g_free(iscsilun->allocationmap);
+        iscsilun->allocationmap =
+            bitmap_new(DIV_ROUND_UP(bs->total_sectors,
+                                    iscsilun->cluster_sectors));
+    }
+
    return 0;
 }

@@ -1450,13 +1561,7 @@ static int iscsi_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
    IscsiLun *iscsilun = bs->opaque;
    bdi->unallocated_blocks_are_zero = !!iscsilun->lbprz;
    bdi->can_write_zeroes_with_unmap = iscsilun->lbprz && iscsilun->lbp.lbpws;
-    /* Guess the internal cluster (page) size of the iscsi target by the means
-     * of opt_unmap_gran. Transfer the unmap granularity only if it has a
-     * reasonable size for bdi->cluster_size */
-    if (iscsilun->bl.opt_unmap_gran * iscsilun->block_size >= 64 * 1024 &&
-        iscsilun->bl.opt_unmap_gran * iscsilun->block_size <= 16 * 1024 * 1024) {
-        bdi->cluster_size = iscsilun->bl.opt_unmap_gran * iscsilun->block_size;
-    }
+    bdi->cluster_size = iscsilun->cluster_sectors * BDRV_SECTOR_SIZE;
    return 0;
 }

--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -48,7 +48,7 @@ The QAPI schema definitions can be modularized using the 'include' directive:
 { 'include': 'path/to/file.json'}

 The directive is evaluated recursively, and include paths are relative to the
-file using the directive.
+file using the directive. Multiple includes of the same file are safe.


 === Complex types ===
@@ -230,14 +230,13 @@ node structure that can be used to chain together a list of such types in
 case we want to accept/return a list of this type with a command), and a
 command which takes that type as a parameter and returns the same type:

-    mdroth@illuin:~/w/qemu2.git$ cat example-schema.json
+    $ cat example-schema.json
    { 'type': 'UserDefOne',
      'data': { 'integer': 'int', 'string': 'str' } }

    { 'command': 'my-command',
      'data':    {'arg1': 'UserDefOne'},
      'returns': 'UserDefOne' }
-    mdroth@illuin:~/w/qemu2.git$

 === scripts/qapi-types.py ===

@@ -255,14 +254,25 @@ created code.

 Example:

-    mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-types.py \
-      --output-dir="qapi-generated" --prefix="example-" --input-file=example-schema.json
-    mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-types.c
-    /* AUTOMATICALLY GENERATED, DO NOT MODIFY */
+    $ python scripts/qapi-types.py --output-dir="qapi-generated" \
+    --prefix="example-" --input-file=example-schema.json
+    $ cat qapi-generated/example-qapi-types.c
+[Uninteresting stuff omitted...]

-    #include "qapi/qapi-dealloc-visitor.h"
-    #include "example-qapi-types.h"
-    #include "example-qapi-visit.h"
+    void qapi_free_UserDefOneList(UserDefOneList * obj)
+    {
+        QapiDeallocVisitor *md;
+        Visitor *v;
+
+        if (!obj) {
+            return;
+        }
+
+        md = qapi_dealloc_visitor_new();
+        v = qapi_dealloc_get_visitor(md);
+        visit_type_UserDefOneList(v, &obj, NULL, NULL);
+        qapi_dealloc_visitor_cleanup(md);
+    }

    void qapi_free_UserDefOne(UserDefOne * obj)
    {
@@ -279,32 +289,38 @@ Example:
        qapi_dealloc_visitor_cleanup(md);
    }

-    mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-types.h
-    /* AUTOMATICALLY GENERATED, DO NOT MODIFY */
-    #ifndef QAPI_GENERATED_EXAMPLE_QAPI_TYPES
-    #define QAPI_GENERATED_EXAMPLE_QAPI_TYPES
+    $ cat qapi-generated/example-qapi-types.h
+[Uninteresting stuff omitted...]

-    #include "qapi/qapi-types-core.h"
+    #ifndef EXAMPLE_QAPI_TYPES_H
+    #define EXAMPLE_QAPI_TYPES_H
+
+[Builtin types omitted...]

    typedef struct UserDefOne UserDefOne;

    typedef struct UserDefOneList
    {
-        UserDefOne *value;
+        union {
+            UserDefOne *value;
+            uint64_t padding;
+        };
        struct UserDefOneList *next;
    } UserDefOneList;

+[Functions on builtin types omitted...]
+
    struct UserDefOne
    {
        int64_t integer;
        char * string;
    };

+    void qapi_free_UserDefOneList(UserDefOneList * obj);
    void qapi_free_UserDefOne(UserDefOne * obj);

    #endif

-
 === scripts/qapi-visit.py ===

 Used to generate the visitor functions used to walk through and convert
@@ -325,51 +341,78 @@ $(prefix)qapi-visit.h: declarations for previously mentioned visitor

 Example:

-    mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-visit.py \
-        --output-dir="qapi-generated" --prefix="example-" --input-file=example-schema.json
-    mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-visit.c
-    /* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+    $ python scripts/qapi-visit.py --output-dir="qapi-generated"
+    --prefix="example-" --input-file=example-schema.json
+    $ cat qapi-generated/example-qapi-visit.c
+[Uninteresting stuff omitted...]

-    #include "example-qapi-visit.h"
+    static void visit_type_UserDefOne_fields(Visitor *m, UserDefOne ** obj, Error **errp)
+    {
+        Error *err = NULL;
+        visit_type_int(m, &(*obj)->integer, "integer", &err);
+        if (err) {
+            goto out;
+        }
+        visit_type_str(m, &(*obj)->string, "string", &err);
+        if (err) {
+            goto out;
+        }
+
+    out:
+        error_propagate(errp, err);
+    }

    void visit_type_UserDefOne(Visitor *m, UserDefOne ** obj, const char *name, Error **errp)
    {
-        visit_start_struct(m, (void **)obj, "UserDefOne", name, sizeof(UserDefOne), errp);
-        visit_type_int(m, (obj && *obj) ? &(*obj)->integer : NULL, "integer", errp);
-        visit_type_str(m, (obj && *obj) ? &(*obj)->string : NULL, "string", errp);
-        visit_end_struct(m, errp);
+        Error *err = NULL;
+
+        visit_start_struct(m, (void **)obj, "UserDefOne", name, sizeof(UserDefOne), &err);
+        if (!err) {
+            if (*obj) {
+                visit_type_UserDefOne_fields(m, obj, errp);
+            }
+            visit_end_struct(m, &err);
+        }
+        error_propagate(errp, err);
    }

    void visit_type_UserDefOneList(Visitor *m, UserDefOneList ** obj, const char *name, Error **errp)
    {
-        GenericList *i, **prev = (GenericList **)obj;
+        Error *err = NULL;
+        GenericList *i, **prev;

-        visit_start_list(m, name, errp);
-
-        for (; (i = visit_next_list(m, prev, errp)) != NULL; prev = &i) {
-            UserDefOneList *native_i = (UserDefOneList *)i;
-            visit_type_UserDefOne(m, &native_i->value, NULL, errp);
+        visit_start_list(m, name, &err);
+        if (err) {
+            goto out;
        }

-        visit_end_list(m, errp);
+        for (prev = (GenericList **)obj;
+             !err && (i = visit_next_list(m, prev, &err)) != NULL;
+             prev = &i) {
+            UserDefOneList *native_i = (UserDefOneList *)i;
+            visit_type_UserDefOne(m, &native_i->value, NULL, &err);
+        }
+
+        error_propagate(errp, err);
+        err = NULL;
+        visit_end_list(m, &err);
+    out:
+        error_propagate(errp, err);
    }
-    mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-visit.h
-    /* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+    $ python scripts/qapi-commands.py --output-dir="qapi-generated" \
+    --prefix="example-" --input-file=example-schema.json
+    $ cat qapi-generated/example-qapi-visit.h
+[Uninteresting stuff omitted...]

-    #ifndef QAPI_GENERATED_EXAMPLE_QAPI_VISIT
-    #define QAPI_GENERATED_EXAMPLE_QAPI_VISIT
+    #ifndef EXAMPLE_QAPI_VISIT_H
+    #define EXAMPLE_QAPI_VISIT_H

-    #include "qapi/qapi-visit-core.h"
-    #include "example-qapi-types.h"
+[Visitors for builtin types omitted...]

    void visit_type_UserDefOne(Visitor *m, UserDefOne ** obj, const char *name, Error **errp);
    void visit_type_UserDefOneList(Visitor *m, UserDefOneList ** obj, const char *name, Error **errp);

    #endif
-    mdroth@illuin:~/w/qemu2.git$
-
-(The actual structure of the visit_type_* functions is a bit more complex
-in order to propagate errors correctly and avoid leaking memory).

 === scripts/qapi-commands.py ===

@@ -390,77 +433,80 @@ $(prefix)qmp-commands.h: Function prototypes for the QMP commands

 Example:

-    mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qmp-marshal.c
-    /* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+    $ cat qapi-generated/example-qmp-marshal.c
+[Uninteresting stuff omitted...]

-    #include "qemu-objects.h"
-    #include "qapi/qmp-core.h"
-    #include "qapi/qapi-visit-core.h"
-    #include "qapi/qmp-output-visitor.h"
-    #include "qapi/qmp-input-visitor.h"
-    #include "qapi/qapi-dealloc-visitor.h"
-    #include "example-qapi-types.h"
-    #include "example-qapi-visit.h"
-
-    #include "example-qmp-commands.h"
    static void qmp_marshal_output_my_command(UserDefOne * ret_in, QObject **ret_out, Error **errp)
    {
-        QapiDeallocVisitor *md = qapi_dealloc_visitor_new();
+        Error *local_err = NULL;
        QmpOutputVisitor *mo = qmp_output_visitor_new();
+        QapiDeallocVisitor *md;
        Visitor *v;

        v = qmp_output_get_visitor(mo);
-        visit_type_UserDefOne(v, &ret_in, "unused", errp);
-        v = qapi_dealloc_get_visitor(md);
-        visit_type_UserDefOne(v, &ret_in, "unused", errp);
-        qapi_dealloc_visitor_cleanup(md);
-
-
+        visit_type_UserDefOne(v, &ret_in, "unused", &local_err);
+        if (local_err) {
+            goto out;
+        }
        *ret_out = qmp_output_get_qobject(mo);
+
+    out:
+        error_propagate(errp, local_err);
+        qmp_output_visitor_cleanup(mo);
+        md = qapi_dealloc_visitor_new();
+        v = qapi_dealloc_get_visitor(md);
+        visit_type_UserDefOne(v, &ret_in, "unused", NULL);
+        qapi_dealloc_visitor_cleanup(md);
    }

-    static void qmp_marshal_input_my_command(QmpState *qmp__sess, QDict *args, QObject **ret, Error **errp)
+    static void qmp_marshal_input_my_command(QDict *args, QObject **ret, Error **errp)
    {
+        Error *local_err = NULL;
        UserDefOne * retval = NULL;
-        QmpInputVisitor *mi;
+        QmpInputVisitor *mi = qmp_input_visitor_new_strict(QOBJECT(args));
        QapiDeallocVisitor *md;
        Visitor *v;
        UserDefOne * arg1 = NULL;

-        mi = qmp_input_visitor_new(QOBJECT(args));
        v = qmp_input_get_visitor(mi);
-        visit_type_UserDefOne(v, &arg1, "arg1", errp);
-
-        if (error_is_set(errp)) {
+        visit_type_UserDefOne(v, &arg1, "arg1", &local_err);
+        if (local_err) {
            goto out;
        }
-        retval = qmp_my_command(arg1, errp);
-        qmp_marshal_output_my_command(retval, ret, errp);
+
+        retval = qmp_my_command(arg1, &local_err);
+        if (local_err) {
+            goto out;
+        }
+
+        qmp_marshal_output_my_command(retval, ret, &local_err);

    out:
+        error_propagate(errp, local_err);
+        qmp_input_visitor_cleanup(mi);
        md = qapi_dealloc_visitor_new();
        v = qapi_dealloc_get_visitor(md);
-        visit_type_UserDefOne(v, &arg1, "arg1", errp);
+        visit_type_UserDefOne(v, &arg1, "arg1", NULL);
        qapi_dealloc_visitor_cleanup(md);
        return;
    }

    static void qmp_init_marshal(void)
    {
-        qmp_register_command("my-command", qmp_marshal_input_my_command);
+        qmp_register_command("my-command", qmp_marshal_input_my_command, QCO_NO_OPTIONS);
    }

    qapi_init(qmp_init_marshal);
-    mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qmp-commands.h
-    /* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+    $ cat qapi-generated/example-qmp-commands.h
+[Uninteresting stuff omitted...]

-    #ifndef QAPI_GENERATED_EXAMPLE_QMP_COMMANDS
-    #define QAPI_GENERATED_EXAMPLE_QMP_COMMANDS
+    #ifndef EXAMPLE_QMP_COMMANDS_H
+    #define EXAMPLE_QMP_COMMANDS_H

    #include "example-qapi-types.h"
-    #include "error.h"
+    #include "qapi/qmp/qdict.h"
+    #include "qapi/error.h"

    UserDefOne * qmp_my_command(UserDefOne * arg1, Error **errp);

    #endif
-    mdroth@illuin:~/w/qemu2.git$
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -556,6 +556,7 @@ ETEXI
        .params     = "keys [hold_ms]",
        .help       = "send keys to the VM (e.g. 'sendkey ctrl-alt-f1', default hold time=100 ms)",
        .mhandler.cmd = hmp_send_key,
+        .command_completion = sendkey_completion,
    },

 STEXI
@@ -1233,9 +1234,10 @@ ETEXI
    {
        .name       = "netdev_add",
        .args_type  = "netdev:O",
-        .params     = "[user|tap|socket|hubport|netmap],id=str[,prop=value][,...]",
+        .params     = "[user|tap|socket|vde|bridge|hubport|netmap],id=str[,prop=value][,...]",
        .help       = "add host network device",
        .mhandler.cmd = hmp_netdev_add,
+        .command_completion = netdev_add_completion,
    },

 STEXI
@@ -1250,6 +1252,7 @@ ETEXI
        .params     = "id",
        .help       = "remove host network device",
        .mhandler.cmd = hmp_netdev_del,
+        .command_completion = netdev_del_completion,
    },

 STEXI
@@ -1339,6 +1342,7 @@ ETEXI
        .params     = "name on|off",
        .help       = "change the link status of a network adapter",
        .mhandler.cmd = hmp_set_link,
+        .command_completion = set_link_completion,
    },

 STEXI
@@ -1622,6 +1626,7 @@ ETEXI
        .params     = "args",
        .help       = "add chardev",
        .mhandler.cmd = hmp_chardev_add,
+        .command_completion = chardev_add_completion,
    },

 STEXI
@@ -1638,6 +1643,7 @@ ETEXI
        .params     = "id",
        .help       = "remove chardev",
        .mhandler.cmd = hmp_chardev_remove,
+        .command_completion = chardev_remove_completion,
    },

 STEXI
--- a/hmp.c
+++ b/hmp.c
@@ -1388,6 +1388,7 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
 void hmp_object_add(Monitor *mon, const QDict *qdict)
 {
    Error *err = NULL;
+    Error *err_end = NULL;
    QemuOpts *opts;
    char *type = NULL;
    char *id = NULL;
@@ -1411,24 +1412,23 @@ void hmp_object_add(Monitor *mon, const QDict *qdict)
    qdict_del(pdict, "qom-type");
    visit_type_str(opts_get_visitor(ov), &type, "qom-type", &err);
    if (err) {
-        goto out_clean;
+        goto out_end;
    }

    qdict_del(pdict, "id");
    visit_type_str(opts_get_visitor(ov), &id, "id", &err);
    if (err) {
-        goto out_clean;
+        goto out_end;
    }

    object_add(type, id, pdict, opts_get_visitor(ov), &err);
-    if (err) {
-        goto out_clean;
-    }
-    visit_end_struct(opts_get_visitor(ov), &err);
-    if (err) {
+
+out_end:
+    visit_end_struct(opts_get_visitor(ov), &err_end);
+    if (!err && err_end) {
        qmp_object_del(id, NULL);
    }
-
+    error_propagate(&err, err_end);
 out_clean:
    opts_visitor_cleanup(ov);

--- a/hmp.h
+++ b/hmp.h
@@ -97,5 +97,11 @@ void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void device_del_completion(ReadLineState *rs, int nb_args, const char *str);
+void sendkey_completion(ReadLineState *rs, int nb_args, const char *str);
+void chardev_remove_completion(ReadLineState *rs, int nb_args, const char *str);
+void chardev_add_completion(ReadLineState *rs, int nb_args, const char *str);
+void set_link_completion(ReadLineState *rs, int nb_args, const char *str);
+void netdev_add_completion(ReadLineState *rs, int nb_args, const char *str);
+void netdev_del_completion(ReadLineState *rs, int nb_args, const char *str);

 #endif
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -21,6 +21,7 @@
 #include "hw/hw.h"
 #include "hw/pci/pci.h"
 #include "sysemu/dma.h"
+#include "hw/pci/msi.h"
 #include "hw/pci/msix.h"
 #include "qemu/iov.h"
 #include "hw/scsi/scsi.h"
@@ -43,9 +44,11 @@

 #define MEGASAS_FLAG_USE_JBOD      0
 #define MEGASAS_MASK_USE_JBOD      (1 << MEGASAS_FLAG_USE_JBOD)
-#define MEGASAS_FLAG_USE_MSIX      1
+#define MEGASAS_FLAG_USE_MSI       1
+#define MEGASAS_MASK_USE_MSI       (1 << MEGASAS_FLAG_USE_MSI)
+#define MEGASAS_FLAG_USE_MSIX      2
 #define MEGASAS_MASK_USE_MSIX      (1 << MEGASAS_FLAG_USE_MSIX)
-#define MEGASAS_FLAG_USE_QUEUE64   2
+#define MEGASAS_FLAG_USE_QUEUE64   3
 #define MEGASAS_MASK_USE_QUEUE64   (1 << MEGASAS_FLAG_USE_QUEUE64)

 static const char *mfi_frame_desc[] = {
@@ -132,6 +135,11 @@ static bool megasas_use_queue64(MegasasState *s)
    return s->flags & MEGASAS_MASK_USE_QUEUE64;
 }

+static bool megasas_use_msi(MegasasState *s)
+{
+    return s->flags & MEGASAS_MASK_USE_MSI;
+}
+
 static bool megasas_use_msix(MegasasState *s)
 {
    return s->flags & MEGASAS_MASK_USE_MSIX;
@@ -538,6 +546,9 @@ static void megasas_complete_frame(MegasasState *s, uint64_t context)
            if (msix_enabled(pci_dev)) {
                trace_megasas_msix_raise(0);
                msix_notify(pci_dev, 0);
+            } else if (msi_enabled(pci_dev)) {
+                trace_megasas_msi_raise(0);
+                msi_notify(pci_dev, 0);
            } else {
                trace_megasas_irq_raise();
                pci_irq_assert(pci_dev);
@@ -1106,6 +1117,21 @@ static int megasas_dcmd_ld_get_list(MegasasState *s, MegasasCmd *cmd)
    return MFI_STAT_OK;
 }

+static int megasas_dcmd_ld_list_query(MegasasState *s, MegasasCmd *cmd)
+{
+    uint16_t flags;
+
+    /* mbox0 contains flags */
+    flags = le16_to_cpu(cmd->frame->dcmd.mbox[0]);
+    trace_megasas_dcmd_ld_list_query(cmd->index, flags);
+    if (flags == MR_LD_QUERY_TYPE_ALL ||
+        flags == MR_LD_QUERY_TYPE_EXPOSED_TO_HOST) {
+        return megasas_dcmd_ld_get_list(s, cmd);
+    }
+
+    return MFI_STAT_OK;
+}
+
 static int megasas_ld_get_info_submit(SCSIDevice *sdev, int lun,
                                      MegasasCmd *cmd)
 {
@@ -1409,6 +1435,8 @@ static const struct dcmd_cmd_tbl_t {
      megasas_dcmd_dummy },
    { MFI_DCMD_LD_GET_LIST, "LD_GET_LIST",
      megasas_dcmd_ld_get_list},
+    { MFI_DCMD_LD_LIST_QUERY, "LD_LIST_QUERY",
+      megasas_dcmd_ld_list_query },
    { MFI_DCMD_LD_GET_INFO, "LD_GET_INFO",
      megasas_dcmd_ld_get_info },
    { MFI_DCMD_LD_GET_PROP, "LD_GET_PROP",
@@ -1939,12 +1967,20 @@ static void megasas_mmio_write(void *opaque, hwaddr addr,
        break;
    case MFI_OMSK:
        s->intr_mask = val;
-        if (!megasas_intr_enabled(s) && !msix_enabled(pci_dev)) {
+        if (!megasas_intr_enabled(s) &&
+            !msi_enabled(pci_dev) &&
+            !msix_enabled(pci_dev)) {
            trace_megasas_irq_lower();
            pci_irq_deassert(pci_dev);
        }
        if (megasas_intr_enabled(s)) {
-            trace_megasas_intr_enabled();
+            if (msix_enabled(pci_dev)) {
+                trace_megasas_msix_enabled(0);
+            } else if (msi_enabled(pci_dev)) {
+                trace_megasas_msi_enabled(0);
+            } else {
+                trace_megasas_intr_enabled();
+            }
        } else {
            trace_megasas_intr_disabled();
        }
@@ -2068,6 +2104,7 @@ static const VMStateDescription vmstate_megasas = {
    .minimum_version_id_old = 0,
    .fields      = (VMStateField[]) {
        VMSTATE_PCI_DEVICE(parent_obj, MegasasState),
+        VMSTATE_MSIX(parent_obj, MegasasState),

        VMSTATE_INT32(fw_state, MegasasState),
        VMSTATE_INT32(intr_mask, MegasasState),
@@ -2083,9 +2120,12 @@ static void megasas_scsi_uninit(PCIDevice *d)
 {
    MegasasState *s = MEGASAS(d);

-#ifdef USE_MSIX
-    msix_uninit(d, &s->mmio_io);
-#endif
+    if (megasas_use_msix(s)) {
+        msix_uninit(d, &s->mmio_io, &s->mmio_io);
+    }
+    if (megasas_use_msi(s)) {
+        msi_uninit(d);
+    }
    memory_region_destroy(&s->mmio_io);
    memory_region_destroy(&s->port_io);
    memory_region_destroy(&s->queue_io);
@@ -2124,15 +2164,15 @@ static int megasas_scsi_init(PCIDevice *dev)
    memory_region_init_io(&s->queue_io, OBJECT(s), &megasas_queue_ops, s,
                          "megasas-queue", 0x40000);

-#ifdef USE_MSIX
-    /* MSI-X support is currently broken */
+    if (megasas_use_msi(s) &&
+        msi_init(dev, 0x50, 1, true, false)) {
+        s->flags &= ~MEGASAS_MASK_USE_MSI;
+    }
    if (megasas_use_msix(s) &&
-        msix_init(dev, 15, &s->mmio_io, 0, 0x2000)) {
+        msix_init(dev, 15, &s->mmio_io, 0, 0x2000,
+                  &s->mmio_io, 0, 0x3800, 0x68)) {
        s->flags &= ~MEGASAS_MASK_USE_MSIX;
    }
-#else
-    s->flags &= ~MEGASAS_MASK_USE_MSIX;
-#endif

    bar_type = PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64;
    pci_register_bar(dev, 0, bar_type, &s->mmio_io);
@@ -2151,7 +2191,7 @@ static int megasas_scsi_init(PCIDevice *dev)
        s->sas_addr |= PCI_FUNC(dev->devfn);
    }
    if (!s->hba_serial) {
-	s->hba_serial = g_strdup(MEGASAS_HBA_SERIAL);
+        s->hba_serial = g_strdup(MEGASAS_HBA_SERIAL);
    }
    if (s->fw_sge >= MEGASAS_MAX_SGE - MFI_PASS_FRAME_SIZE) {
        s->fw_sge = MEGASAS_MAX_SGE - MFI_PASS_FRAME_SIZE;
@@ -2164,7 +2204,6 @@ static int megasas_scsi_init(PCIDevice *dev)
        s->fw_cmds = MEGASAS_MAX_FRAMES;
    }
    trace_megasas_init(s->fw_sge, s->fw_cmds,
-                       megasas_use_msix(s) ? "MSI-X" : "INTx",
                       megasas_is_jbod(s) ? "jbod" : "raid");
    s->fw_luns = (MFI_MAX_LD > MAX_SCSI_DEVS) ?
        MAX_SCSI_DEVS : MFI_MAX_LD;
@@ -2189,6 +2228,13 @@ static int megasas_scsi_init(PCIDevice *dev)
    return 0;
 }

+static void
+megasas_write_config(PCIDevice *pci, uint32_t addr, uint32_t val, int len)
+{
+    pci_default_write_config(pci, addr, val, len);
+    msi_write_config(pci, addr, val, len);
+}
+
 static Property megasas_properties[] = {
    DEFINE_PROP_UINT32("max_sge", MegasasState, fw_sge,
                       MEGASAS_DEFAULT_SGE),
@@ -2196,10 +2242,10 @@ static Property megasas_properties[] = {
                       MEGASAS_DEFAULT_FRAMES),
    DEFINE_PROP_STRING("hba_serial", MegasasState, hba_serial),
    DEFINE_PROP_UINT64("sas_address", MegasasState, sas_addr, 0),
-#ifdef USE_MSIX
+    DEFINE_PROP_BIT("use_msi", MegasasState, flags,
+                    MEGASAS_FLAG_USE_MSI, false),
    DEFINE_PROP_BIT("use_msix", MegasasState, flags,
                    MEGASAS_FLAG_USE_MSIX, false),
-#endif
    DEFINE_PROP_BIT("use_jbod", MegasasState, flags,
                    MEGASAS_FLAG_USE_JBOD, false),
    DEFINE_PROP_END_OF_LIST(),
@@ -2222,6 +2268,7 @@ static void megasas_class_init(ObjectClass *oc, void *data)
    dc->vmsd = &vmstate_megasas;
    set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
    dc->desc = "LSI MegaRAID SAS 1078";
+    pc->config_write = megasas_write_config;
 }

 static const TypeInfo megasas_info = {
--- a/hw/scsi/mfi.h
+++ b/hw/scsi/mfi.h
@@ -164,6 +164,7 @@ typedef enum {
    MFI_DCMD_PD_BLINK =                 0x02070100,
    MFI_DCMD_PD_UNBLINK =               0x02070200,
    MFI_DCMD_LD_GET_LIST =              0x03010000,
+    MFI_DCMD_LD_LIST_QUERY =            0x03010100,
    MFI_DCMD_LD_GET_INFO =              0x03020000,
    MFI_DCMD_LD_GET_PROP =              0x03030000,
    MFI_DCMD_LD_SET_PROP =              0x03040000,
@@ -411,6 +412,14 @@ typedef enum {
    MR_PD_QUERY_TYPE_EXPOSED_TO_HOST =  5, /*query for system drives */
 } mfi_pd_query_type;

+typedef enum {
+    MR_LD_QUERY_TYPE_ALL =              0,
+    MR_LD_QUERY_TYPE_EXPOSED_TO_HOST =  1,
+    MR_LD_QUERY_TYPE_USED_TGT_IDS =     2,
+    MR_LD_QUERY_TYPE_CLUSTER_ACCESS =   3,
+    MR_LD_QUERY_TYPE_CLUSTER_LOCALE =   4,
+} mfi_ld_query_type;
+
 /*
 * Other propertities and definitions
 */
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2458,21 +2458,27 @@ static int scsi_block_initfn(SCSIDevice *dev)
    int rc;

    if (!s->qdev.conf.bs) {
-        error_report("scsi-block: drive property not set");
+        error_report("drive property not set");
        return -1;
    }

    /* check we are using a driver managing SG_IO (version 3 and after) */
-    if (bdrv_ioctl(s->qdev.conf.bs, SG_GET_VERSION_NUM, &sg_version) < 0 ||
-        sg_version < 30000) {
-        error_report("scsi-block: scsi generic interface too old");
+    rc = bdrv_ioctl(s->qdev.conf.bs, SG_GET_VERSION_NUM, &sg_version);
+    if (rc < 0) {
+        error_report("cannot get SG_IO version number: %s.  "
+                     "Is this a SCSI device?",
+                     strerror(-rc));
+        return -1;
+    }
+    if (sg_version < 30000) {
+        error_report("scsi generic interface too old");
        return -1;
    }

    /* get device type from INQUIRY data */
    rc = get_device_type(s);
    if (rc < 0) {
-        error_report("scsi-block: INQUIRY failed");
+        error_report("INQUIRY failed");
        return -1;
    }

--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -394,6 +394,7 @@ static void scsi_destroy(SCSIDevice *s)

 static int scsi_generic_initfn(SCSIDevice *s)
 {
+    int rc;
    int sg_version;
    struct sg_scsi_id scsiid;

@@ -412,8 +413,11 @@ static int scsi_generic_initfn(SCSIDevice *s)
    }

    /* check we are using a driver managing SG_IO (version 3 and after */
-    if (bdrv_ioctl(s->conf.bs, SG_GET_VERSION_NUM, &sg_version) < 0) {
-        error_report("scsi generic interface not supported");
+    rc = bdrv_ioctl(s->conf.bs, SG_GET_VERSION_NUM, &sg_version);
+    if (rc < 0) {
+        error_report("cannot get SG_IO version number: %s.  "
+                     "Is this a SCSI device?",
+                     strerror(-rc));
        return -1;
    }
    if (sg_version < 30000) {
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -793,19 +793,46 @@ static const MemoryRegionOps cmos_ops = {
 static void rtc_get_date(Object *obj, Visitor *v, void *opaque,
                         const char *name, Error **errp)
 {
+    Error *err = NULL;
    RTCState *s = MC146818_RTC(obj);
    struct tm current_tm;

    rtc_update_time(s);
    rtc_get_time(s, &current_tm);
-    visit_start_struct(v, NULL, "struct tm", name, 0, errp);
-    visit_type_int32(v, &current_tm.tm_year, "tm_year", errp);
-    visit_type_int32(v, &current_tm.tm_mon, "tm_mon", errp);
-    visit_type_int32(v, &current_tm.tm_mday, "tm_mday", errp);
-    visit_type_int32(v, &current_tm.tm_hour, "tm_hour", errp);
-    visit_type_int32(v, &current_tm.tm_min, "tm_min", errp);
-    visit_type_int32(v, &current_tm.tm_sec, "tm_sec", errp);
+    visit_start_struct(v, NULL, "struct tm", name, 0, &err);
+    if (err) {
+        goto out;
+    }
+    visit_type_int32(v, &current_tm.tm_year, "tm_year", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_int32(v, &current_tm.tm_mon, "tm_mon", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_int32(v, &current_tm.tm_mday, "tm_mday", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_int32(v, &current_tm.tm_hour, "tm_hour", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_int32(v, &current_tm.tm_min, "tm_min", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_int32(v, &current_tm.tm_sec, "tm_sec", &err);
+    if (err) {
+        goto out_end;
+    }
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
    visit_end_struct(v, errp);
+out:
+    error_propagate(errp, err);
 }

 static void rtc_realizefn(DeviceState *dev, Error **errp)
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -108,6 +108,7 @@ static void balloon_stats_poll_cb(void *opaque)
 static void balloon_stats_get_all(Object *obj, struct Visitor *v,
                                  void *opaque, const char *name, Error **errp)
 {
+    Error *err = NULL;
    VirtIOBalloon *s = opaque;
    int i;

@@ -116,17 +117,33 @@ static void balloon_stats_get_all(Object *obj, struct Visitor *v,
        return;
    }

-    visit_start_struct(v, NULL, "guest-stats", name, 0, errp);
-    visit_type_int(v, &s->stats_last_update, "last-update", errp);
-
-    visit_start_struct(v, NULL, NULL, "stats", 0, errp);
-    for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
-        visit_type_int64(v, (int64_t *) &s->stats[i], balloon_stat_names[i],
-                         errp);
+    visit_start_struct(v, NULL, "guest-stats", name, 0, &err);
+    if (err) {
+        goto out;
+    }
+    visit_type_int(v, &s->stats_last_update, "last-update", &err);
+    if (err) {
+        goto out_end;
    }
-    visit_end_struct(v, errp);

-    visit_end_struct(v, errp);
+    visit_start_struct(v, NULL, NULL, "stats", 0, &err);
+    if (err) {
+        goto out_end;
+    }
+    for (i = 0; !err && i < VIRTIO_BALLOON_S_NR; i++) {
+        visit_type_int64(v, (int64_t *) &s->stats[i], balloon_stat_names[i],
+                         &err);
+    }
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_struct(v, &err);
+
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_struct(v, &err);
+out:
+    error_propagate(errp, err);
 }

 static void balloon_stats_get_poll_interval(Object *obj, struct Visitor *v,
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -67,12 +67,6 @@ void error_set_win32(Error **errp, int win32_err, ErrorClass err_class,
 */
 void error_setg_file_open(Error **errp, int os_errno, const char *filename);

-/**
- * Returns true if an indirect pointer to an error is pointing to a valid
- * error object.
- */
-bool error_is_set(Error **errp);
-
 /*
 * Get the error class of an error object.
 */
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -42,13 +42,9 @@ struct Visitor
                        Error **errp);

    /* May be NULL */
-    void (*start_optional)(Visitor *v, bool *present, const char *name,
-                           Error **errp);
-    void (*end_optional)(Visitor *v, Error **errp);
+    void (*optional)(Visitor *v, bool *present, const char *name,
+                     Error **errp);

-    void (*start_handle)(Visitor *v, void **obj, const char *kind,
-                         const char *name, Error **errp);
-    void (*end_handle)(Visitor *v, Error **errp);
    void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error **errp);
    void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error **errp);
    void (*type_uint32)(Visitor *v, uint32_t *obj, const char *name, Error **errp);
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -39,9 +39,8 @@ void visit_end_implicit_struct(Visitor *v, Error **errp);
 void visit_start_list(Visitor *v, const char *name, Error **errp);
 GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp);
 void visit_end_list(Visitor *v, Error **errp);
-void visit_start_optional(Visitor *v, bool *present, const char *name,
-                          Error **errp);
-void visit_end_optional(Visitor *v, Error **errp);
+void visit_optional(Visitor *v, bool *present, const char *name,
+                    Error **errp);
 void visit_get_next_type(Visitor *v, int *obj, const int *qtypes,
                         const char *name, Error **errp);
 void visit_type_enum(Visitor *v, int *obj, const char *strings[],
--- a/monitor.c
+++ b/monitor.c
@@ -4269,6 +4269,55 @@ static const char *next_arg_type(const char *typestr)
    return (p != NULL ? ++p : typestr);
 }

+static void add_completion_option(ReadLineState *rs, const char *str,
+                                  const char *option)
+{
+    if (!str || !option) {
+        return;
+    }
+    if (!strncmp(option, str, strlen(str))) {
+        readline_add_completion(rs, option);
+    }
+}
+
+void chardev_add_completion(ReadLineState *rs, int nb_args, const char *str)
+{
+    size_t len;
+    ChardevBackendInfoList *list, *start;
+
+    if (nb_args != 2) {
+        return;
+    }
+    len = strlen(str);
+    readline_set_completion_index(rs, len);
+
+    start = list = qmp_query_chardev_backends(NULL);
+    while (list) {
+        const char *chr_name = list->value->name;
+
+        if (!strncmp(chr_name, str, len)) {
+            readline_add_completion(rs, chr_name);
+        }
+        list = list->next;
+    }
+    qapi_free_ChardevBackendInfoList(start);
+}
+
+void netdev_add_completion(ReadLineState *rs, int nb_args, const char *str)
+{
+    size_t len;
+    int i;
+
+    if (nb_args != 2) {
+        return;
+    }
+    len = strlen(str);
+    readline_set_completion_index(rs, len);
+    for (i = 0; NetClientOptionsKind_lookup[i]; i++) {
+        add_completion_option(rs, str, NetClientOptionsKind_lookup[i]);
+    }
+}
+
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str)
 {
    GSList *list, *elt;
@@ -4339,6 +4388,29 @@ static void device_del_bus_completion(ReadLineState *rs,  BusState *bus,
    }
 }

+void chardev_remove_completion(ReadLineState *rs, int nb_args, const char *str)
+{
+    size_t len;
+    ChardevInfoList *list, *start;
+
+    if (nb_args != 2) {
+        return;
+    }
+    len = strlen(str);
+    readline_set_completion_index(rs, len);
+
+    start = list = qmp_query_chardev(NULL);
+    while (list) {
+        ChardevInfo *chr = list->value;
+
+        if (!strncmp(chr->label, str, len)) {
+            readline_add_completion(rs, chr->label);
+        }
+        list = list->next;
+    }
+    qapi_free_ChardevInfoList(start);
+}
+
 void device_del_completion(ReadLineState *rs, int nb_args, const char *str)
 {
    size_t len;
@@ -4376,6 +4448,77 @@ void object_del_completion(ReadLineState *rs, int nb_args, const char *str)
    qapi_free_ObjectPropertyInfoList(start);
 }

+void sendkey_completion(ReadLineState *rs, int nb_args, const char *str)
+{
+    int i;
+    char *sep;
+    size_t len;
+
+    if (nb_args != 2) {
+        return;
+    }
+    sep = strrchr(str, '-');
+    if (sep) {
+        str = sep + 1;
+    }
+    len = strlen(str);
+    readline_set_completion_index(rs, len);
+    for (i = 0; i < Q_KEY_CODE_MAX; i++) {
+        if (!strncmp(str, QKeyCode_lookup[i], len)) {
+            readline_add_completion(rs, QKeyCode_lookup[i]);
+        }
+    }
+}
+
+void set_link_completion(ReadLineState *rs, int nb_args, const char *str)
+{
+    size_t len;
+
+    len = strlen(str);
+    readline_set_completion_index(rs, len);
+    if (nb_args == 2) {
+        NetClientState *ncs[255];
+        int count, i;
+        count = qemu_find_net_clients_except(NULL, ncs,
+                                             NET_CLIENT_OPTIONS_KIND_NONE, 255);
+        for (i = 0; i < count; i++) {
+            const char *name = ncs[i]->name;
+            if (!strncmp(str, name, len)) {
+                readline_add_completion(rs, name);
+            }
+        }
+    } else if (nb_args == 3) {
+        add_completion_option(rs, str, "on");
+        add_completion_option(rs, str, "off");
+    }
+}
+
+void netdev_del_completion(ReadLineState *rs, int nb_args, const char *str)
+{
+    int len, count, i;
+    NetClientState *ncs[255];
+
+    if (nb_args != 2) {
+        return;
+    }
+
+    len = strlen(str);
+    readline_set_completion_index(rs, len);
+    count = qemu_find_net_clients_except(NULL, ncs, NET_CLIENT_OPTIONS_KIND_NIC,
+                                         255);
+    for (i = 0; i < count; i++) {
+        QemuOpts *opts;
+        const char *name = ncs[i]->name;
+        if (strncmp(str, name, len)) {
+            continue;
+        }
+        opts = qemu_opts_find(qemu_find_opts_err("netdev", NULL), name);
+        if (opts) {
+            readline_add_completion(rs, name);
+        }
+    }
+}
+
 static void monitor_find_completion_by_table(Monitor *mon,
                                             const mon_cmd_t *cmd_table,
                                             char **args,
@@ -4444,15 +4587,7 @@ static void monitor_find_completion_by_table(Monitor *mon,
            break;
        case 's':
        case 'S':
-            if (!strcmp(cmd->name, "sendkey")) {
-                char *sep = strrchr(str, '-');
-                if (sep)
-                    str = sep + 1;
-                readline_set_completion_index(mon->rs, strlen(str));
-                for (i = 0; i < Q_KEY_CODE_MAX; i++) {
-                    cmd_completion(mon, str, QKeyCode_lookup[i]);
-                }
-            } else if (!strcmp(cmd->name, "help|?")) {
+            if (!strcmp(cmd->name, "help|?")) {
                monitor_find_completion_by_table(mon, cmd_table,
                                                 &args[1], nb_args - 1);
            }
--- a/net/net.c
+++ b/net/net.c
@@ -633,7 +633,7 @@ int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
        if (nc->info->type == type) {
            continue;
        }
-        if (!strcmp(nc->name, id)) {
+        if (!id || !strcmp(nc->name, id)) {
            if (ret < max) {
                ncs[ret] = nc;
            }
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -484,8 +484,7 @@ opts_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp)


 static void
-opts_start_optional(Visitor *v, bool *present, const char *name,
-                       Error **errp)
+opts_optional(Visitor *v, bool *present, const char *name, Error **errp)
 {
    OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);

@@ -528,7 +527,7 @@ opts_visitor_new(const QemuOpts *opts)
    /* type_number() is not filled in, but this is not the first visitor to
     * skip some mandatory methods... */

-    ov->visitor.start_optional = &opts_start_optional;
+    ov->visitor.optional = &opts_optional;

    ov->opts_root = opts;

--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -17,46 +17,27 @@
 #include "qapi/visitor.h"
 #include "qapi/visitor-impl.h"

-void visit_start_handle(Visitor *v, void **obj, const char *kind,
-                        const char *name, Error **errp)
-{
-    if (!error_is_set(errp) && v->start_handle) {
-        v->start_handle(v, obj, kind, name, errp);
-    }
-}
-
-void visit_end_handle(Visitor *v, Error **errp)
-{
-    if (!error_is_set(errp) && v->end_handle) {
-        v->end_handle(v, errp);
-    }
-}
-
 void visit_start_struct(Visitor *v, void **obj, const char *kind,
                        const char *name, size_t size, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->start_struct(v, obj, kind, name, size, errp);
-    }
+    v->start_struct(v, obj, kind, name, size, errp);
 }

 void visit_end_struct(Visitor *v, Error **errp)
 {
-    assert(!error_is_set(errp));
    v->end_struct(v, errp);
 }

 void visit_start_implicit_struct(Visitor *v, void **obj, size_t size,
                                 Error **errp)
 {
-    if (!error_is_set(errp) && v->start_implicit_struct) {
+    if (v->start_implicit_struct) {
        v->start_implicit_struct(v, obj, size, errp);
    }
 }

 void visit_end_implicit_struct(Visitor *v, Error **errp)
 {
-    assert(!error_is_set(errp));
    if (v->end_implicit_struct) {
        v->end_implicit_struct(v, errp);
    }
@@ -64,45 +45,31 @@ void visit_end_implicit_struct(Visitor *v, Error **errp)

 void visit_start_list(Visitor *v, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->start_list(v, name, errp);
-    }
+    v->start_list(v, name, errp);
 }

 GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        return v->next_list(v, list, errp);
-    }
-
-    return 0;
+    return v->next_list(v, list, errp);
 }

 void visit_end_list(Visitor *v, Error **errp)
 {
-    assert(!error_is_set(errp));
    v->end_list(v, errp);
 }

-void visit_start_optional(Visitor *v, bool *present, const char *name,
-                          Error **errp)
+void visit_optional(Visitor *v, bool *present, const char *name,
+                    Error **errp)
 {
-    if (!error_is_set(errp) && v->start_optional) {
-        v->start_optional(v, present, name, errp);
-    }
-}
-
-void visit_end_optional(Visitor *v, Error **errp)
-{
-    if (!error_is_set(errp) && v->end_optional) {
-        v->end_optional(v, errp);
+    if (v->optional) {
+        v->optional(v, present, name, errp);
    }
 }

 void visit_get_next_type(Visitor *v, int *obj, const int *qtypes,
                         const char *name, Error **errp)
 {
-    if (!error_is_set(errp) && v->get_next_type) {
+    if (v->get_next_type) {
        v->get_next_type(v, obj, qtypes, name, errp);
    }
 }
@@ -110,192 +77,172 @@ void visit_get_next_type(Visitor *v, int *obj, const int *qtypes,
 void visit_type_enum(Visitor *v, int *obj, const char *strings[],
                     const char *kind, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->type_enum(v, obj, strings, kind, name, errp);
-    }
+    v->type_enum(v, obj, strings, kind, name, errp);
 }

 void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->type_int(v, obj, name, errp);
-    }
+    v->type_int(v, obj, name, errp);
 }

 void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp)
 {
    int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_uint8) {
-            v->type_uint8(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < 0 || value > UINT8_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "uint8_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_uint8) {
+        v->type_uint8(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < 0 || value > UINT8_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "uint8_t");
+            return;
        }
+        *obj = value;
    }
 }

 void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp)
 {
    int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_uint16) {
-            v->type_uint16(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < 0 || value > UINT16_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "uint16_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_uint16) {
+        v->type_uint16(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < 0 || value > UINT16_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "uint16_t");
+            return;
        }
+        *obj = value;
    }
 }

 void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp)
 {
    int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_uint32) {
-            v->type_uint32(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < 0 || value > UINT32_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "uint32_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_uint32) {
+        v->type_uint32(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < 0 || value > UINT32_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "uint32_t");
+            return;
        }
+        *obj = value;
    }
 }

 void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
 {
    int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_uint64) {
-            v->type_uint64(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            *obj = value;
-        }
+
+    if (v->type_uint64) {
+        v->type_uint64(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        *obj = value;
    }
 }

 void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp)
 {
    int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_int8) {
-            v->type_int8(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < INT8_MIN || value > INT8_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "int8_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_int8) {
+        v->type_int8(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < INT8_MIN || value > INT8_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "int8_t");
+            return;
        }
+        *obj = value;
    }
 }

 void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp)
 {
    int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_int16) {
-            v->type_int16(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < INT16_MIN || value > INT16_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "int16_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_int16) {
+        v->type_int16(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < INT16_MIN || value > INT16_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "int16_t");
+            return;
        }
+        *obj = value;
    }
 }

 void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp)
 {
    int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_int32) {
-            v->type_int32(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            if (value < INT32_MIN || value > INT32_MAX) {
-                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
-                          "int32_t");
-                return;
-            }
-            *obj = value;
+
+    if (v->type_int32) {
+        v->type_int32(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        if (value < INT32_MIN || value > INT32_MAX) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+                      "int32_t");
+            return;
        }
+        *obj = value;
    }
 }

 void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        if (v->type_int64) {
-            v->type_int64(v, obj, name, errp);
-        } else {
-            v->type_int(v, obj, name, errp);
-        }
+    if (v->type_int64) {
+        v->type_int64(v, obj, name, errp);
+    } else {
+        v->type_int(v, obj, name, errp);
    }
 }

 void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp)
 {
    int64_t value;
-    if (!error_is_set(errp)) {
-        if (v->type_size) {
-            v->type_size(v, obj, name, errp);
-        } else if (v->type_uint64) {
-            v->type_uint64(v, obj, name, errp);
-        } else {
-            value = *obj;
-            v->type_int(v, &value, name, errp);
-            *obj = value;
-        }
+
+    if (v->type_size) {
+        v->type_size(v, obj, name, errp);
+    } else if (v->type_uint64) {
+        v->type_uint64(v, obj, name, errp);
+    } else {
+        value = *obj;
+        v->type_int(v, &value, name, errp);
+        *obj = value;
    }
 }

 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->type_bool(v, obj, name, errp);
-    }
+    v->type_bool(v, obj, name, errp);
 }

 void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->type_str(v, obj, name, errp);
-    }
+    v->type_str(v, obj, name, errp);
 }

 void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp)
 {
-    if (!error_is_set(errp)) {
-        v->type_number(v, obj, name, errp);
-    }
+    v->type_number(v, obj, name, errp);
 }

 void output_type_enum(Visitor *v, int *obj, const char *strings[],
@@ -321,13 +268,15 @@ void input_type_enum(Visitor *v, int *obj, const char *strings[],
                     const char *kind, const char *name,
                     Error **errp)
 {
+    Error *local_err = NULL;
    int64_t value = 0;
    char *enum_str;

    assert(strings);

-    visit_type_str(v, &enum_str, name, errp);
-    if (error_is_set(errp)) {
+    visit_type_str(v, &enum_str, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
        return;
    }

--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -286,8 +286,8 @@ static void qmp_input_type_number(Visitor *v, double *obj, const char *name,
    }
 }

-static void qmp_input_start_optional(Visitor *v, bool *present,
-                                     const char *name, Error **errp)
+static void qmp_input_optional(Visitor *v, bool *present, const char *name,
+                               Error **errp)
 {
    QmpInputVisitor *qiv = to_qiv(v);
    QObject *qobj = qmp_input_get_object(qiv, name, true);
@@ -329,7 +329,7 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
    v->visitor.type_bool = qmp_input_type_bool;
    v->visitor.type_str = qmp_input_type_str;
    v->visitor.type_number = qmp_input_type_number;
-    v->visitor.start_optional = qmp_input_start_optional;
+    v->visitor.optional = qmp_input_optional;
    v->visitor.get_next_type = qmp_input_get_next_type;

    qmp_input_push(v, obj, NULL);
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -120,8 +120,8 @@ static void parse_type_number(Visitor *v, double *obj, const char *name,
    *obj = val;
 }

-static void parse_start_optional(Visitor *v, bool *present,
-                                 const char *name, Error **errp)
+static void parse_optional(Visitor *v, bool *present, const char *name,
+                           Error **errp)
 {
    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);

@@ -155,7 +155,7 @@ StringInputVisitor *string_input_visitor_new(const char *str)
    v->visitor.type_bool = parse_type_bool;
    v->visitor.type_str = parse_type_str;
    v->visitor.type_number = parse_type_number;
-    v->visitor.start_optional = parse_start_optional;
+    v->visitor.optional = parse_optional;

    v->string = str;
    return v;
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3204,6 +3204,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
                                    void (*init)(struct CharDriverState *s),
                                    Error **errp)
 {
+    Error *local_err = NULL;
    CharDriver *cd;
    CharDriverState *chr;
    GSList *i;
@@ -3245,13 +3246,14 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
        chr = NULL;
        backend->kind = cd->kind;
        if (cd->parse) {
-            cd->parse(opts, backend, errp);
-            if (error_is_set(errp)) {
+            cd->parse(opts, backend, &local_err);
+            if (local_err) {
+                error_propagate(errp, local_err);
                goto qapi_out;
            }
        }
        ret = qmp_chardev_add(bid ? bid : id, backend, errp);
-        if (error_is_set(errp)) {
+        if (!ret) {
            goto qapi_out;
        }

@@ -3263,7 +3265,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
            backend->kind = CHARDEV_BACKEND_KIND_MUX;
            backend->mux->chardev = g_strdup(bid);
            ret = qmp_chardev_add(id, backend, errp);
-            if (error_is_set(errp)) {
+            if (!ret) {
                chr = qemu_chr_find(bid);
                qemu_chr_delete(chr);
                chr = NULL;
@@ -3620,18 +3622,18 @@ static int qmp_chardev_open_file_source(char *src, int flags,

 static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
 {
-    int flags, in = -1, out = -1;
+    int flags, in = -1, out;

    flags = O_WRONLY | O_TRUNC | O_CREAT | O_BINARY;
    out = qmp_chardev_open_file_source(file->out, flags, errp);
-    if (error_is_set(errp)) {
+    if (out < 0) {
        return NULL;
    }

    if (file->has_in) {
        flags = O_RDONLY;
        in = qmp_chardev_open_file_source(file->in, flags, errp);
-        if (error_is_set(errp)) {
+        if (in < 0) {
            qemu_close(out);
            return NULL;
        }
@@ -3647,7 +3649,7 @@ static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
    int fd;

    fd = qmp_chardev_open_file_source(serial->device, O_RDWR, errp);
-    if (error_is_set(errp)) {
+    if (fd < 0) {
        return NULL;
    }
    qemu_set_nonblock(fd);
@@ -3665,7 +3667,7 @@ static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
    int fd;

    fd = qmp_chardev_open_file_source(parallel->device, O_RDWR, errp);
-    if (error_is_set(errp)) {
+    if (fd < 0) {
        return NULL;
    }
    return qemu_chr_open_pp_fd(fd);
@@ -3692,7 +3694,7 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
    } else {
        fd = socket_connect(addr, errp, NULL, NULL);
    }
-    if (error_is_set(errp)) {
+    if (fd < 0) {
        return NULL;
    }
    return qemu_chr_open_socket_fd(fd, do_nodelay, is_listen,
@@ -3705,7 +3707,7 @@ static CharDriverState *qmp_chardev_open_udp(ChardevUdp *udp,
    int fd;

    fd = socket_dgram(udp->remote, udp->local, errp);
-    if (error_is_set(errp)) {
+    if (fd < 0) {
        return NULL;
    }
    return qemu_chr_open_udp_fd(fd);
@@ -3796,7 +3798,13 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
        break;
    }

-    if (chr == NULL && !error_is_set(errp)) {
+    /*
+     * Character backend open hasn't been fully converted to the Error
+     * API.  Some opens fail without setting an error.  Set a generic
+     * error then.
+     * TODO full conversion to Error API
+     */
+    if (chr == NULL && errp && !*errp) {
        error_setg(errp, "Failed to create chardev");
    }
    if (chr) {
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -2,16 +2,19 @@
 # QAPI command marshaller generator
 #
 # Copyright IBM, Corp. 2011
+# Copyright (C) 2014 Red Hat, Inc.
 #
 # Authors:
 #  Anthony Liguori <aliguori@us.ibm.com>
 #  Michael Roth    <mdroth@linux.vnet.ibm.com>
+#  Markus Armbruster <armbru@redhat.com>
 #
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.

 from ordereddict import OrderedDict
 from qapi import *
+import re
 import sys
 import os
 import getopt
@@ -37,6 +40,15 @@ def generate_command_decl(name, args, ret_type):
 ''',
                 ret_type=c_type(ret_type), name=c_fun(name), args=arglist).strip()

+def gen_err_check(errvar):
+    if errvar:
+        return mcgen('''
+if (local_err) {
+    goto out;
+}
+''')
+    return ''
+
 def gen_sync_call(name, args, ret_type, indent=0):
    ret = ""
    arglist=""
@@ -49,15 +61,14 @@ def gen_sync_call(name, args, ret_type, indent=0):
        arglist += "%s, " % (c_var(argname))
    push_indent(indent)
    ret = mcgen('''
-%(retval)sqmp_%(name)s(%(args)serrp);
+%(retval)sqmp_%(name)s(%(args)s&local_err);

 ''',
                name=c_fun(name), args=arglist, retval=retval).rstrip()
    if ret_type:
+        ret += "\n" + gen_err_check('local_err')
        ret += "\n" + mcgen(''''
-if (!error_is_set(errp)) {
-    %(marshal_output_call)s
-}
+%(marshal_output_call)s
 ''',
                            marshal_output_call=gen_marshal_output_call(name, ret_type)).rstrip()
    pop_indent(indent)
@@ -67,18 +78,19 @@ if (!error_is_set(errp)) {
 def gen_marshal_output_call(name, ret_type):
    if not ret_type:
        return ""
-    return "qmp_marshal_output_%s(retval, ret, errp);" % c_fun(name)
+    return "qmp_marshal_output_%s(retval, ret, &local_err);" % c_fun(name)

-def gen_visitor_input_containers_decl(args):
+def gen_visitor_input_containers_decl(args, obj):
    ret = ""

    push_indent()
    if len(args) > 0:
        ret += mcgen('''
-QmpInputVisitor *mi;
+QmpInputVisitor *mi = qmp_input_visitor_new_strict(%(obj)s);
 QapiDeallocVisitor *md;
 Visitor *v;
-''')
+''',
+                     obj=obj)
    pop_indent()

    return ret.rstrip()
@@ -106,9 +118,10 @@ bool has_%(argname)s = false;
    pop_indent()
    return ret.rstrip()

-def gen_visitor_input_block(args, obj, dealloc=False):
+def gen_visitor_input_block(args, dealloc=False):
    ret = ""
-    errparg = 'errp'
+    errparg = '&local_err'
+    errarg = 'local_err'

    if len(args) == 0:
        return ret
@@ -117,44 +130,44 @@ def gen_visitor_input_block(args, obj, dealloc=False):

    if dealloc:
        errparg = 'NULL'
+        errarg = None;
        ret += mcgen('''
+qmp_input_visitor_cleanup(mi);
 md = qapi_dealloc_visitor_new();
 v = qapi_dealloc_get_visitor(md);
 ''')
    else:
        ret += mcgen('''
-mi = qmp_input_visitor_new_strict(%(obj)s);
 v = qmp_input_get_visitor(mi);
-''',
-                     obj=obj)
+''')

    for argname, argtype, optional, structured in parse_args(args):
        if optional:
            ret += mcgen('''
-visit_start_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s);
-if (has_%(c_name)s) {
+visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s);
 ''',
                         c_name=c_var(argname), name=argname, errp=errparg)
+            ret += gen_err_check(errarg)
+            ret += mcgen('''
+if (has_%(c_name)s) {
+''',
+                         c_name=c_var(argname))
            push_indent()
        ret += mcgen('''
 %(visitor)s(v, &%(c_name)s, "%(name)s", %(errp)s);
 ''',
                     c_name=c_var(argname), name=argname, argtype=argtype,
                     visitor=type_visitor(argtype), errp=errparg)
+        ret += gen_err_check(errarg)
        if optional:
            pop_indent()
            ret += mcgen('''
 }
-visit_end_optional(v, %(errp)s);
-''', errp=errparg)
+''')

    if dealloc:
        ret += mcgen('''
 qapi_dealloc_visitor_cleanup(md);
-''')
-    else:
-        ret += mcgen('''
-qmp_input_visitor_cleanup(mi);
 ''')
    pop_indent()
    return ret.rstrip()
@@ -166,16 +179,22 @@ def gen_marshal_output(name, args, ret_type, middle_mode):
    ret = mcgen('''
 static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject **ret_out, Error **errp)
 {
-    QapiDeallocVisitor *md = qapi_dealloc_visitor_new();
+    Error *local_err = NULL;
    QmpOutputVisitor *mo = qmp_output_visitor_new();
+    QapiDeallocVisitor *md;
    Visitor *v;

    v = qmp_output_get_visitor(mo);
-    %(visitor)s(v, &ret_in, "unused", errp);
-    if (!error_is_set(errp)) {
-        *ret_out = qmp_output_get_qobject(mo);
+    %(visitor)s(v, &ret_in, "unused", &local_err);
+    if (local_err) {
+        goto out;
    }
+    *ret_out = qmp_output_get_qobject(mo);
+
+out:
+    error_propagate(errp, local_err);
    qmp_output_visitor_cleanup(mo);
+    md = qapi_dealloc_visitor_new();
    v = qapi_dealloc_get_visitor(md);
    %(visitor)s(v, &ret_in, "unused", NULL);
    qapi_dealloc_visitor_cleanup(md);
@@ -200,13 +219,12 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
    ret = mcgen('''
 %(header)s
 {
+    Error *local_err = NULL;
 ''',
                header=hdr)

    if middle_mode:
        ret += mcgen('''
-    Error *local_err = NULL;
-    Error **errp = &local_err;
    QDict *args = (QDict *)qdict;
 ''')

@@ -228,29 +246,32 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
 %(visitor_input_block)s

 ''',
-                     visitor_input_containers_decl=gen_visitor_input_containers_decl(args),
+                     visitor_input_containers_decl=gen_visitor_input_containers_decl(args, "QOBJECT(args)"),
                     visitor_input_vars_decl=gen_visitor_input_vars_decl(args),
-                     visitor_input_block=gen_visitor_input_block(args, "QOBJECT(args)"))
+                     visitor_input_block=gen_visitor_input_block(args))
    else:
        ret += mcgen('''
+
    (void)args;
 ''')

    ret += mcgen('''
-    if (error_is_set(errp)) {
-        goto out;
-    }
 %(sync_call)s
 ''',
                 sync_call=gen_sync_call(name, args, ret_type, indent=4))
-    ret += mcgen('''
+    if re.search('^ *goto out\\;', ret, re.MULTILINE):
+        ret += mcgen('''

 out:
+''')
+    if not middle_mode:
+        ret += mcgen('''
+    error_propagate(errp, local_err);
 ''')
    ret += mcgen('''
 %(visitor_input_block_cleanup)s
 ''',
-                 visitor_input_block_cleanup=gen_visitor_input_block(args, None,
+                 visitor_input_block_cleanup=gen_visitor_input_block(args,
                                                                     dealloc=True))

    if middle_mode:
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -2,21 +2,47 @@
 # QAPI visitor generator
 #
 # Copyright IBM, Corp. 2011
+# Copyright (C) 2014 Red Hat, Inc.
 #
 # Authors:
 #  Anthony Liguori <aliguori@us.ibm.com>
 #  Michael Roth    <mdroth@linux.vnet.ibm.com>
+#  Markus Armbruster <armbru@redhat.com>
 #
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.

 from ordereddict import OrderedDict
 from qapi import *
+import re
 import sys
 import os
 import getopt
 import errno

+implicit_structs = []
+
+def generate_visit_implicit_struct(type):
+    global implicit_structs
+    if type in implicit_structs:
+        return ''
+    implicit_structs.append(type)
+    return mcgen('''
+
+static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error **errp)
+{
+    Error *err = NULL;
+
+    visit_start_implicit_struct(m, (void **)obj, sizeof(%(c_type)s), &err);
+    if (!err) {
+        visit_type_%(c_type)s_fields(m, obj, errp);
+        visit_end_implicit_struct(m, &err);
+    }
+    error_propagate(errp, err);
+}
+''',
+                 c_type=type_name(type))
+
 def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base = None):
    substructs = []
    ret = ''
@@ -35,6 +61,19 @@ def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base =
            nested_field_prefix = "%s%s." % (field_prefix, argname)
            ret += generate_visit_struct_fields(name, nested_field_prefix,
                                                nested_fn_prefix, argentry)
+            ret += mcgen('''
+
+static void visit_type_%(full_name)s_field_%(c_name)s(Visitor *m, %(name)s **obj, Error **errp)
+{
+''',
+                         name=name, full_name=full_name, c_name=c_var(argname))
+            ret += generate_visit_struct_body(full_name, argname, argentry)
+            ret += mcgen('''
+}
+''')
+
+    if base:
+        ret += generate_visit_implicit_struct(base)

    ret += mcgen('''

@@ -47,12 +86,9 @@ static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s ** obj, Error *

    if base:
        ret += mcgen('''
-visit_start_implicit_struct(m, (void**) &(*obj)->%(c_name)s, sizeof(%(type)s), &err);
-if (!err) {
-    visit_type_%(type)s_fields(m, &(*obj)->%(c_prefix)s%(c_name)s, &err);
-    error_propagate(errp, err);
-    err = NULL;
-    visit_end_implicit_struct(m, &err);
+visit_type_implicit_%(type)s(m, &(*obj)->%(c_prefix)s%(c_name)s, &err);
+if (err) {
+    goto out;
 }
 ''',
                     c_prefix=c_var(field_prefix),
@@ -61,15 +97,18 @@ if (!err) {
    for argname, argentry, optional, structured in parse_args(members):
        if optional:
            ret += mcgen('''
-visit_start_optional(m, &(*obj)->%(c_prefix)shas_%(c_name)s, "%(name)s", &err);
-if ((*obj)->%(prefix)shas_%(c_name)s) {
+visit_optional(m, &(*obj)->%(c_prefix)shas_%(c_name)s, "%(name)s", &err);
+if (!err && (*obj)->%(prefix)shas_%(c_name)s) {
 ''',
                         c_prefix=c_var(field_prefix), prefix=field_prefix,
                         c_name=c_var(argname), name=argname)
            push_indent()

        if structured:
-            ret += generate_visit_struct_body(full_name, argname, argentry)
+            ret += mcgen('''
+visit_type_%(full_name)s_field_%(c_name)s(m, obj, &err);
+''',
+                         full_name=full_name, c_name=c_var(argname))
        else:
            ret += mcgen('''
 visit_type_%(type)s(m, &(*obj)->%(c_prefix)s%(c_name)s, "%(name)s", &err);
@@ -82,12 +121,20 @@ visit_type_%(type)s(m, &(*obj)->%(c_prefix)s%(c_name)s, "%(name)s", &err);
            pop_indent()
            ret += mcgen('''
 }
-visit_end_optional(m, &err);
+''')
+        ret += mcgen('''
+if (err) {
+    goto out;
+}
 ''')

    pop_indent()
-    ret += mcgen('''
+    if re.search('^ *goto out\\;', ret, re.MULTILINE):
+        ret += mcgen('''

+out:
+''')
+    ret += mcgen('''
    error_propagate(errp, err);
 }
 ''')
@@ -96,9 +143,9 @@ visit_end_optional(m, &err);

 def generate_visit_struct_body(field_prefix, name, members):
    ret = mcgen('''
-if (!error_is_set(errp)) {
+    Error *err = NULL;
+
 ''')
-    push_indent()

    if not field_prefix:
        full_name = name
@@ -107,36 +154,26 @@ if (!error_is_set(errp)) {

    if len(field_prefix):
        ret += mcgen('''
-Error **errp = &err; /* from outer scope */
-Error *err = NULL;
-visit_start_struct(m, NULL, "", "%(name)s", 0, &err);
+    visit_start_struct(m, NULL, "", "%(name)s", 0, &err);
 ''',
                name=name)
    else:
        ret += mcgen('''
-Error *err = NULL;
-visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err);
+    visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err);
 ''',
                name=name)

    ret += mcgen('''
-if (!err) {
-    if (*obj) {
-        visit_type_%(name)s_fields(m, obj, &err);
-        error_propagate(errp, err);
-        err = NULL;
-    }
-''',
-        name=full_name)
-
-    pop_indent()
-    ret += mcgen('''
-        /* Always call end_struct if start_struct succeeded.  */
+    if (!err) {
+        if (*obj) {
+            visit_type_%(name)s_fields(m, obj, errp);
+        }
        visit_end_struct(m, &err);
    }
    error_propagate(errp, err);
-}
-''')
+''',
+        name=full_name)
+
    return ret

 def generate_visit_struct(expr):
@@ -154,9 +191,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
 ''',
                name=name)

-    push_indent()
    ret += generate_visit_struct_body("", name, members)
-    pop_indent()

    ret += mcgen('''
 }
@@ -168,24 +203,26 @@ def generate_visit_list(name, members):

 void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, Error **errp)
 {
-    GenericList *i, **prev = (GenericList **)obj;
    Error *err = NULL;
+    GenericList *i, **prev;

-    if (!error_is_set(errp)) {
-        visit_start_list(m, name, &err);
-        if (!err) {
-            for (; (i = visit_next_list(m, prev, &err)) != NULL; prev = &i) {
-                %(name)sList *native_i = (%(name)sList *)i;
-                visit_type_%(name)s(m, &native_i->value, NULL, &err);
-            }
-            error_propagate(errp, err);
-            err = NULL;
-
-            /* Always call end_list if start_list succeeded.  */
-            visit_end_list(m, &err);
-        }
-        error_propagate(errp, err);
+    visit_start_list(m, name, &err);
+    if (err) {
+        goto out;
    }
+
+    for (prev = (GenericList **)obj;
+         !err && (i = visit_next_list(m, prev, &err)) != NULL;
+         prev = &i) {
+        %(name)sList *native_i = (%(name)sList *)i;
+        visit_type_%(name)s(m, &native_i->value, NULL, &err);
+    }
+
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_list(m, &err);
+out:
+    error_propagate(errp, err);
 }
 ''',
                name=name)
@@ -207,10 +244,15 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
 {
    Error *err = NULL;

-    if (!error_is_set(errp)) {
-        visit_start_implicit_struct(m, (void**) obj, sizeof(%(name)s), &err);
-        visit_get_next_type(m, (int*) &(*obj)->kind, %(name)s_qtypes, name, &err);
-        switch ((*obj)->kind) {
+    visit_start_implicit_struct(m, (void**) obj, sizeof(%(name)s), &err);
+    if (err) {
+        goto out;
+    }
+    visit_get_next_type(m, (int*) &(*obj)->kind, %(name)s_qtypes, name, &err);
+    if (err) {
+        goto out_end;
+    }
+    switch ((*obj)->kind) {
 ''',
    name=name)

@@ -225,22 +267,24 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **

        enum_full_value = generate_enum_full_value(disc_type, key)
        ret += mcgen('''
-        case %(enum_full_value)s:
-            visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
-            break;
+    case %(enum_full_value)s:
+        visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
+        break;
 ''',
                enum_full_value = enum_full_value,
                c_type = type_name(members[key]),
                c_name = c_fun(key))

    ret += mcgen('''
-        default:
-            abort();
-        }
-        error_propagate(errp, err);
-        err = NULL;
-        visit_end_implicit_struct(m, &err);
+    default:
+        abort();
    }
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_implicit_struct(m, &err);
+out:
+    error_propagate(errp, err);
 }
 ''')

@@ -277,40 +321,43 @@ def generate_visit_union(expr):
            del base_fields[discriminator]
        ret += generate_visit_struct_fields(name, "", "", base_fields)

+    if discriminator:
+        for key in members:
+            ret += generate_visit_implicit_struct(members[key])
+
    ret += mcgen('''

 void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp)
 {
    Error *err = NULL;

-    if (!error_is_set(errp)) {
-        visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err);
-        if (!err) {
-            if (*obj) {
+    visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err);
+    if (err) {
+        goto out;
+    }
+    if (*obj) {
 ''',
                 name=name)

-
-    push_indent()
-    push_indent()
-    push_indent()
-
    if base:
        ret += mcgen('''
-    visit_type_%(name)s_fields(m, obj, &err);
+        visit_type_%(name)s_fields(m, obj, &err);
+        if (err) {
+            goto out_obj;
+        }
 ''',
            name=name)

-    pop_indent()
-
    if not discriminator:
        disc_key = "type"
    else:
        disc_key = discriminator
    ret += mcgen('''
        visit_type_%(disc_type)s(m, &(*obj)->kind, "%(disc_key)s", &err);
-        if (!err) {
-            switch ((*obj)->kind) {
+        if (err) {
+            goto out_obj;
+        }
+        switch ((*obj)->kind) {
 ''',
                 disc_type = disc_type,
                 disc_key = disc_key)
@@ -319,45 +366,30 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
        if not discriminator:
            fmt = 'visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "data", &err);'
        else:
-            fmt = '''visit_start_implicit_struct(m, (void**) &(*obj)->%(c_name)s, sizeof(%(c_type)s), &err);
-                if (!err) {
-                    visit_type_%(c_type)s_fields(m, &(*obj)->%(c_name)s, &err);
-                    error_propagate(errp, err);
-                    err = NULL;
-                    visit_end_implicit_struct(m, &err);
-                }'''
+            fmt = 'visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err);'

        enum_full_value = generate_enum_full_value(disc_type, key)
        ret += mcgen('''
-            case %(enum_full_value)s:
-                ''' + fmt + '''
-                break;
+        case %(enum_full_value)s:
+            ''' + fmt + '''
+            break;
 ''',
                enum_full_value = enum_full_value,
                c_type=type_name(members[key]),
                c_name=c_fun(key))

    ret += mcgen('''
-            default:
-                abort();
-            }
+        default:
+            abort();
        }
+out_obj:
        error_propagate(errp, err);
        err = NULL;
    }
-''')
-    pop_indent()
-    ret += mcgen('''
-        /* Always call end_struct if start_struct succeeded.  */
-        visit_end_struct(m, &err);
-    }
+    visit_end_struct(m, &err);
+out:
    error_propagate(errp, err);
 }
-''')
-
-    pop_indent();
-    ret += mcgen('''
-}
 ''')

    return ret
@@ -476,7 +508,7 @@ fdecl.write(mcgen('''
 /* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */

 /*
- * schema-defined QAPI visitor function
+ * schema-defined QAPI visitor functions
 *
 * Copyright IBM, Corp. 2011
 *
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -73,13 +73,18 @@ class QAPIExprError(Exception):

 class QAPISchema:

-    def __init__(self, fp, input_relname=None, include_hist=[], parent_info=None):
+    def __init__(self, fp, input_relname=None, include_hist=[],
+                 previously_included=[], parent_info=None):
+        """ include_hist is a stack used to detect inclusion cycles
+            previously_included is a global state used to avoid multiple
+                                inclusions of the same file"""
        input_fname = os.path.abspath(fp.name)
        if input_relname is None:
            input_relname = fp.name
        self.input_dir = os.path.dirname(input_fname)
        self.input_file = input_relname
        self.include_hist = include_hist + [(input_relname, input_fname)]
+        previously_included.append(input_fname)
        self.parent_info = parent_info
        self.src = fp.read()
        if self.src == '' or self.src[-1] != '\n':
@@ -106,13 +111,16 @@ class QAPISchema:
                       for elem in self.include_hist):
                    raise QAPIExprError(expr_info, "Inclusion loop for %s"
                                        % include)
+                # skip multiple include of the same file
+                if include_path in previously_included:
+                    continue
                try:
                    fobj = open(include_path, 'r')
                except IOError as e:
                    raise QAPIExprError(expr_info,
                                        '%s: %s' % (e.strerror, include))
-                exprs_include = QAPISchema(fobj, include,
-                                           self.include_hist, expr_info)
+                exprs_include = QAPISchema(fobj, include, self.include_hist,
+                                           previously_included, expr_info)
                self.exprs.extend(exprs_include.exprs)
            else:
                expr_elem = {'expr': expr,
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -193,7 +193,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
        flat-union-string-discriminator.json \
        include-simple.json include-relpath.json include-format-err.json \
        include-non-file.json include-no-file.json include-before-err.json \
-        include-nested-err.json include-self-cycle.json include-cycle.json)
+        include-nested-err.json include-self-cycle.json include-cycle.json \
+        include-repetition.json)

 GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h

--- a/tests/qapi-schema/include-repetition-sub.json
+++ b/tests/qapi-schema/include-repetition-sub.json
@@ -0,0 +1,2 @@
+{ 'include': 'comments.json' }
+{ 'include': 'comments.json' }
--- a/tests/qapi-schema/include-repetition.err
+++ b/tests/qapi-schema/include-repetition.err
--- a/tests/qapi-schema/include-repetition.exit
+++ b/tests/qapi-schema/include-repetition.exit
@@ -0,0 +1 @@
+0
--- a/tests/qapi-schema/include-repetition.json
+++ b/tests/qapi-schema/include-repetition.json
@@ -0,0 +1,3 @@
+{ 'include': 'comments.json' }
+{ 'include': 'include-repetition-sub.json' }
+{ 'include': 'comments.json' }
--- a/tests/qapi-schema/include-repetition.out
+++ b/tests/qapi-schema/include-repetition.out
@@ -0,0 +1,3 @@
+[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
+[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}]
+[]
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -72,14 +72,30 @@ typedef struct TestStruct
 static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
                                  const char *name, Error **errp)
 {
+    Error *err = NULL;
+
    visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct),
-                       errp);
+                       &err);
+    if (err) {
+        goto out;
+    }

-    visit_type_int(v, &(*obj)->integer, "integer", errp);
-    visit_type_bool(v, &(*obj)->boolean, "boolean", errp);
-    visit_type_str(v, &(*obj)->string, "string", errp);
+    visit_type_int(v, &(*obj)->integer, "integer", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_str(v, &(*obj)->string, "string", &err);

-    visit_end_struct(v, errp);
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_struct(v, &err);
+out:
+    error_propagate(errp, err);
 }

 static void test_validate_struct(TestInputVisitorData *data,
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -199,16 +199,24 @@ static void visit_type_TestStruct(Visitor *v, TestStruct **obj,

    visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct),
                       &err);
-    if (!err) {
-        visit_type_int(v, &(*obj)->integer, "integer", &err);
-        visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
-        visit_type_str(v, &(*obj)->string, "string", &err);
-
-        /* Always call end_struct if start_struct succeeded.  */
-        error_propagate(errp, err);
-        err = NULL;
-        visit_end_struct(v, &err);
+    if (err) {
+        goto out;
    }
+    visit_type_int(v, &(*obj)->integer, "integer", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_str(v, &(*obj)->string, "string", &err);
+
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_struct(v, &err);
+out:
    error_propagate(errp, err);
 }

--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -176,14 +176,30 @@ typedef struct TestStruct
 static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
                                  const char *name, Error **errp)
 {
+    Error *err = NULL;
+
    visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct),
-                       errp);
+                       &err);
+    if (err) {
+        goto out;
+    }

-    visit_type_int(v, &(*obj)->integer, "integer", errp);
-    visit_type_bool(v, &(*obj)->boolean, "boolean", errp);
-    visit_type_str(v, &(*obj)->string, "string", errp);
+    visit_type_int(v, &(*obj)->integer, "integer", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_str(v, &(*obj)->string, "string", &err);

-    visit_end_struct(v, errp);
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_struct(v, &err);
+out:
+    error_propagate(errp, err);
 }

 static void test_visitor_out_struct(TestOutputVisitorData *data,
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -195,13 +195,29 @@ typedef struct TestStruct
 static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
                                  const char *name, Error **errp)
 {
-    visit_start_struct(v, (void **)obj, NULL, name, sizeof(TestStruct), errp);
+    Error *err = NULL;

-    visit_type_int(v, &(*obj)->integer, "integer", errp);
-    visit_type_bool(v, &(*obj)->boolean, "boolean", errp);
-    visit_type_str(v, &(*obj)->string, "string", errp);
+    visit_start_struct(v, (void **)obj, NULL, name, sizeof(TestStruct), &err);
+    if (err) {
+        goto out;
+    }

-    visit_end_struct(v, errp);
+    visit_type_int(v, &(*obj)->integer, "integer", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
+    if (err) {
+        goto out_end;
+    }
+    visit_type_str(v, &(*obj)->string, "string", &err);
+
+out_end:
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_struct(v, &err);
+out:
+    error_propagate(errp, err);
 }

 static TestStruct *struct_create(void)
--- a/6
+++ b/6
@@ -688,17 +688,21 @@ megasas_dcmd_ld_get_list(int cmd, int num, int max) "scmd %d: DCMD LD get list:
 megasas_dcmd_ld_get_info(int cmd, int ld_id) "scmd %d: DCMD LD get info for dev %d"
 megasas_dcmd_pd_get_info(int cmd, int pd_id) "scmd %d: DCMD PD get info for dev %d"
 megasas_dcmd_pd_list_query(int cmd, int flags) "scmd %d: DCMD PD list query flags %x"
+megasas_dcmd_ld_list_query(int cmd, int flags) "scmd %d: DCMD LD list query flags %x"
 megasas_dcmd_unsupported(int cmd, unsigned long size) "scmd %d: set properties len %ld"
 megasas_abort_frame(int cmd, int abort_cmd) "scmd %d: aborting frame %x"
 megasas_abort_no_cmd(int cmd, uint64_t context) "scmd %d: no active command for frame context %" PRIx64 ""
 megasas_abort_invalid_context(int cmd, uint64_t context, int abort_cmd) "scmd %d: invalid frame context %" PRIx64 " for abort frame %x"
 megasas_reset(void) "Reset"
-megasas_init(int sges, int cmds, const char *intr, const char *mode) "Using %d sges, %d cmds, %s, %s mode"
+megasas_init(int sges, int cmds, const char *mode) "Using %d sges, %d cmds, %s mode"
 megasas_msix_raise(int vector) "vector %d"
+megasas_msi_raise(int vector) "vector %d"
 megasas_irq_lower(void) "INTx"
 megasas_irq_raise(void) "INTx"
 megasas_intr_enabled(void) "Interrupts enabled"
 megasas_intr_disabled(void) "Interrupts disabled"
+megasas_msix_enabled(int vector) "vector %d"
+megasas_msi_enabled(int vector) "vector %d"
 megasas_mmio_readl(unsigned long addr, uint32_t val) "addr 0x%lx: 0x%x"
 megasas_mmio_invalid_readl(unsigned long addr) "addr 0x%lx"
 megasas_mmio_writel(uint32_t addr, uint32_t val) "addr 0x%x: 0x%x"
--- a/util/error.c
+++ b/util/error.c
@@ -142,11 +142,6 @@ Error *error_copy(const Error *err)
    return err_new;
 }

-bool error_is_set(Error **errp)
-{
-    return (errp && *errp);
-}
-
 ErrorClass error_get_class(const Error *err)
 {
    return err->err_class;
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -354,6 +354,7 @@ static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp)
 int inet_connect_opts(QemuOpts *opts, Error **errp,
                      NonBlockingConnectHandler *callback, void *opaque)
 {
+    Error *local_err = NULL;
    struct addrinfo *res, *e;
    int sock = -1;
    bool in_progress;
@@ -372,24 +373,27 @@ int inet_connect_opts(QemuOpts *opts, Error **errp,
    }

    for (e = res; e != NULL; e = e->ai_next) {
-        if (error_is_set(errp)) {
-            error_free(*errp);
-            *errp = NULL;
-        }
+        error_free(local_err);
+        local_err = NULL;
        if (connect_state != NULL) {
            connect_state->current_addr = e;
        }
-        sock = inet_connect_addr(e, &in_progress, connect_state, errp);
-        if (in_progress) {
-            return sock;
-        } else if (sock >= 0) {
-            /* non blocking socket immediate success, call callback */
-            if (callback != NULL) {
-                callback(sock, opaque);
-            }
+        sock = inet_connect_addr(e, &in_progress, connect_state, &local_err);
+        if (sock >= 0) {
            break;
        }
    }
+
+    if (sock < 0) {
+        error_propagate(errp, local_err);
+    } else if (in_progress) {
+        /* wait_for_connect() will do the rest */
+        return sock;
+    } else {
+        if (callback) {
+            callback(sock, opaque);
+        }
+    }
    g_free(connect_state);
    freeaddrinfo(res);
    return sock;
Author	SHA1	Message	Date
Markus Armbruster	d2e064a73e	error: error_is_set() is finally unused; remove Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>	2014-05-21 11:57:58 +02:00
Markus Armbruster	3894c78764	char: Explain qmp_chardev_add()'s unusual error handling Character backend open hasn't been fully converted to the Error API. Some opens fail without setting an error. qmp_chardev_add() needs to detect when that happens, and set a generic error. Explain that in a comment, and inline error_is_set() for clarity. Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>	2014-05-21 11:57:58 +02:00
Markus Armbruster	0aff637e92	char: Clean up fragile use of error_is_set() Using error_is_set(ERRP) to find out whether a function failed is either wrong, fragile, or unnecessarily opaque. It's wrong when ERRP may be null, because errors go undetected when it is. It's fragile when proving ERRP non-null involves a non-local argument. Else, it's unnecessarily opaque (see commit `84d18f0`). The error_is_set(errp) in qemu_chr_new_from_opts() is merely fragile, because the callers never pass a null errp argument. Make the code more robust and more obviously correct: receive the error in a local variable, then propagate it through the parameter. Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>	2014-05-21 11:57:58 +02:00
Markus Armbruster	5f758366c0	char: Use return values instead of error_is_set(errp) Using error_is_set(errp) to check whether a function call failed is fragile: it breaks when errp is null. Check perfectly suitable return values instead when possible. As far as I can tell, errp can't be null there, but this is more robust and more obviously correct Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>	2014-05-21 11:57:57 +02:00
Markus Armbruster	3f9286b721	qemu-socket: Clean up inet_connect_opts() Separate the search for a working addrinfo from the code that does something with it. Makes for a clearer search loop. Use a local Error * to simplify resetting the error in the search loop. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>	2014-05-21 11:57:57 +02:00
Peter Maydell	c5fa6c86d0	Merge remote-tracking branch 'remotes/qmp-unstable/queue/qmp' into staging * remotes/qmp-unstable/queue/qmp: qapi: skip redundant includes monitor: Add netdev_del id argument completion. monitor: Add netdev_add type argument completion. monitor: Add set_link arguments completion. monitor: Add chardev-add backend argument completion. monitor: Add chardev-remove command completion. monitor: Convert sendkey to use command_completion. qapi: Show qapi-commands.py invocation in qapi-code-gen.txt qapi: Replace uncommon use of the error API by the common one tests: Don't call visit_end_struct() after visit_start_struct() fails hw: Don't call visit_end_struct() after visit_start_struct() fails hmp: Call visit_end_struct() after visit_start_struct() succeeds qapi: Un-inline visit of implicit struct qapi-visit.py: Clean up a sloppy use of field prefix qapi: Clean up shadowing of parameters and locals in inner scopes qapi-visit.py: Clean up confusing push_indent() / pop_indent() use qapi: Replace start_optional()/end_optional() by optional() qapi: Remove unused Visitor callbacks start_handle(), end_handle() qapi: Normalize marshalling's visitor initialization and cleanup qapi: Update qapi-code-gen.txt example to match current code Signed-off-by: Peter Maydell <peter.maydell@linaro.org>	2014-05-19 14:10:01 +01:00
Peter Maydell	5bc8f026dd	Merge remote-tracking branch 'remotes/kraxel/tags/pull-input-8' into staging Input code update: - add keycode mapping helpers to core. - start switching devices to new input api. - misc bugfixes. # gpg: Signature made Fri 16 May 2014 07:43:45 BST using RSA key ID D3E87138 # gpg: Good signature from "Gerd Hoffmann (work) <kraxel@redhat.com>" # gpg: aka "Gerd Hoffmann <gerd@kraxel.org>" # gpg: aka "Gerd Hoffmann (private) <kraxel@gmail.com>" * remotes/kraxel/tags/pull-input-8: input: sparc32 kbd: claim en-us layout input: sparc32 kbd: fix some key mappings input: remove sparc keymap hack input: switch sparc32 kbd to new input api input: switch ps/2 mouse to new input api input: switch ps/2 kbd to new input api input: use KeyValue directly in sendkey monitor command input: add qemu_input_handler_deactivate input: key mapping helpers ps2: set ps/2 output buffer size as the same as kernel Signed-off-by: Peter Maydell <peter.maydell@linaro.org>	2014-05-19 12:53:07 +01:00
Peter Maydell	6a23082b4e	Merge remote-tracking branch 'remotes/bonzini/scsi-next' into staging * remotes/bonzini/scsi-next: [PATCH] block/iscsi: bump year in copyright notice block/iscsi: allow cluster_size of 4K and greater block/iscsi: clarify the meaning of ISCSI_CHECKALLOC_THRES block/iscsi: speed up read for unallocated sectors block/iscsi: allow fall back to WRITE SAME without UNMAP MAINTAINERS: mark megasas as maintained megasas: Add MSI support megasas: Enable MSI-X support megasas: Implement LD_LIST_QUERY scsi: Improve error messages more scsi-disk: Improve error messager if can't get version number Signed-off-by: Peter Maydell <peter.maydell@linaro.org>	2014-05-19 12:30:06 +01:00
Benoît Canet	24fd848950	qapi: skip redundant includes The purpose of this change is to help create a json file containing common definitions; each bit of generated C code must be emitted only one time. A second history global to all QAPISchema instances has been added to detect when a file is included more than one time and skip these includes. It does not act as a stack and the changes made to it by the __init__ function are propagated back to the caller so it's really a global state. Signed-off-by: Benoit Canet <benoit@irqsave.net> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-16 10:35:59 -04:00
Hani Benhabiles	11b389f21e	monitor: Add netdev_del id argument completion. Signed-off-by: Hani Benhabiles <hani@linux.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 15:16:02 -04:00
Hani Benhabiles	b162b49adc	monitor: Add netdev_add type argument completion. Also update the command's documentation. Signed-off-by: Hani Benhabiles <hani@linux.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 15:16:02 -04:00
Hani Benhabiles	40d19394b7	monitor: Add set_link arguments completion. Make it possible to query all net clients without specifying an ID when calling qemu_find_net_clients_except(). This also adds the add_completion_option() function which is to be used for other commands completions as well. Signed-off-by: Hani Benhabiles <hani@linux.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 15:16:02 -04:00
Hani Benhabiles	13e315dada	monitor: Add chardev-add backend argument completion. Signed-off-by: Hani Benhabiles <hani@linux.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 15:16:01 -04:00
Hani Benhabiles	6297d9a279	monitor: Add chardev-remove command completion. Signed-off-by: Hani Benhabiles <hani@linux.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 15:16:01 -04:00
Hani Benhabiles	29136cd8a4	monitor: Convert sendkey to use command_completion. Signed-off-by: Hani Benhabiles <hani@linux.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 15:16:01 -04:00
Markus Armbruster	87a560c455	qapi: Show qapi-commands.py invocation in qapi-code-gen.txt While there, pare down the shell prompts. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 15:16:01 -04:00
Markus Armbruster	297a3646c2	qapi: Replace uncommon use of the error API by the common one We commonly use the error API like this: err = NULL; foo(..., &err); if (err) { goto out; } bar(..., &err); Every error source is checked separately. The second function is only called when the first one succeeds. Both functions are free to pass their argument to error_set(). Because error_set() asserts no error has been set, this effectively means they must not be called with an error set. The qapi-generated code uses the error API differently: // errp was initialized to NULL somewhere up the call chain frob(..., errp); gnat(..., errp); Errors accumulate in errp: first error wins, subsequent errors get dropped. To make this work, the second function does nothing when called with an error set. Requires non-null errp, or else the second function can't see the first one fail. This usage has also bled into visitor tests, and two device model object property getters rtc_get_date() and balloon_stats_get_all(). With the "accumulate" technique, you need fewer error checks in callers, and buy that with an error check in every callee. Can be nice. However, mixing the two techniques is confusing. You can't use the "accumulate" technique with functions designed for the "check separately" technique. You can use the "check separately" technique with functions designed for the "accumulate" technique, but then error_set() can't catch you setting an error more than once. Standardize on the "check separately" technique for now, because it's overwhelmingly prevalent. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 14:00:46 -04:00
Markus Armbruster	cdaec3808e	tests: Don't call visit_end_struct() after visit_start_struct() fails When visit_start_struct() fails, visit_end_struct() must not be called. Three out of four visit_type_TestStruct() call it anyway. As far as I can tell, visit_start_struct() doesn't actually fail there. Fix them anyway. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 14:00:46 -04:00
Markus Armbruster	2ddb16a95f	hw: Don't call visit_end_struct() after visit_start_struct() fails When visit_start_struct() fails, visit_end_struct() must not be called. rtc_get_date() and balloon_stats_all() call it anyway. As far as I can tell, they're only used with the string output visitor, which doesn't care. Fix them anyway. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 14:00:46 -04:00
Markus Armbruster	f9f3a5ecde	hmp: Call visit_end_struct() after visit_start_struct() succeeds When visit_start_struct() succeeds, visit_end_struct() must be called. hmp_object_add() doesn't when a member visit fails. As far as I can tell, the opts visitor copes okay with the misuse. Fix it anyway. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 14:00:46 -04:00
Markus Armbruster	be3c771796	qapi: Un-inline visit of implicit struct In preparation of error handling changes. Bonus: generates less duplicated code. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 14:00:46 -04:00
Markus Armbruster	192cca60ae	qapi-visit.py: Clean up a sloppy use of field prefix generate_visit_struct_fields() generates the base type's struct member name both with and without the field prefix. Harmless, because the field prefix is always empty there: only unboxed complex members have a prefix, and those can't have a base type. Clean it up anyway. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 14:00:46 -04:00
Markus Armbruster	4fa953f20d	qapi: Clean up shadowing of parameters and locals in inner scopes By un-inlining the visit of nested complex types. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 14:00:45 -04:00
Markus Armbruster	468866b816	qapi-visit.py: Clean up confusing push_indent() / pop_indent() use Changing implicit indentation in the middle of generating a block makes following the code being generated unnecessarily hard. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 14:00:45 -04:00
Markus Armbruster	e2cd0f4fb4	qapi: Replace start_optional()/end_optional() by optional() Semantics of end_optional() differ subtly from the other end_FOO() callbacks: when start_FOO() succeeds, the matching end_FOO() gets called regardless of what happens in between. end_optional() gets called only when everything in between succeeds as well. Entirely undocumented, like all of the visitor API. The only user of Visitor Callback end_optional() never did anything, and was removed in commit `9f9ab46`. I'm about to clean up error handling in the generated visitor code, and end_optional() is in my way. No users mean no test cases, and making non-trivial cleanup transformations without test cases doesn't strike me as a good idea. Drop end_optional(), and rename start_optional() to optional(). We can always go back to a pair of callbacks when we have an actual need. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 14:00:45 -04:00
Markus Armbruster	cbc95538ed	qapi: Remove unused Visitor callbacks start_handle(), end_handle() These have never been called or implemented by anything, and their intended use is undocumented, like all of the visitor API. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 14:00:45 -04:00
Markus Armbruster	f9bee751be	qapi: Normalize marshalling's visitor initialization and cleanup Input and output marshalling functions do it differently. Change them to work the same: initialize the I/O visitor, use it, clean it up, initialize the dealloc visitor, use it, clean it up. This delays dealloc visitor initialization in output marshalling functions, and input visitor cleanup in input marshalling functions. No functional change, but the latter will be convenient when I change the error handling. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 14:00:45 -04:00
Markus Armbruster	6e2bb3ec70	qapi: Update qapi-code-gen.txt example to match current code Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>	2014-05-15 14:00:45 -04:00
Peter Lieven	6a86dec619	[PATCH] block/iscsi: bump year in copyright notice Signed-off-by: Peter Lieven <pl@kamp.de>	2014-05-05 10:04:30 +02:00
Peter Lieven	3d2acaa308	block/iscsi: allow cluster_size of 4K and greater depending on the target the opt_unmap_gran might be as low as 4K. As we know use this also as a knob to activate the allocationmap feature lower the barrier. The limit 4K (and not 512) is choosen to avoid a potentially too big allocationmap. Signed-off-by: Peter Lieven <pl@kamp.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>	2014-04-29 11:15:01 +02:00
Peter Lieven	5917af812e	block/iscsi: clarify the meaning of ISCSI_CHECKALLOC_THRES Signed-off-by: Peter Lieven <pl@kamp.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>	2014-04-29 11:14:41 +02:00
Peter Lieven	b03c38057b	block/iscsi: speed up read for unallocated sectors this patch implements a cache that tracks if a page on the iscsi target is allocated or not. The cache is implemented in a way that it allows for false positives (e.g. pretending a page is allocated, but it isn't), but no false negatives. The cached allocation info is then used to speed up the read process for unallocated sectors by issueing a GET_LBA_STATUS request for all sectors that are not yet known to be allocated. If the read request is confirmed to fall into an unallocated range we directly return zeroes and do not transfer the data over the wire. Tests have shown that a relatively small amount of GET_LBA_STATUS requests happens a vServer boot time to fill the allocation cache (all those blocks are not queried again). Not to transfer all the data of unallocated sectors saves a lot of time, bandwidth and storage I/O load during block jobs or storage migration and it saves a lot of bandwidth as well for any big sequential read of the whole disk (e.g. block copy or speed tests) if a significant number of blocks is unallocated. Signed-off-by: Peter Lieven <pl@kamp.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>	2014-04-29 11:14:25 +02:00
Peter Lieven	dbe5c58f2a	block/iscsi: allow fall back to WRITE SAME without UNMAP if the iscsi driver receives a write zeroes request with the BDRV_REQ_MAY_UNMAP flag set it fails with -ENOTSUP if the iscsi target does not support WRITE SAME with UNMAP. However, the BDRV_REQ_MAY_UNMAP is only a hint and writing zeroes with WRITE SAME will still be better than falling back to writing zeroes with WRITE16. Signed-off-by: Peter Lieven <pl@kamp.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>	2014-04-28 15:04:17 +02:00
Hannes Reinecke	d383c625e2	MAINTAINERS: mark megasas as maintained Signed-off-by: Hannes Reinecke <hare@suse.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>	2014-04-28 13:51:23 +02:00
Hannes Reinecke	4522b69c6c	megasas: Add MSI support Some hardware instances do support MSI, so we should do likewise. Signed-off-by: Hannes Reinecke <hare@suse.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>	2014-04-28 13:51:13 +02:00
Hannes Reinecke	23335f6273	megasas: Enable MSI-X support MSI-X support has been fixed in qemu, so we can enable it again. Signed-off-by: Hannes Reinecke <hare@suse.de> [Do not change VMSTATE_PCI_DEVICE to PCIE. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>	2014-04-28 13:50:01 +02:00
Hannes Reinecke	34bb4d02e0	megasas: Implement LD_LIST_QUERY Newer firmware implement a LD_LIST_QUERY command, and due to a driver issue no drives might be detected if this command isn't supported. So add emulation for this command, too. Cc: qemu-stable@nongnu.org Signed-off-by: Hannes Reinecke <hare@suse.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>	2014-04-28 13:48:41 +02:00
Paolo Bonzini	6ee143a0a4	scsi: Improve error messages more Remove the "scsi-block:" prefix for error messages as suggested by Markus. Improve the previous patch by making the message the same for both scsi-block and scsi-generic, including the strerror() output in both and making an explicit reference to SG_IO. Also s/can not/cannot/. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>	2014-04-28 13:38:00 +02:00
Fam Zheng	4bbeb8b173	scsi-disk: Improve error messager if can't get version number More often it is that bdrv_ioctl fails due to not supported by driver or whatever reason, in this case we should be specific, because "interface too old" is very confusing. Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>	2014-04-28 12:09:12 +02:00