diff options
author | Peter Maydell <peter.maydell@linaro.org> | 2014-05-09 15:46:34 +0100 |
---|---|---|
committer | Peter Maydell <peter.maydell@linaro.org> | 2014-05-09 15:46:34 +0100 |
commit | 06b4f00d53637f2c16a62c2cbaa30bffb045cf88 (patch) | |
tree | 4b3c8803e3947c3f1a87d4da1b5c77362dede250 | |
parent | 43cbeffb19877c62cbe0aaf08b2f235d98d71340 (diff) | |
parent | b690d679c1ca65d71b0544a2331d50e9f0f95116 (diff) | |
download | qemu-06b4f00d53637f2c16a62c2cbaa30bffb045cf88.zip |
Merge remote-tracking branch 'remotes/qmp-unstable/queue/qmp' into staging
* remotes/qmp-unstable/queue/qmp: (38 commits)
Revert "qapi: Clean up superfluous null check in qapi_dealloc_type_str()"
qapi: Document optional arguments' backwards compatibility
qmp: use valid JSON in transaction example
qmp: Don't use error_is_set() to suppress additional errors
dump: Drop pointless error_is_set(), DumpState member errp
qemu-option: Clean up fragile use of error_is_set()
qga: Drop superfluous error_is_set()
qga: Clean up fragile use of error_is_set()
qapi: Clean up fragile use of error_is_set()
tests/qapi-schema: Drop superfluous error_is_set()
qapi: Drop redundant, unclean error_is_set()
hmp: Guard against misuse of hmp_handle_error()
qga: Use return values instead of error_is_set(errp)
error: Consistently name Error ** objects errp, and not err
qmp: Consistently name Error ** objects errp, and not err
qga: Consistently name Error ** objects errp, and not err
qmp hmp: Consistently name Error * objects err, and not errp
pci-assign: assigned_initfn(): set monitor error in common error handler
pci-assign: propagate errors from assign_intx()
pci-assign: propagate errors from assign_device()
...
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
95 files changed, 942 insertions, 665 deletions
@@ -238,23 +238,35 @@ qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ + $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \ + " GEN $@") qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \ + $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \ + " GEN $@") qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \ + $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \ + " GEN $@") qapi-types.c qapi-types.h :\ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o "." -b < $<, " GEN $@") + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ + $(gen-out-type) -o "." -b -i $<, \ + " GEN $@") qapi-visit.c qapi-visit.h :\ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o "." -b < $<, " GEN $@") + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \ + $(gen-out-type) -o "." -b -i $<, \ + " GEN $@") qmp-commands.h qmp-marshal.c :\ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o "." < $<, " GEN $@") + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \ + $(gen-out-type) -o "." -m -i $<, \ + " GEN $@") QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h) $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN) diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index d78921f875..26312d84e8 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -40,6 +40,17 @@ enumeration types and union types. Generally speaking, types definitions should always use CamelCase for the type names. Command names should be all lower case with words separated by a hyphen. + +=== Includes === + +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. + + === Complex types === A complex type is a dictionary containing a single key whose value is a @@ -49,10 +60,34 @@ example of a complex type is: { 'type': 'MyType', 'data': { 'member1': 'str', 'member2': 'int', '*member3': 'str' } } -The use of '*' as a prefix to the name means the member is optional. Optional -members should always be added to the end of the dictionary to preserve -backwards compatibility. +The use of '*' as a prefix to the name means the member is optional. + +The default initialization value of an optional argument should not be changed +between versions of QEMU unless the new default maintains backward +compatibility to the user-visible behavior of the old default. + +With proper documentation, this policy still allows some flexibility; for +example, documenting that a default of 0 picks an optimal buffer size allows +one release to declare the optimal size at 512 while another release declares +the optimal size at 4096 - the user-visible behavior is not the bytes used by +the buffer, but the fact that the buffer was optimal size. + +On input structures (only mentioned in the 'data' side of a command), changing +from mandatory to optional is safe (older clients will supply the option, and +newer clients can benefit from the default); changing from optional to +mandatory is backwards incompatible (older clients may be omitting the option, +and must continue to work). + +On output structures (only mentioned in the 'returns' side of a command), +changing from mandatory to optional is in general unsafe (older clients may be +expecting the field, and could crash if it is missing), although it can be done +if the only way that the optional argument will be omitted is when it is +triggered by the presence of a new input flag to the command that older clients +don't know to send. Changing from optional to mandatory is safe. +A structure that is used in both input and output of various commands +must consider the backwards compatibility constraints of both directions +of use. A complex type definition can specify another complex type as its base. In this case, the fields of the base type are included as top-level fields @@ -221,7 +256,7 @@ created code. Example: mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-types.py \ - --output-dir="qapi-generated" --prefix="example-" < example-schema.json + --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 */ @@ -291,7 +326,7 @@ $(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-" < example-schema.json + --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 */ diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt index 3930a9ba70..4d86c2477b 100644 --- a/docs/writing-qmp-commands.txt +++ b/docs/writing-qmp-commands.txt @@ -308,12 +308,12 @@ Here's the implementation of the "hello-world" HMP command: void hmp_hello_world(Monitor *mon, const QDict *qdict) { const char *message = qdict_get_try_str(qdict, "message"); - Error *errp = NULL; + Error *err = NULL; - qmp_hello_world(!!message, message, &errp); - if (errp) { - monitor_printf(mon, "%s\n", error_get_pretty(errp)); - error_free(errp); + qmp_hello_world(!!message, message, &err); + if (err) { + monitor_printf(mon, "%s\n", error_get_pretty(err)); + error_free(err); return; } } @@ -328,7 +328,7 @@ There are three important points to be noticed: 2. hmp_hello_world() performs error checking. In this example we just print the error description to the user, but we could do more, like taking different actions depending on the error qmp_hello_world() returns -3. The "errp" variable must be initialized to NULL before performing the +3. The "err" variable must be initialized to NULL before performing the QMP call There's one last step to actually make the command available to monitor users, @@ -480,12 +480,12 @@ Here's the HMP counterpart of the query-alarm-clock command: void hmp_info_alarm_clock(Monitor *mon) { QemuAlarmClock *clock; - Error *errp = NULL; + Error *err = NULL; - clock = qmp_query_alarm_clock(&errp); - if (errp) { + clock = qmp_query_alarm_clock(&err); + if (err) { monitor_printf(mon, "Could not query alarm clock information\n"); - error_free(errp); + error_free(err); return; } @@ -631,12 +631,12 @@ has to traverse the list, it's shown below for reference: void hmp_info_alarm_methods(Monitor *mon) { TimerAlarmMethodList *method_list, *method; - Error *errp = NULL; + Error *err = NULL; - method_list = qmp_query_alarm_methods(&errp); - if (errp) { + method_list = qmp_query_alarm_methods(&err); + if (err) { monitor_printf(mon, "Could not query alarm methods\n"); - error_free(errp); + error_free(err); return; } @@ -86,7 +86,6 @@ typedef struct DumpState { bool has_filter; int64_t begin; int64_t length; - Error **errp; uint8_t *note_buf; /* buffer for notes */ size_t note_buf_offset; /* the writing place in note_buf */ @@ -1570,7 +1569,6 @@ static int dump_init(DumpState *s, int fd, bool has_format, nr_cpus++; } - s->errp = errp; s->fd = fd; s->has_filter = has_filter; s->begin = begin; @@ -1780,11 +1778,11 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin, } if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) { - if (create_kdump_vmcore(s) < 0 && !error_is_set(s->errp)) { + if (create_kdump_vmcore(s) < 0) { error_set(errp, QERR_IO_ERROR); } } else { - if (create_vmcore(s) < 0 && !error_is_set(s->errp)) { + if (create_vmcore(s) < 0) { error_set(errp, QERR_IO_ERROR); } } @@ -28,7 +28,8 @@ static void hmp_handle_error(Monitor *mon, Error **errp) { - if (error_is_set(errp)) { + assert(errp); + if (*errp) { monitor_printf(mon, "%s\n", error_get_pretty(*errp)); error_free(*errp); } @@ -754,10 +755,10 @@ void hmp_memsave(Monitor *mon, const QDict *qdict) uint32_t size = qdict_get_int(qdict, "size"); const char *filename = qdict_get_str(qdict, "filename"); uint64_t addr = qdict_get_int(qdict, "val"); - Error *errp = NULL; + Error *err = NULL; - qmp_memsave(addr, size, filename, true, monitor_get_cpu_index(), &errp); - hmp_handle_error(mon, &errp); + qmp_memsave(addr, size, filename, true, monitor_get_cpu_index(), &err); + hmp_handle_error(mon, &err); } void hmp_pmemsave(Monitor *mon, const QDict *qdict) @@ -765,21 +766,21 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict) uint32_t size = qdict_get_int(qdict, "size"); const char *filename = qdict_get_str(qdict, "filename"); uint64_t addr = qdict_get_int(qdict, "val"); - Error *errp = NULL; + Error *err = NULL; - qmp_pmemsave(addr, size, filename, &errp); - hmp_handle_error(mon, &errp); + qmp_pmemsave(addr, size, filename, &err); + hmp_handle_error(mon, &err); } void hmp_ringbuf_write(Monitor *mon, const QDict *qdict) { const char *chardev = qdict_get_str(qdict, "device"); const char *data = qdict_get_str(qdict, "data"); - Error *errp = NULL; + Error *err = NULL; - qmp_ringbuf_write(chardev, data, false, 0, &errp); + qmp_ringbuf_write(chardev, data, false, 0, &err); - hmp_handle_error(mon, &errp); + hmp_handle_error(mon, &err); } void hmp_ringbuf_read(Monitor *mon, const QDict *qdict) @@ -787,13 +788,13 @@ void hmp_ringbuf_read(Monitor *mon, const QDict *qdict) uint32_t size = qdict_get_int(qdict, "size"); const char *chardev = qdict_get_str(qdict, "device"); char *data; - Error *errp = NULL; + Error *err = NULL; int i; - data = qmp_ringbuf_read(chardev, size, false, 0, &errp); - if (errp) { - monitor_printf(mon, "%s\n", error_get_pretty(errp)); - error_free(errp); + data = qmp_ringbuf_read(chardev, size, false, 0, &err); + if (err) { + monitor_printf(mon, "%s\n", error_get_pretty(err)); + error_free(err); return; } @@ -828,7 +829,7 @@ static bool key_is_missing(const BlockInfo *bdev) void hmp_cont(Monitor *mon, const QDict *qdict) { BlockInfoList *bdev_list, *bdev; - Error *errp = NULL; + Error *err = NULL; bdev_list = qmp_query_block(NULL); for (bdev = bdev_list; bdev; bdev = bdev->next) { @@ -839,8 +840,8 @@ void hmp_cont(Monitor *mon, const QDict *qdict) } } - qmp_cont(&errp); - hmp_handle_error(mon, &errp); + qmp_cont(&err); + hmp_handle_error(mon, &err); out: qapi_free_BlockInfoList(bdev_list); @@ -853,41 +854,41 @@ void hmp_system_wakeup(Monitor *mon, const QDict *qdict) void hmp_inject_nmi(Monitor *mon, const QDict *qdict) { - Error *errp = NULL; + Error *err = NULL; - qmp_inject_nmi(&errp); - hmp_handle_error(mon, &errp); + qmp_inject_nmi(&err); + hmp_handle_error(mon, &err); } void hmp_set_link(Monitor *mon, const QDict *qdict) { const char *name = qdict_get_str(qdict, "name"); int up = qdict_get_bool(qdict, "up"); - Error *errp = NULL; + Error *err = NULL; - qmp_set_link(name, up, &errp); - hmp_handle_error(mon, &errp); + qmp_set_link(name, up, &err); + hmp_handle_error(mon, &err); } void hmp_block_passwd(Monitor *mon, const QDict *qdict) { const char *device = qdict_get_str(qdict, "device"); const char *password = qdict_get_str(qdict, "password"); - Error *errp = NULL; + Error *err = NULL; - qmp_block_passwd(true, device, false, NULL, password, &errp); - hmp_handle_error(mon, &errp); + qmp_block_passwd(true, device, false, NULL, password, &err); + hmp_handle_error(mon, &err); } void hmp_balloon(Monitor *mon, const QDict *qdict) { int64_t value = qdict_get_int(qdict, "value"); - Error *errp = NULL; + Error *err = NULL; - qmp_balloon(value, &errp); - if (errp) { - monitor_printf(mon, "balloon: %s\n", error_get_pretty(errp)); - error_free(errp); + qmp_balloon(value, &err); + if (err) { + monitor_printf(mon, "balloon: %s\n", error_get_pretty(err)); + error_free(err); } } @@ -895,10 +896,10 @@ void hmp_block_resize(Monitor *mon, const QDict *qdict) { const char *device = qdict_get_str(qdict, "device"); int64_t size = qdict_get_int(qdict, "size"); - Error *errp = NULL; + Error *err = NULL; - qmp_block_resize(true, device, false, NULL, size, &errp); - hmp_handle_error(mon, &errp); + qmp_block_resize(true, device, false, NULL, size, &err); + hmp_handle_error(mon, &err); } void hmp_drive_mirror(Monitor *mon, const QDict *qdict) @@ -909,11 +910,11 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict) int reuse = qdict_get_try_bool(qdict, "reuse", 0); int full = qdict_get_try_bool(qdict, "full", 0); enum NewImageMode mode; - Error *errp = NULL; + Error *err = NULL; if (!filename) { - error_set(&errp, QERR_MISSING_PARAMETER, "target"); - hmp_handle_error(mon, &errp); + error_set(&err, QERR_MISSING_PARAMETER, "target"); + hmp_handle_error(mon, &err); return; } @@ -926,8 +927,8 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict) qmp_drive_mirror(device, filename, !!format, format, full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP, true, mode, false, 0, false, 0, false, 0, - false, 0, false, 0, &errp); - hmp_handle_error(mon, &errp); + false, 0, false, 0, &err); + hmp_handle_error(mon, &err); } void hmp_drive_backup(Monitor *mon, const QDict *qdict) @@ -938,11 +939,11 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict) int reuse = qdict_get_try_bool(qdict, "reuse", 0); int full = qdict_get_try_bool(qdict, "full", 0); enum NewImageMode mode; - Error *errp = NULL; + Error *err = NULL; if (!filename) { - error_set(&errp, QERR_MISSING_PARAMETER, "target"); - hmp_handle_error(mon, &errp); + error_set(&err, QERR_MISSING_PARAMETER, "target"); + hmp_handle_error(mon, &err); return; } @@ -954,8 +955,8 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict) qmp_drive_backup(device, filename, !!format, format, full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP, - true, mode, false, 0, false, 0, false, 0, &errp); - hmp_handle_error(mon, &errp); + true, mode, false, 0, false, 0, false, 0, &err); + hmp_handle_error(mon, &err); } void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict) @@ -965,13 +966,13 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict) const char *format = qdict_get_try_str(qdict, "format"); int reuse = qdict_get_try_bool(qdict, "reuse", 0); enum NewImageMode mode; - Error *errp = NULL; + Error *err = NULL; if (!filename) { /* In the future, if 'snapshot-file' is not specified, the snapshot will be taken internally. Today it's actually required. */ - error_set(&errp, QERR_MISSING_PARAMETER, "snapshot-file"); - hmp_handle_error(mon, &errp); + error_set(&err, QERR_MISSING_PARAMETER, "snapshot-file"); + hmp_handle_error(mon, &err); return; } @@ -979,18 +980,18 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict) qmp_blockdev_snapshot_sync(true, device, false, NULL, filename, false, NULL, !!format, format, - true, mode, &errp); - hmp_handle_error(mon, &errp); + true, mode, &err); + hmp_handle_error(mon, &err); } void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict) { const char *device = qdict_get_str(qdict, "device"); const char *name = qdict_get_str(qdict, "name"); - Error *errp = NULL; + Error *err = NULL; - qmp_blockdev_snapshot_internal_sync(device, name, &errp); - hmp_handle_error(mon, &errp); + qmp_blockdev_snapshot_internal_sync(device, name, &err); + hmp_handle_error(mon, &err); } void hmp_snapshot_delete_blkdev_internal(Monitor *mon, const QDict *qdict) @@ -998,11 +999,11 @@ void hmp_snapshot_delete_blkdev_internal(Monitor *mon, const QDict *qdict) const char *device = qdict_get_str(qdict, "device"); const char *name = qdict_get_str(qdict, "name"); const char *id = qdict_get_try_str(qdict, "id"); - Error *errp = NULL; + Error *err = NULL; qmp_blockdev_snapshot_delete_internal_sync(device, !!id, id, - true, name, &errp); - hmp_handle_error(mon, &errp); + true, name, &err); + hmp_handle_error(mon, &err); } void hmp_migrate_cancel(Monitor *mon, const QDict *qdict) @@ -1310,7 +1311,7 @@ void hmp_device_del(Monitor *mon, const QDict *qdict) void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict) { - Error *errp = NULL; + Error *err = NULL; int paging = qdict_get_try_bool(qdict, "paging", 0); int zlib = qdict_get_try_bool(qdict, "zlib", 0); int lzo = qdict_get_try_bool(qdict, "lzo", 0); @@ -1324,8 +1325,8 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict) char *prot; if (zlib + lzo + snappy > 1) { - error_setg(&errp, "only one of '-z|-l|-s' can be set"); - hmp_handle_error(mon, &errp); + error_setg(&err, "only one of '-z|-l|-s' can be set"); + hmp_handle_error(mon, &err); return; } @@ -1351,8 +1352,8 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict) prot = g_strconcat("file:", file, NULL); qmp_dump_guest_memory(paging, prot, has_begin, begin, has_length, length, - true, dump_format, &errp); - hmp_handle_error(mon, &errp); + true, dump_format, &err); + hmp_handle_error(mon, &err); g_free(prot); } @@ -1444,19 +1445,19 @@ out: void hmp_getfd(Monitor *mon, const QDict *qdict) { const char *fdname = qdict_get_str(qdict, "fdname"); - Error *errp = NULL; + Error *err = NULL; - qmp_getfd(fdname, &errp); - hmp_handle_error(mon, &errp); + qmp_getfd(fdname, &err); + hmp_handle_error(mon, &err); } void hmp_closefd(Monitor *mon, const QDict *qdict) { const char *fdname = qdict_get_str(qdict, "fdname"); - Error *errp = NULL; + Error *err = NULL; - qmp_closefd(fdname, &errp); - hmp_handle_error(mon, &errp); + qmp_closefd(fdname, &err); + hmp_handle_error(mon, &err); } void hmp_send_key(Monitor *mon, const QDict *qdict) @@ -1606,10 +1607,10 @@ void hmp_nbd_server_add(Monitor *mon, const QDict *qdict) void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict) { - Error *errp = NULL; + Error *err = NULL; - qmp_nbd_server_stop(&errp); - hmp_handle_error(mon, &errp); + qmp_nbd_server_stop(&err); + hmp_handle_error(mon, &err); } void hmp_cpu_add(Monitor *mon, const QDict *qdict) diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c index a825871d8a..e55421adcd 100644 --- a/hw/i386/kvm/pci-assign.c +++ b/hw/i386/kvm/pci-assign.c @@ -394,9 +394,10 @@ static uint8_t pci_find_cap_offset(PCIDevice *d, uint8_t cap, uint8_t start) return 0; } -static int assigned_dev_register_regions(PCIRegion *io_regions, - unsigned long regions_num, - AssignedDevice *pci_dev) +static void assigned_dev_register_regions(PCIRegion *io_regions, + unsigned long regions_num, + AssignedDevice *pci_dev, + Error **errp) { uint32_t i; PCIRegion *cur_region = io_regions; @@ -425,9 +426,9 @@ static int assigned_dev_register_regions(PCIRegion *io_regions, if (pci_dev->v_addrs[i].u.r_virtbase == MAP_FAILED) { pci_dev->v_addrs[i].u.r_virtbase = NULL; - error_report("%s: Error: Couldn't mmap 0x%" PRIx64 "!", - __func__, cur_region->base_addr); - return -1; + error_setg_errno(errp, errno, "Couldn't mmap 0x%" PRIx64 "!", + cur_region->base_addr); + return; } pci_dev->v_addrs[i].r_size = cur_region->size; @@ -496,10 +497,10 @@ static int assigned_dev_register_regions(PCIRegion *io_regions, } /* success */ - return 0; } -static int get_real_id(const char *devpath, const char *idname, uint16_t *val) +static void get_real_id(const char *devpath, const char *idname, uint16_t *val, + Error **errp) { FILE *f; char name[128]; @@ -508,39 +509,39 @@ static int get_real_id(const char *devpath, const char *idname, uint16_t *val) snprintf(name, sizeof(name), "%s%s", devpath, idname); f = fopen(name, "r"); if (f == NULL) { - error_report("%s: %s: %m", __func__, name); - return -1; + error_setg_file_open(errp, errno, name); + return; } if (fscanf(f, "%li\n", &id) == 1) { *val = id; } else { - fclose(f); - return -1; + error_setg(errp, "Failed to parse contents of '%s'", name); } fclose(f); - - return 0; } -static int get_real_vendor_id(const char *devpath, uint16_t *val) +static void get_real_vendor_id(const char *devpath, uint16_t *val, + Error **errp) { - return get_real_id(devpath, "vendor", val); + get_real_id(devpath, "vendor", val, errp); } -static int get_real_device_id(const char *devpath, uint16_t *val) +static void get_real_device_id(const char *devpath, uint16_t *val, + Error **errp) { - return get_real_id(devpath, "device", val); + get_real_id(devpath, "device", val, errp); } -static int get_real_device(AssignedDevice *pci_dev) +static void get_real_device(AssignedDevice *pci_dev, Error **errp) { char dir[128], name[128]; - int fd, r = 0, v; + int fd, r = 0; FILE *f; uint64_t start, end, size, flags; uint16_t id; PCIRegion *rp; PCIDevRegions *dev = &pci_dev->real_device; + Error *local_err = NULL; dev->region_number = 0; @@ -551,16 +552,19 @@ static int get_real_device(AssignedDevice *pci_dev) snprintf(name, sizeof(name), "%sconfig", dir); if (pci_dev->configfd_name && *pci_dev->configfd_name) { - dev->config_fd = monitor_handle_fd_param(cur_mon, pci_dev->configfd_name); - if (dev->config_fd < 0) { - return 1; + dev->config_fd = monitor_handle_fd_param2(cur_mon, + pci_dev->configfd_name, + &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; } } else { dev->config_fd = open(name, O_RDWR); if (dev->config_fd == -1) { - error_report("%s: %s: %m", __func__, name); - return 1; + error_setg_file_open(errp, errno, name); + return; } } again: @@ -570,7 +574,10 @@ again: if (errno == EINTR || errno == EAGAIN) { goto again; } - error_report("%s: read failed, errno = %d", __func__, errno); + error_setg_errno(errp, errno, "read(\"%s\")", + (pci_dev->configfd_name && *pci_dev->configfd_name) ? + pci_dev->configfd_name : name); + return; } /* Restore or clear multifunction, this is always controlled by qemu */ @@ -590,8 +597,8 @@ again: f = fopen(name, "r"); if (f == NULL) { - error_report("%s: %s: %m", __func__, name); - return 1; + error_setg_file_open(errp, errno, name); + return; } for (r = 0; r < PCI_ROM_SLOT; r++) { @@ -634,17 +641,19 @@ again: fclose(f); /* read and fill vendor ID */ - v = get_real_vendor_id(dir, &id); - if (v) { - return 1; + get_real_vendor_id(dir, &id, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; } pci_dev->dev.config[0] = id & 0xff; pci_dev->dev.config[1] = (id & 0xff00) >> 8; /* read and fill device ID */ - v = get_real_device_id(dir, &id); - if (v) { - return 1; + get_real_device_id(dir, &id, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; } pci_dev->dev.config[2] = id & 0xff; pci_dev->dev.config[3] = (id & 0xff00) >> 8; @@ -653,7 +662,6 @@ again: PCI_COMMAND_MASTER | PCI_COMMAND_INTX_DISABLE); dev->region_number = r; - return 0; } static void free_msi_virqs(AssignedDevice *dev) @@ -726,11 +734,17 @@ static void free_assigned_device(AssignedDevice *dev) free_msi_virqs(dev); } -static void assign_failed_examine(AssignedDevice *dev) +/* This function tries to determine the cause of the PCI assignment failure. It + * always returns the cause as a dynamically allocated, human readable string. + * If the function fails to determine the cause for any internal reason, then + * the returned string will state that fact. + */ +static char *assign_failed_examine(const AssignedDevice *dev) { char name[PATH_MAX], dir[PATH_MAX], driver[PATH_MAX] = {}, *ns; uint16_t vendor_id, device_id; int r; + Error *local_err = NULL; snprintf(dir, sizeof(dir), "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/", dev->host.domain, dev->host.bus, dev->host.slot, @@ -751,13 +765,17 @@ static void assign_failed_examine(AssignedDevice *dev) ns++; - if (get_real_vendor_id(dir, &vendor_id) || - get_real_device_id(dir, &device_id)) { + if ((get_real_vendor_id(dir, &vendor_id, &local_err), local_err) || + (get_real_device_id(dir, &device_id, &local_err), local_err)) { + /* We're already analyzing an assignment error, so we suppress this + * one just like the others above. + */ + error_free(local_err); goto fail; } - error_printf("*** The driver '%s' is occupying your device " - "%04x:%02x:%02x.%x.\n" + return g_strdup_printf( + "*** The driver '%s' is occupying your device %04x:%02x:%02x.%x.\n" "***\n" "*** You can try the following commands to free it:\n" "***\n" @@ -773,13 +791,11 @@ static void assign_failed_examine(AssignedDevice *dev) ns, dev->host.domain, dev->host.bus, dev->host.slot, dev->host.function, vendor_id, device_id); - return; - fail: - error_report("Couldn't find out why."); + return g_strdup("Couldn't find out why."); } -static int assign_device(AssignedDevice *dev) +static void assign_device(AssignedDevice *dev, Error **errp) { uint32_t flags = KVM_DEV_ASSIGN_ENABLE_IOMMU; int r; @@ -787,15 +803,15 @@ static int assign_device(AssignedDevice *dev) /* Only pass non-zero PCI segment to capable module */ if (!kvm_check_extension(kvm_state, KVM_CAP_PCI_SEGMENT) && dev->host.domain) { - error_report("Can't assign device inside non-zero PCI segment " - "as this KVM module doesn't support it."); - return -ENODEV; + error_setg(errp, "Can't assign device inside non-zero PCI segment " + "as this KVM module doesn't support it."); + return; } if (!kvm_check_extension(kvm_state, KVM_CAP_IOMMU)) { - error_report("No IOMMU found. Unable to assign device \"%s\"", - dev->dev.qdev.id); - return -ENODEV; + error_setg(errp, "No IOMMU found. Unable to assign device \"%s\"", + dev->dev.qdev.id); + return; } if (dev->features & ASSIGNED_DEVICE_SHARE_INTX_MASK && @@ -805,36 +821,39 @@ static int assign_device(AssignedDevice *dev) r = kvm_device_pci_assign(kvm_state, &dev->host, flags, &dev->dev_id); if (r < 0) { - error_report("Failed to assign device \"%s\" : %s", - dev->dev.qdev.id, strerror(-r)); - switch (r) { - case -EBUSY: - assign_failed_examine(dev); + case -EBUSY: { + char *cause; + + cause = assign_failed_examine(dev); + error_setg_errno(errp, -r, "Failed to assign device \"%s\"\n%s", + dev->dev.qdev.id, cause); + g_free(cause); break; + } default: + error_setg_errno(errp, -r, "Failed to assign device \"%s\"", + dev->dev.qdev.id); break; } } - return r; } -static bool check_irqchip_in_kernel(void) +static void verify_irqchip_in_kernel(Error **errp) { if (kvm_irqchip_in_kernel()) { - return true; + return; } - error_report("pci-assign: error: requires KVM with in-kernel irqchip " - "enabled"); - return false; + error_setg(errp, "pci-assign requires KVM with in-kernel irqchip enabled"); } -static int assign_intx(AssignedDevice *dev) +static int assign_intx(AssignedDevice *dev, Error **errp) { AssignedIRQType new_type; PCIINTxRoute intx_route; bool intx_host_msi; int r; + Error *local_err = NULL; /* Interrupt PIN 0 means don't use INTx */ if (assigned_dev_pci_read_byte(&dev->dev, PCI_INTERRUPT_PIN) == 0) { @@ -842,7 +861,9 @@ static int assign_intx(AssignedDevice *dev) return 0; } - if (!check_irqchip_in_kernel()) { + verify_irqchip_in_kernel(&local_err); + if (local_err) { + error_propagate(errp, local_err); return -ENOTSUP; } @@ -905,10 +926,11 @@ retry: dev->features |= ASSIGNED_DEVICE_PREFER_MSI_MASK; goto retry; } - error_report("Failed to assign irq for \"%s\": %s", - dev->dev.qdev.id, strerror(-r)); - error_report("Perhaps you are assigning a device " - "that shares an IRQ with another device?"); + error_setg_errno(errp, -r, + "Failed to assign irq for \"%s\"\n" + "Perhaps you are assigning a device " + "that shares an IRQ with another device?", + dev->dev.qdev.id); return r; } @@ -934,8 +956,11 @@ static void assigned_dev_update_irq_routing(PCIDevice *dev) Error *err = NULL; int r; - r = assign_intx(assigned_dev); + r = assign_intx(assigned_dev, &err); if (r < 0) { + error_report("%s", error_get_pretty(err)); + error_free(err); + err = NULL; qdev_unplug(&dev->qdev, &err); assert(!err); } @@ -986,7 +1011,13 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev) assigned_dev->intx_route.irq = -1; assigned_dev->assigned_irq_type = ASSIGNED_IRQ_MSI; } else { - assign_intx(assigned_dev); + Error *local_err = NULL; + + assign_intx(assigned_dev, &local_err); + if (local_err) { + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); + } } } @@ -1128,7 +1159,13 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev) assigned_dev->intx_route.irq = -1; assigned_dev->assigned_irq_type = ASSIGNED_IRQ_MSIX; } else { - assign_intx(assigned_dev); + Error *local_err = NULL; + + assign_intx(assigned_dev, &local_err); + if (local_err) { + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); + } } } @@ -1214,11 +1251,12 @@ static void assigned_dev_setup_cap_read(AssignedDevice *dev, uint32_t offset, assigned_dev_emulate_config_read(dev, offset + PCI_CAP_LIST_NEXT, 1); } -static int assigned_device_pci_cap_init(PCIDevice *pci_dev) +static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp) { AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev); PCIRegion *pci_region = dev->real_device.regions; int ret, pos; + Error *local_err = NULL; /* Clear initial capabilities pointer and status copied from hw */ pci_set_byte(pci_dev->config + PCI_CAPABILITY_LIST, 0); @@ -1230,13 +1268,17 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) * MSI capability is the 1st capability in capability config */ pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0); if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ)) { - if (!check_irqchip_in_kernel()) { + verify_irqchip_in_kernel(&local_err); + if (local_err) { + error_propagate(errp, local_err); return -ENOTSUP; } dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI; /* Only 32-bit/no-mask currently supported */ - ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSI, pos, 10); + ret = pci_add_capability2(pci_dev, PCI_CAP_ID_MSI, pos, 10, + &local_err); if (ret < 0) { + error_propagate(errp, local_err); return ret; } pci_dev->msi_cap = pos; @@ -1259,12 +1301,16 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) int bar_nr; uint32_t msix_table_entry; - if (!check_irqchip_in_kernel()) { + verify_irqchip_in_kernel(&local_err); + if (local_err) { + error_propagate(errp, local_err); return -ENOTSUP; } dev->cap.available |= ASSIGNED_DEVICE_CAP_MSIX; - ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSIX, pos, 12); + ret = pci_add_capability2(pci_dev, PCI_CAP_ID_MSIX, pos, 12, + &local_err); if (ret < 0) { + error_propagate(errp, local_err); return ret; } pci_dev->msix_cap = pos; @@ -1291,8 +1337,10 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) if (pos) { uint16_t pmc; - ret = pci_add_capability(pci_dev, PCI_CAP_ID_PM, pos, PCI_PM_SIZEOF); + ret = pci_add_capability2(pci_dev, PCI_CAP_ID_PM, pos, PCI_PM_SIZEOF, + &local_err); if (ret < 0) { + error_propagate(errp, local_err); return ret; } @@ -1330,8 +1378,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) */ size = MIN(0x3c, PCI_CONFIG_SPACE_SIZE - pos); if (size < 0x34) { - error_report("%s: Invalid size PCIe cap-id 0x%x", - __func__, PCI_CAP_ID_EXP); + error_setg(errp, "Invalid size PCIe cap-id 0x%x", + PCI_CAP_ID_EXP); return -EINVAL; } else if (size != 0x3c) { error_report("WARNING, %s: PCIe cap-id 0x%x has " @@ -1352,13 +1400,15 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) } if (size == 0) { - error_report("%s: Unsupported PCI express capability version %d", - __func__, version); + error_setg(errp, "Unsupported PCI express capability version %d", + version); return -EINVAL; } - ret = pci_add_capability(pci_dev, PCI_CAP_ID_EXP, pos, size); + ret = pci_add_capability2(pci_dev, PCI_CAP_ID_EXP, pos, size, + &local_err); if (ret < 0) { + error_propagate(errp, local_err); return ret; } @@ -1368,8 +1418,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) type = (type & PCI_EXP_FLAGS_TYPE) >> 4; if (type != PCI_EXP_TYPE_ENDPOINT && type != PCI_EXP_TYPE_LEG_END && type != PCI_EXP_TYPE_RC_END) { - error_report("Device assignment only supports endpoint assignment," - " device type %d", type); + error_setg(errp, "Device assignment only supports endpoint " + "assignment, device type %d", type); return -EINVAL; } @@ -1431,8 +1481,10 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) uint32_t status; /* Only expose the minimum, 8 byte capability */ - ret = pci_add_capability(pci_dev, PCI_CAP_ID_PCIX, pos, 8); + ret = pci_add_capability2(pci_dev, PCI_CAP_ID_PCIX, pos, 8, + &local_err); if (ret < 0) { + error_propagate(errp, local_err); return ret; } @@ -1457,8 +1509,10 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_VPD, 0); if (pos) { /* Direct R/W passthrough */ - ret = pci_add_capability(pci_dev, PCI_CAP_ID_VPD, pos, 8); + ret = pci_add_capability2(pci_dev, PCI_CAP_ID_VPD, pos, 8, + &local_err); if (ret < 0) { + error_propagate(errp, local_err); return ret; } @@ -1473,8 +1527,10 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) pos += PCI_CAP_LIST_NEXT) { uint8_t len = pci_get_byte(pci_dev->config + pos + PCI_CAP_FLAGS); /* Direct R/W passthrough */ - ret = pci_add_capability(pci_dev, PCI_CAP_ID_VNDR, pos, len); + ret = pci_add_capability2(pci_dev, PCI_CAP_ID_VNDR, pos, len, + &local_err); if (ret < 0) { + error_propagate(errp, local_err); return ret; } @@ -1602,20 +1658,19 @@ static void assigned_dev_msix_reset(AssignedDevice *dev) } } -static int assigned_dev_register_msix_mmio(AssignedDevice *dev) +static void assigned_dev_register_msix_mmio(AssignedDevice *dev, Error **errp) { dev->msix_table = mmap(NULL, MSIX_PAGE_SIZE, PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE, 0, 0); if (dev->msix_table == MAP_FAILED) { - error_report("fail allocate msix_table! %s", strerror(errno)); - return -EFAULT; + error_setg_errno(errp, errno, "failed to allocate msix_table"); + return; } assigned_dev_msix_reset(dev); memory_region_init_io(&dev->mmio, OBJECT(dev), &assigned_dev_msix_mmio_ops, dev, "assigned-dev-msix", MSIX_PAGE_SIZE); - return 0; } static void assigned_dev_unregister_msix_mmio(AssignedDevice *dev) @@ -1698,16 +1753,17 @@ static int assigned_initfn(struct PCIDevice *pci_dev) AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev); uint8_t e_intx; int r; + Error *local_err = NULL; if (!kvm_enabled()) { - error_report("pci-assign: error: requires KVM support"); - return -1; + error_setg(&local_err, "pci-assign requires KVM support"); + goto exit_with_error; } if (!dev->host.domain && !dev->host.bus && !dev->host.slot && !dev->host.function) { - error_report("pci-assign: error: no host device specified"); - return -1; + error_setg(&local_err, "no host device specified"); + goto exit_with_error; } /* @@ -1730,27 +1786,28 @@ static int assigned_initfn(struct PCIDevice *pci_dev) memcpy(dev->emulate_config_write, dev->emulate_config_read, sizeof(dev->emulate_config_read)); - if (get_real_device(dev)) { - error_report("pci-assign: Error: Couldn't get real device (%s)!", - dev->dev.qdev.id); + get_real_device(dev, &local_err); + if (local_err) { goto out; } - if (assigned_device_pci_cap_init(pci_dev) < 0) { + if (assigned_device_pci_cap_init(pci_dev, &local_err) < 0) { goto out; } /* intercept MSI-X entry page in the MMIO */ if (dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX) { - if (assigned_dev_register_msix_mmio(dev)) { + assigned_dev_register_msix_mmio(dev, &local_err); + if (local_err) { goto out; } } /* handle real device's MMIO/PIO BARs */ - if (assigned_dev_register_regions(dev->real_device.regions, - dev->real_device.region_number, - dev)) { + assigned_dev_register_regions(dev->real_device.regions, + dev->real_device.region_number, dev, + &local_err); + if (local_err) { goto out; } @@ -1761,13 +1818,13 @@ static int assigned_initfn(struct PCIDevice *pci_dev) dev->intx_route.irq = -1; /* assign device to guest */ - r = assign_device(dev); - if (r < 0) { + assign_device(dev, &local_err); + if (local_err) { goto out; } /* assign legacy INTx to the device */ - r = assign_intx(dev); + r = assign_intx(dev, &local_err); if (r < 0) { goto assigned_out; } @@ -1780,8 +1837,14 @@ static int assigned_initfn(struct PCIDevice *pci_dev) assigned_out: deassign_device(dev); + out: free_assigned_device(dev); + +exit_with_error: + assert(local_err); + qerror_report_err(local_err); + error_free(local_err); return -1; } diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 517ff2a21b..22fe5eec36 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -2013,12 +2013,32 @@ static void pci_del_option_rom(PCIDevice *pdev) int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t offset, uint8_t size) { + int ret; + Error *local_err = NULL; + + ret = pci_add_capability2(pdev, cap_id, offset, size, &local_err); + if (local_err) { + assert(ret < 0); + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); + } else { + /* success implies a positive offset in config space */ + assert(ret > 0); + } + return ret; +} + +int pci_add_capability2(PCIDevice *pdev, uint8_t cap_id, + uint8_t offset, uint8_t size, + Error **errp) +{ uint8_t *config; int i, overlapping_cap; if (!offset) { offset = pci_find_space(pdev, size); if (!offset) { + error_setg(errp, "out of PCI config space"); return -ENOSPC; } } else { @@ -2029,12 +2049,12 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, for (i = offset; i < offset + size; i++) { overlapping_cap = pci_find_capability_at_offset(pdev, i); if (overlapping_cap) { - fprintf(stderr, "ERROR: %s:%02x:%02x.%x " - "Attempt to add PCI capability %x at offset " - "%x overlaps existing capability %x at offset %x\n", - pci_root_bus_path(pdev), pci_bus_num(pdev->bus), - PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), - cap_id, offset, overlapping_cap, i); + error_setg(errp, "%s:%02x:%02x.%x " + "Attempt to add PCI capability %x at offset " + "%x overlaps existing capability %x at offset %x", + pci_root_bus_path(pdev), pci_bus_num(pdev->bus), + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), + cap_id, offset, overlapping_cap, i); return -EINVAL; } } diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 693dd6b658..8c25ae5d1d 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -6,6 +6,7 @@ #include "hw/qdev.h" #include "exec/memory.h" #include "sysemu/dma.h" +#include "qapi/error.h" /* PCI includes legacy ISA access. */ #include "hw/isa/isa.h" @@ -308,6 +309,9 @@ pcibus_t pci_get_bar_addr(PCIDevice *pci_dev, int region_num); int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t offset, uint8_t size); +int pci_add_capability2(PCIDevice *pdev, uint8_t cap_id, + uint8_t offset, uint8_t size, + Error **errp); void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size); diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h index 42d867155b..1c1f56f36b 100644 --- a/include/monitor/monitor.h +++ b/include/monitor/monitor.h @@ -75,6 +75,7 @@ int monitor_read_block_device_key(Monitor *mon, const char *device, int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp); int monitor_handle_fd_param(Monitor *mon, const char *fdname); +int monitor_handle_fd_param2(Monitor *mon, const char *fdname, Error **errp); void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0); diff --git a/include/qapi/error.h b/include/qapi/error.h index c0f0c3b432..79958011db 100644 --- a/include/qapi/error.h +++ b/include/qapi/error.h @@ -27,14 +27,16 @@ typedef struct Error Error; * printf-style human message. This function is not meant to be used outside * of QEMU. */ -void error_set(Error **err, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(3, 4); +void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) + GCC_FMT_ATTR(3, 4); /** * Set an indirect pointer to an error given a ErrorClass value and a * printf-style human message, followed by a strerror() string if * @os_error is not zero. */ -void error_set_errno(Error **err, int os_error, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(4, 5); +void error_set_errno(Error **errp, int os_error, ErrorClass err_class, + const char *fmt, ...) GCC_FMT_ATTR(4, 5); #ifdef _WIN32 /** @@ -42,19 +44,22 @@ void error_set_errno(Error **err, int os_error, ErrorClass err_class, const char * printf-style human message, followed by a g_win32_error_message() string if * @win32_err is not zero. */ -void error_set_win32(Error **err, int win32_err, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(4, 5); +void error_set_win32(Error **errp, int win32_err, ErrorClass err_class, + const char *fmt, ...) GCC_FMT_ATTR(4, 5); #endif /** * Same as error_set(), but sets a generic error */ -#define error_setg(err, fmt, ...) \ - error_set(err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__) -#define error_setg_errno(err, os_error, fmt, ...) \ - error_set_errno(err, os_error, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__) +#define error_setg(errp, fmt, ...) \ + error_set(errp, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__) +#define error_setg_errno(errp, os_error, fmt, ...) \ + error_set_errno(errp, os_error, ERROR_CLASS_GENERIC_ERROR, \ + fmt, ## __VA_ARGS__) #ifdef _WIN32 -#define error_setg_win32(err, win32_err, fmt, ...) \ - error_set_win32(err, win32_err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__) +#define error_setg_win32(errp, win32_err, fmt, ...) \ + error_set_win32(errp, win32_err, ERROR_CLASS_GENERIC_ERROR, \ + fmt, ## __VA_ARGS__) #endif /** @@ -66,7 +71,7 @@ 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 **err); +bool error_is_set(Error **errp); /* * Get the error class of an error object. @@ -88,7 +93,7 @@ const char *error_get_pretty(Error *err); * always transfer ownership of the error reference and handles the case where * dst_err is NULL correctly. Errors after the first are discarded. */ -void error_propagate(Error **dst_err, Error *local_err); +void error_propagate(Error **dst_errp, Error *local_err); /** * Free an error object. diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h index cea38181bf..e389697f19 100644 --- a/include/qapi/qmp/dispatch.h +++ b/include/qapi/qmp/dispatch.h @@ -50,7 +50,7 @@ void qmp_enable_command(const char *name); bool qmp_command_is_enabled(const QmpCommand *cmd); const char *qmp_command_name(const QmpCommand *cmd); bool qmp_has_success_response(const QmpCommand *cmd); -QObject *qmp_build_error_object(Error *errp); +QObject *qmp_build_error_object(Error *err); typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque); void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque); @@ -2611,16 +2611,33 @@ int monitor_handle_fd_param(Monitor *mon, const char *fdname) int fd; Error *local_err = NULL; - if (!qemu_isdigit(fdname[0]) && mon) { + fd = monitor_handle_fd_param2(mon, fdname, &local_err); + if (local_err) { + qerror_report_err(local_err); + error_free(local_err); + } + return fd; +} + +int monitor_handle_fd_param2(Monitor *mon, const char *fdname, Error **errp) +{ + int fd; + Error *local_err = NULL; + if (!qemu_isdigit(fdname[0]) && mon) { fd = monitor_get_fd(mon, fdname, &local_err); + } else { + fd = qemu_parse_fd(fdname); if (fd == -1) { - qerror_report_err(local_err); - error_free(local_err); - return -1; + error_setg(&local_err, "Invalid file descriptor number '%s'", + fdname); } + } + if (local_err) { + error_propagate(errp, local_err); + assert(fd == -1); } else { - fd = qemu_parse_fd(fdname); + assert(fd != -1); } return fd; diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 5d830a2b56..87c1c789c9 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -472,13 +472,14 @@ opts_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp) val = strtosz_suffix(opt->str ? opt->str : "", &endptr, STRTOSZ_DEFSUFFIX_B); - if (val != -1 && *endptr == '\0') { - *obj = val; - processed(ov, name); + if (val < 0 || *endptr) { + error_set(errp, QERR_INVALID_PARAMETER_VALUE, opt->name, + "a size value representible as a non-negative int64"); return; } - error_set(errp, QERR_INVALID_PARAMETER_VALUE, opt->name, - "a size value representible as a non-negative int64"); + + *obj = val; + processed(ov, name); } diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c index d0ea118fe3..dc53545fa5 100644 --- a/qapi/qapi-dealloc-visitor.c +++ b/qapi/qapi-dealloc-visitor.c @@ -131,7 +131,9 @@ static void qapi_dealloc_end_list(Visitor *v, Error **errp) static void qapi_dealloc_type_str(Visitor *v, char **obj, const char *name, Error **errp) { - g_free(*obj); + if (obj) { + g_free(*obj); + } } static void qapi_dealloc_type_int(Visitor *v, int64_t *obj, const char *name, diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c index 9c614494f1..168b083c87 100644 --- a/qapi/qmp-dispatch.c +++ b/qapi/qmp-dispatch.c @@ -62,14 +62,14 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp) static QObject *do_qmp_dispatch(QObject *request, Error **errp) { + Error *local_err = NULL; const char *command; QDict *args, *dict; QmpCommand *cmd; QObject *ret = NULL; - dict = qmp_dispatch_check_obj(request, errp); - if (!dict || error_is_set(errp)) { + if (!dict) { return NULL; } @@ -94,13 +94,13 @@ static QObject *do_qmp_dispatch(QObject *request, Error **errp) switch (cmd->type) { case QCT_NORMAL: - cmd->fn(args, &ret, errp); - if (!error_is_set(errp)) { - if (cmd->options & QCO_NO_SUCCESS_RESP) { - g_assert(!ret); - } else if (!ret) { - ret = QOBJECT(qdict_new()); - } + cmd->fn(args, &ret, &local_err); + if (local_err) { + error_propagate(errp, local_err); + } else if (cmd->options & QCO_NO_SUCCESS_RESP) { + g_assert(!ret); + } else if (!ret) { + ret = QOBJECT(qdict_new()); } break; } @@ -110,11 +110,11 @@ static QObject *do_qmp_dispatch(QObject *request, Error **errp) return ret; } -QObject *qmp_build_error_object(Error *errp) +QObject *qmp_build_error_object(Error *err) { return qobject_from_jsonf("{ 'class': %s, 'desc': %s }", - ErrorClass_lookup[error_get_class(errp)], - error_get_pretty(errp)); + ErrorClass_lookup[error_get_class(err)], + error_get_pretty(err)); } QObject *qmp_dispatch(QObject *request) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 935a4ec14d..34ddba0531 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -53,7 +53,7 @@ extern char **environ; #endif #endif -static void ga_wait_child(pid_t pid, int *status, Error **err) +static void ga_wait_child(pid_t pid, int *status, Error **errp) { pid_t rpid; @@ -64,14 +64,15 @@ static void ga_wait_child(pid_t pid, int *status, Error **err) } while (rpid == -1 && errno == EINTR); if (rpid == -1) { - error_setg_errno(err, errno, "failed to wait for child (pid: %d)", pid); + error_setg_errno(errp, errno, "failed to wait for child (pid: %d)", + pid); return; } g_assert(rpid == pid); } -void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err) +void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp) { const char *shutdown_flag; Error *local_err = NULL; @@ -86,7 +87,7 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err) } else if (strcmp(mode, "reboot") == 0) { shutdown_flag = "-r"; } else { - error_setg(err, + error_setg(errp, "mode is invalid (valid values are: halt|powerdown|reboot"); return; } @@ -103,23 +104,23 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err) "hypervisor initiated shutdown", (char*)NULL, environ); _exit(EXIT_FAILURE); } else if (pid < 0) { - error_setg_errno(err, errno, "failed to create child process"); + error_setg_errno(errp, errno, "failed to create child process"); return; } ga_wait_child(pid, &status, &local_err); if (local_err) { - error_propagate(err, local_err); + error_propagate(errp, local_err); return; } if (!WIFEXITED(status)) { - error_setg(err, "child process has terminated abnormally"); + error_setg(errp, "child process has terminated abnormally"); return; } if (WEXITSTATUS(status)) { - error_setg(err, "child process has failed to shutdown"); + error_setg(errp, "child process has failed to shutdown"); return; } @@ -222,8 +223,8 @@ static int64_t guest_file_handle_add(FILE *fh, Error **errp) int64_t handle; handle = ga_get_fd_handle(ga_state, errp); - if (error_is_set(errp)) { - return 0; + if (handle < 0) { + return -1; } gfh = g_malloc0(sizeof(GuestFileHandle)); @@ -234,7 +235,7 @@ static int64_t guest_file_handle_add(FILE *fh, Error **errp) return handle; } -static GuestFileHandle *guest_file_handle_find(int64_t id, Error **err) +static GuestFileHandle *guest_file_handle_find(int64_t id, Error **errp) { GuestFileHandle *gfh; @@ -245,7 +246,7 @@ static GuestFileHandle *guest_file_handle_find(int64_t id, Error **err) } } - error_setg(err, "handle '%" PRId64 "' has not been found", id); + error_setg(errp, "handle '%" PRId64 "' has not been found", id); return NULL; } @@ -275,7 +276,7 @@ static const struct { }; static int -find_open_flag(const char *mode_str, Error **err) +find_open_flag(const char *mode_str, Error **errp) { unsigned mode; @@ -292,7 +293,7 @@ find_open_flag(const char *mode_str, Error **err) } if (mode == ARRAY_SIZE(guest_file_open_modes)) { - error_setg(err, "invalid file open mode '%s'", mode_str); + error_setg(errp, "invalid file open mode '%s'", mode_str); return -1; } return guest_file_open_modes[mode].oflag_base | O_NOCTTY | O_NONBLOCK; @@ -303,7 +304,7 @@ find_open_flag(const char *mode_str, Error **err) S_IROTH | S_IWOTH) static FILE * -safe_open_or_create(const char *path, const char *mode, Error **err) +safe_open_or_create(const char *path, const char *mode, Error **errp) { Error *local_err = NULL; int oflag; @@ -370,11 +371,12 @@ safe_open_or_create(const char *path, const char *mode, Error **err) } } - error_propagate(err, local_err); + error_propagate(errp, local_err); return NULL; } -int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, Error **err) +int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, + Error **errp) { FILE *fh; Error *local_err = NULL; @@ -387,7 +389,7 @@ int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, E slog("guest-file-open called, filepath: %s, mode: %s", path, mode); fh = safe_open_or_create(path, mode, &local_err); if (local_err != NULL) { - error_propagate(err, local_err); + error_propagate(errp, local_err); return -1; } @@ -398,14 +400,14 @@ int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, E ret = fcntl(fd, F_GETFL); ret = fcntl(fd, F_SETFL, ret | O_NONBLOCK); if (ret == -1) { - error_setg_errno(err, errno, "failed to make file '%s' non-blocking", + error_setg_errno(errp, errno, "failed to make file '%s' non-blocking", path); fclose(fh); return -1; } - handle = guest_file_handle_add(fh, err); - if (error_is_set(err)) { + handle = guest_file_handle_add(fh, errp); + if (handle < 0) { fclose(fh); return -1; } @@ -414,9 +416,9 @@ int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, E return handle; } -void qmp_guest_file_close(int64_t handle, Error **err) +void qmp_guest_file_close(int64_t handle, Error **errp) { - GuestFileHandle *gfh = guest_file_handle_find(handle, err); + GuestFileHandle *gfh = guest_file_handle_find(handle, errp); int ret; slog("guest-file-close called, handle: %" PRId64, handle); @@ -426,7 +428,7 @@ void qmp_guest_file_close(int64_t handle, Error **err) ret = fclose(gfh->fh); if (ret == EOF) { - error_setg_errno(err, errno, "failed to close handle"); + error_setg_errno(errp, errno, "failed to close handle"); return; } @@ -435,9 +437,9 @@ void qmp_guest_file_close(int64_t handle, Error **err) } struct GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count, - int64_t count, Error **err) + int64_t count, Error **errp) { - GuestFileHandle *gfh = guest_file_handle_find(handle, err); + GuestFileHandle *gfh = guest_file_handle_find(handle, errp); GuestFileRead *read_data = NULL; guchar *buf; FILE *fh; @@ -450,7 +452,7 @@ struct GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count, if (!has_count) { count = QGA_READ_COUNT_DEFAULT; } else if (count < 0) { - error_setg(err, "value '%" PRId64 "' is invalid for argument count", + error_setg(errp, "value '%" PRId64 "' is invalid for argument count", count); return NULL; } @@ -459,7 +461,7 @@ struct GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count, buf = g_malloc0(count+1); read_count = fread(buf, 1, count, fh); if (ferror(fh)) { - error_setg_errno(err, errno, "failed to read file"); + error_setg_errno(errp, errno, "failed to read file"); slog("guest-file-read failed, handle: %" PRId64, handle); } else { buf[read_count] = 0; @@ -477,13 +479,14 @@ struct GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count, } GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64, - bool has_count, int64_t count, Error **err) + bool has_count, int64_t count, + Error **errp) { GuestFileWrite *write_data = NULL; guchar *buf; gsize buf_len; int write_count; - GuestFileHandle *gfh = guest_file_handle_find(handle, err); + GuestFileHandle *gfh = guest_file_handle_find(handle, errp); FILE *fh; if (!gfh) { @@ -496,7 +499,7 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64, if (!has_count) { count = buf_len; } else if (count < 0 || count > buf_len) { - error_setg(err, "value '%" PRId64 "' is invalid for argument count", + error_setg(errp, "value '%" PRId64 "' is invalid for argument count", count); g_free(buf); return NULL; @@ -504,7 +507,7 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64, write_count = fwrite(buf, 1, count, fh); if (ferror(fh)) { - error_setg_errno(err, errno, "failed to write to file"); + error_setg_errno(errp, errno, "failed to write to file"); slog("guest-file-write failed, handle: %" PRId64, handle); } else { write_data = g_malloc0(sizeof(GuestFileWrite)); @@ -518,9 +521,9 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64, } struct GuestFileSeek *qmp_guest_file_seek(int64_t handle, int64_t offset, - int64_t whence, Error **err) + int64_t whence, Error **errp) { - GuestFileHandle *gfh = guest_file_handle_find(handle, err); + GuestFileHandle *gfh = guest_file_handle_find(handle, errp); GuestFileSeek *seek_data = NULL; FILE *fh; int ret; @@ -532,7 +535,7 @@ struct GuestFileSeek *qmp_guest_file_seek(int64_t handle, int64_t offset, fh = gfh->fh; ret = fseek(fh, offset, whence); if (ret == -1) { - error_setg_errno(err, errno, "failed to seek file"); + error_setg_errno(errp, errno, "failed to seek file"); } else { seek_data = g_new0(GuestFileSeek, 1); seek_data->position = ftell(fh); @@ -543,9 +546,9 @@ struct GuestFileSeek *qmp_guest_file_seek(int64_t handle, int64_t offset, return seek_data; } -void qmp_guest_file_flush(int64_t handle, Error **err) +void qmp_guest_file_flush(int64_t handle, Error **errp) { - GuestFileHandle *gfh = guest_file_handle_find(handle, err); + GuestFileHandle *gfh = guest_file_handle_find(handle, errp); FILE *fh; int ret; @@ -556,7 +559,7 @@ void qmp_guest_file_flush(int64_t handle, Error **err) fh = gfh->fh; ret = fflush(fh); if (ret == EOF) { - error_setg_errno(err, errno, "failed to flush file"); + error_setg_errno(errp, errno, "failed to flush file"); } } @@ -596,7 +599,7 @@ static void free_fs_mount_list(FsMountList *mounts) /* * Walk the mount table and build a list of local file systems */ -static void build_fs_mount_list(FsMountList *mounts, Error **err) +static void build_fs_mount_list(FsMountList *mounts, Error **errp) { struct mntent *ment; FsMount *mount; @@ -605,7 +608,7 @@ static void build_fs_mount_list(FsMountList *mounts, Error **err) fp = setmntent(mtab, "r"); if (!fp) { - error_setg(err, "failed to open mtab file: '%s'", mtab); + error_setg(errp, "failed to open mtab file: '%s'", mtab); return; } @@ -645,7 +648,7 @@ const char *fsfreeze_hook_arg_string[] = { "freeze", }; -static void execute_fsfreeze_hook(FsfreezeHookArg arg, Error **err) +static void execute_fsfreeze_hook(FsfreezeHookArg arg, Error **errp) { int status; pid_t pid; @@ -658,7 +661,7 @@ static void execute_fsfreeze_hook(FsfreezeHookArg arg, Error **err) return; } if (access(hook, X_OK) != 0) { - error_setg_errno(err, errno, "can't access fsfreeze hook '%s'", hook); + error_setg_errno(errp, errno, "can't access fsfreeze hook '%s'", hook); return; } @@ -673,24 +676,24 @@ static void execute_fsfreeze_hook(FsfreezeHookArg arg, Error **err) execle(hook, hook, arg_str, NULL, environ); _exit(EXIT_FAILURE); } else if (pid < 0) { - error_setg_errno(err, errno, "failed to create child process"); + error_setg_errno(errp, errno, "failed to create child process"); return; } ga_wait_child(pid, &status, &local_err); if (local_err) { - error_propagate(err, local_err); + error_propagate(errp, local_err); return; } if (!WIFEXITED(status)) { - error_setg(err, "fsfreeze hook has terminated abnormally"); + error_setg(errp, "fsfreeze hook has terminated abnormally"); return; } status = WEXITSTATUS(status); if (status) { - error_setg(err, "fsfreeze hook has failed with status %d", status); + error_setg(errp, "fsfreeze hook has failed with status %d", status); return; } } @@ -698,7 +701,7 @@ static void execute_fsfreeze_hook(FsfreezeHookArg arg, Error **err) /* * Return status of freeze/thaw */ -GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err) +GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp) { if (ga_is_frozen(ga_state)) { return GUEST_FSFREEZE_STATUS_FROZEN; @@ -711,7 +714,7 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err) * Walk list of mounted file systems in the guest, and freeze the ones which * are real local file systems. */ -int64_t qmp_guest_fsfreeze_freeze(Error **err) +int64_t qmp_guest_fsfreeze_freeze(Error **errp) { int ret = 0, i = 0; FsMountList mounts; @@ -723,14 +726,14 @@ int64_t qmp_guest_fsfreeze_freeze(Error **err) execute_fsfreeze_hook(FSFREEZE_HOOK_FREEZE, &local_err); if (local_err) { - error_propagate(err, local_err); + error_propagate(errp, local_err); return -1; } QTAILQ_INIT(&mounts); build_fs_mount_list(&mounts, &local_err); if (local_err) { - error_propagate(err, local_err); + error_propagate(errp, local_err); return -1; } @@ -740,7 +743,7 @@ int64_t qmp_guest_fsfreeze_freeze(Error **err) QTAILQ_FOREACH_REVERSE(mount, &mounts, FsMountList, next) { fd = qemu_open(mount->dirname, O_RDONLY); if (fd == -1) { - error_setg_errno(err, errno, "failed to open %s", mount->dirname); + error_setg_errno(errp, errno, "failed to open %s", mount->dirname); goto error; } @@ -756,7 +759,7 @@ int64_t qmp_guest_fsfreeze_freeze(Error **err) ret = ioctl(fd, FIFREEZE); if (ret == -1) { if (errno != EOPNOTSUPP) { - error_setg_errno(err, errno, "failed to freeze %s", + error_setg_errno(errp, errno, "failed to freeze %s", mount->dirname); close(fd); goto error; @@ -779,7 +782,7 @@ error: /* * Walk list of frozen file systems in the guest, and thaw them. */ -int64_t qmp_guest_fsfreeze_thaw(Error **err) +int64_t qmp_guest_fsfreeze_thaw(Error **errp) { int ret; FsMountList mounts; @@ -790,7 +793,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err) QTAILQ_INIT(&mounts); build_fs_mount_list(&mounts, &local_err); if (local_err) { - error_propagate(err, local_err); + error_propagate(errp, local_err); return 0; } @@ -829,7 +832,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err) ga_unset_frozen(ga_state); free_fs_mount_list(&mounts); - execute_fsfreeze_hook(FSFREEZE_HOOK_THAW, err); + execute_fsfreeze_hook(FSFREEZE_HOOK_THAW, errp); return i; } @@ -853,7 +856,7 @@ static void guest_fsfreeze_cleanup(void) /* * Walk list of mounted file systems in the guest, and trim them. */ -void qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **err) +void qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp) { int ret = 0; FsMountList mounts; @@ -871,14 +874,14 @@ void qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **err) QTAILQ_INIT(&mounts); build_fs_mount_list(&mounts, &local_err); if (local_err) { - error_propagate(err, local_err); + error_propagate(errp, local_err); return; } QTAILQ_FOREACH(mount, &mounts, next) { fd = qemu_open(mount->dirname, O_RDONLY); if (fd == -1) { - error_setg_errno(err, errno, "failed to open %s", mount->dirname); + error_setg_errno(errp, errno, "failed to open %s", mount->dirname); goto error; } @@ -891,7 +894,7 @@ void qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **err) ret = ioctl(fd, FITRIM, &r); if (ret == -1) { if (errno != ENOTTY && errno != EOPNOTSUPP) { - error_setg_errno(err, errno, "failed to trim %s", + error_setg_errno(errp, errno, "failed to trim %s", mount->dirname); close(fd); goto error; @@ -911,7 +914,7 @@ error: #define SUSPEND_NOT_SUPPORTED 1 static void bios_supports_mode(const char *pmutils_bin, const char *pmutils_arg, - const char *sysfile_str, Error **err) + const char *sysfile_str, Error **errp) { Error *local_err = NULL; char *pmutils_path; @@ -961,18 +964,18 @@ static void bios_supports_mode(const char *pmutils_bin, const char *pmutils_arg, _exit(SUSPEND_NOT_SUPPORTED); } else if (pid < 0) { - error_setg_errno(err, errno, "failed to create child process"); + error_setg_errno(errp, errno, "failed to create child process"); goto out; } ga_wait_child(pid, &status, &local_err); if (local_err) { - error_propagate(err, local_err); + error_propagate(errp, local_err); goto out; } if (!WIFEXITED(status)) { - error_setg(err, "child process has terminated abnormally"); + error_setg(errp, "child process has terminated abnormally"); goto out; } @@ -980,11 +983,11 @@ static void bios_supports_mode(const char *pmutils_bin, const char *pmutils_arg, case SUSPEND_SUPPORTED: goto out; case SUSPEND_NOT_SUPPORTED: - error_setg(err, + error_setg(errp, "the requested suspend mode is not supported by the guest"); goto out; default: - error_setg(err, + error_setg(errp, "the helper program '%s' returned an unexpected exit status" " code (%d)", pmutils_path, WEXITSTATUS(status)); goto out; @@ -995,7 +998,7 @@ out: } static void guest_suspend(const char *pmutils_bin, const char *sysfile_str, - Error **err) + Error **errp) { Error *local_err = NULL; char *pmutils_path; @@ -1038,23 +1041,23 @@ static void guest_suspend(const char *pmutils_bin, const char *sysfile_str, _exit(EXIT_SUCCESS); } else if (pid < 0) { - error_setg_errno(err, errno, "failed to create child process"); + error_setg_errno(errp, errno, "failed to create child process"); goto out; } ga_wait_child(pid, &status, &local_err); if (local_err) { - error_propagate(err, local_err); + error_propagate(errp, local_err); goto out; } if (!WIFEXITED(status)) { - error_setg(err, "child process has terminated abnormally"); + error_setg(errp, "child process has terminated abnormally"); goto out; } if (WEXITSTATUS(status)) { - error_setg(err, "child process has failed to suspend"); + error_setg(errp, "child process has failed to suspend"); goto out; } @@ -1062,34 +1065,44 @@ out: g_free(pmutils_path); } -void qmp_guest_suspend_disk(Error **err) +void qmp_guest_suspend_disk(Error **errp) { - bios_supports_mode("pm-is-supported", "--hibernate", "disk", err); - if (error_is_set(err)) { + Error *local_err = NULL; + + bios_supports_mode("pm-is-supported", "--hibernate", "disk", &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } - guest_suspend("pm-hibernate", "disk", err); + guest_suspend("pm-hibernate", "disk", errp); } -void qmp_guest_suspend_ram(Error **err) +void qmp_guest_suspend_ram(Error **errp) { - bios_supports_mode("pm-is-supported", "--suspend", "mem", err); - if (error_is_set(err)) { + Error *local_err = NULL; + + bios_supports_mode("pm-is-supported", "--suspend", "mem", &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } - guest_suspend("pm-suspend", "mem", err); + guest_suspend("pm-suspend", "mem", errp); } -void qmp_guest_suspend_hybrid(Error **err) +void qmp_guest_suspend_hybrid(Error **errp) { - bios_supports_mode("pm-is-supported", "--suspend-hybrid", NULL, err); - if (error_is_set(err)) { + Error *local_err = NULL; + + bios_supports_mode("pm-is-supported", "--suspend-hybrid", NULL, + &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } - guest_suspend("pm-suspend-hybrid", NULL, err); + guest_suspend("pm-suspend-hybrid", NULL, errp); } static GuestNetworkInterfaceList * @@ -1252,9 +1265,9 @@ error: return NULL; } -#define SYSCONF_EXACT(name, err) sysconf_exact((name), #name, (err)) +#define SYSCONF_EXACT(name, errp) sysconf_exact((name), #name, (errp)) -static long sysconf_exact(int name, const char *name_str, Error **err) +static long sysconf_exact(int name, const char *name_str, Error **errp) { long ret; @@ -1262,9 +1275,9 @@ static long sysconf_exact(int name, const char *name_str, Error **err) ret = sysconf(name); if (ret == -1) { if (errno == 0) { - error_setg(err, "sysconf(%s): value indefinite", name_str); + error_setg(errp, "sysconf(%s): value indefinite", name_str); } else { - error_setg_errno(err, errno, "sysconf(%s)", name_str); + error_setg_errno(errp, errno, "sysconf(%s)", name_str); } } return ret; @@ -1410,19 +1423,19 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) #else /* defined(__linux__) */ -void qmp_guest_suspend_disk(Error **err) +void qmp_guest_suspend_disk(Error **errp) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); } -void qmp_guest_suspend_ram(Error **err) +void qmp_guest_suspend_ram(Error **errp) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); } -void qmp_guest_suspend_hybrid(Error **err) +void qmp_guest_suspend_hybrid(Error **errp) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); } GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp) @@ -1447,32 +1460,32 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) #if !defined(CONFIG_FSFREEZE) -GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err) +GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); return 0; } -int64_t qmp_guest_fsfreeze_freeze(Error **err) +int64_t qmp_guest_fsfreeze_freeze(Error **errp) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); return 0; } -int64_t qmp_guest_fsfreeze_thaw(Error **err) +int64_t qmp_guest_fsfreeze_thaw(Error **errp) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); return 0; } #endif /* CONFIG_FSFREEZE */ #if !defined(CONFIG_FSTRIM) -void qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **err) +void qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); } #endif diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 0ee07b6e23..d793dd0c85 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -29,16 +29,12 @@ (365 * (1970 - 1601) + \ (1970 - 1601) / 4 - 3)) -static void acquire_privilege(const char *name, Error **err) +static void acquire_privilege(const char *name, Error **errp) { HANDLE token; TOKEN_PRIVILEGES priv; Error *local_err = NULL; - if (error_is_set(err)) { - return; - } - if (OpenProcessToken(GetCurrentProcess(), TOKEN_ADJUST_PRIVILEGES|TOKEN_QUERY, &token)) { @@ -65,27 +61,26 @@ static void acquire_privilege(const char *name, Error **err) out: if (local_err) { - error_propagate(err, local_err); + error_propagate(errp, local_err); } } -static void execute_async(DWORD WINAPI (*func)(LPVOID), LPVOID opaque, Error **err) +static void execute_async(DWORD WINAPI (*func)(LPVOID), LPVOID opaque, + Error **errp) { Error *local_err = NULL; - if (error_is_set(err)) { - return; - } HANDLE thread = CreateThread(NULL, 0, func, opaque, 0, NULL); if (!thread) { error_set(&local_err, QERR_QGA_COMMAND_FAILED, "failed to dispatch asynchronous command"); - error_propagate(err, local_err); + error_propagate(errp, local_err); } } -void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err) +void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp) { + Error *local_err = NULL; UINT shutdown_flag = EWX_FORCE; slog("guest-shutdown called, mode: %s", mode); @@ -97,68 +92,71 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err) } else if (strcmp(mode, "reboot") == 0) { shutdown_flag |= EWX_REBOOT; } else { - error_set(err, QERR_INVALID_PARAMETER_VALUE, "mode", + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "mode", "halt|powerdown|reboot"); return; } /* Request a shutdown privilege, but try to shut down the system anyway. */ - acquire_privilege(SE_SHUTDOWN_NAME, err); - if (error_is_set(err)) { + acquire_privilege(SE_SHUTDOWN_NAME, &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } if (!ExitWindowsEx(shutdown_flag, SHTDN_REASON_FLAG_PLANNED)) { slog("guest-shutdown failed: %lu", GetLastError()); - error_set(err, QERR_UNDEFINED_ERROR); + error_set(errp, QERR_UNDEFINED_ERROR); } } -int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, Error **err) +int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, + Error **errp) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); return 0; } -void qmp_guest_file_close(int64_t handle, Error **err) +void qmp_guest_file_close(int64_t handle, Error **errp) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); } GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count, - int64_t count, Error **err) + int64_t count, Error **errp) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); return 0; } GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64, - bool has_count, int64_t count, Error **err) + bool has_count, int64_t count, + Error **errp) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); return 0; } GuestFileSeek *qmp_guest_file_seek(int64_t handle, int64_t offset, - int64_t whence, Error **err) + int64_t whence, Error **errp) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); return 0; } -void qmp_guest_file_flush(int64_t handle, Error **err) +void qmp_guest_file_flush(int64_t handle, Error **errp) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); } /* * Return status of freeze/thaw */ -GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err) +GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp) { if (!vss_initialized()) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); return 0; } @@ -173,13 +171,13 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err) * Freeze local file systems using Volume Shadow-copy Service. * The frozen state is limited for up to 10 seconds by VSS. */ -int64_t qmp_guest_fsfreeze_freeze(Error **err) +int64_t qmp_guest_fsfreeze_freeze(Error **errp) { int i; Error *local_err = NULL; if (!vss_initialized()) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); return 0; } @@ -188,14 +186,16 @@ int64_t qmp_guest_fsfreeze_freeze(Error **err) /* cannot risk guest agent blocking itself on a write in this state */ ga_set_frozen(ga_state); - qga_vss_fsfreeze(&i, err, true); - if (error_is_set(err)) { + qga_vss_fsfreeze(&i, &local_err, true); + if (local_err) { + error_propagate(errp, local_err); goto error; } return i; error: + local_err = NULL; qmp_guest_fsfreeze_thaw(&local_err); if (local_err) { g_debug("cleanup thaw: %s", error_get_pretty(local_err)); @@ -207,16 +207,16 @@ error: /* * Thaw local file systems using Volume Shadow-copy Service. */ -int64_t qmp_guest_fsfreeze_thaw(Error **err) +int64_t qmp_guest_fsfreeze_thaw(Error **errp) { int i; if (!vss_initialized()) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); return 0; } - qga_vss_fsfreeze(&i, err, false); + qga_vss_fsfreeze(&i, errp, false); ga_unset_frozen(ga_state); return i; @@ -246,9 +246,9 @@ static void guest_fsfreeze_cleanup(void) * Walk list of mounted file systems in the guest, and discard unused * areas. */ -void qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **err) +void qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); } typedef enum { @@ -256,14 +256,11 @@ typedef enum { GUEST_SUSPEND_MODE_RAM } GuestSuspendMode; -static void check_suspend_mode(GuestSuspendMode mode, Error **err) +static void check_suspend_mode(GuestSuspendMode mode, Error **errp) { SYSTEM_POWER_CAPABILITIES sys_pwr_caps; Error *local_err = NULL; - if (error_is_set(err)) { - return; - } ZeroMemory(&sys_pwr_caps, sizeof(sys_pwr_caps)); if (!GetPwrCapabilities(&sys_pwr_caps)) { error_set(&local_err, QERR_QGA_COMMAND_FAILED, @@ -291,7 +288,7 @@ static void check_suspend_mode(GuestSuspendMode mode, Error **err) out: if (local_err) { - error_propagate(err, local_err); + error_propagate(errp, local_err); } } @@ -308,42 +305,46 @@ static DWORD WINAPI do_suspend(LPVOID opaque) return ret; } -void qmp_guest_suspend_disk(Error **err) +void qmp_guest_suspend_disk(Error **errp) { + Error *local_err = NULL; GuestSuspendMode *mode = g_malloc(sizeof(GuestSuspendMode)); *mode = GUEST_SUSPEND_MODE_DISK; - check_suspend_mode(*mode, err); - acquire_privilege(SE_SHUTDOWN_NAME, err); - execute_async(do_suspend, mode, err); + check_suspend_mode(*mode, &local_err); + acquire_privilege(SE_SHUTDOWN_NAME, &local_err); + execute_async(do_suspend, mode, &local_err); - if (error_is_set(err)) { + if (local_err) { + error_propagate(errp, local_err); g_free(mode); } } -void qmp_guest_suspend_ram(Error **err) +void qmp_guest_suspend_ram(Error **errp) { + Error *local_err = NULL; GuestSuspendMode *mode = g_malloc(sizeof(GuestSuspendMode)); *mode = GUEST_SUSPEND_MODE_RAM; - check_suspend_mode(*mode, err); - acquire_privilege(SE_SHUTDOWN_NAME, err); - execute_async(do_suspend, mode, err); + check_suspend_mode(*mode, &local_err); + acquire_privilege(SE_SHUTDOWN_NAME, &local_err); + execute_async(do_suspend, mode, &local_err); - if (error_is_set(err)) { + if (local_err) { + error_propagate(errp, local_err); g_free(mode); } } -void qmp_guest_suspend_hybrid(Error **err) +void qmp_guest_suspend_hybrid(Error **errp) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); } -GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **err) +GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp) { - error_set(err, QERR_UNSUPPORTED); + error_set(errp, QERR_UNSUPPORTED); return NULL; } @@ -372,6 +373,7 @@ int64_t qmp_guest_get_time(Error **errp) void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) { + Error *local_err = NULL; SYSTEMTIME ts; FILETIME tf; LONGLONG time; @@ -403,8 +405,9 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) } } - acquire_privilege(SE_SYSTEMTIME_NAME, errp); - if (error_is_set(errp)) { + acquire_privilege(SE_SYSTEMTIME_NAME, &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } diff --git a/qga/commands.c b/qga/commands.c index a0c2de07ec..783496791e 100644 --- a/qga/commands.c +++ b/qga/commands.c @@ -40,7 +40,7 @@ int64_t qmp_guest_sync(int64_t id, Error **errp) return id; } -void qmp_guest_ping(Error **err) +void qmp_guest_ping(Error **errp) { slog("guest-ping called"); } @@ -62,7 +62,7 @@ static void qmp_command_info(QmpCommand *cmd, void *opaque) info->supported_commands = cmd_info_list; } -struct GuestAgentInfo *qmp_guest_info(Error **err) +struct GuestAgentInfo *qmp_guest_info(Error **errp) { GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo)); diff --git a/qga/main.c b/qga/main.c index 38219c9d77..8b927c9db7 100644 --- a/qga/main.c +++ b/qga/main.c @@ -910,6 +910,7 @@ int64_t ga_get_fd_handle(GAState *s, Error **errp) if (!write_persistent_state(&s->pstate, s->pstate_filepath)) { error_setg(errp, "failed to commit persistent state to disk"); + return -1; } return handle; diff --git a/qga/vss-win32.c b/qga/vss-win32.c index 24c428842b..0e4095736e 100644 --- a/qga/vss-win32.c +++ b/qga/vss-win32.c @@ -145,19 +145,19 @@ void ga_uninstall_vss_provider(void) } /* Call VSS requester and freeze/thaw filesystems and applications */ -void qga_vss_fsfreeze(int *nr_volume, Error **err, bool freeze) +void qga_vss_fsfreeze(int *nr_volume, Error **errp, bool freeze) { const char *func_name = freeze ? "requester_freeze" : "requester_thaw"; QGAVSSRequesterFunc func; ErrorSet errset = { .error_set = (ErrorSetFunc)error_set_win32, - .errp = (void **)err, + .errp = (void **)errp, .err_class = ERROR_CLASS_GENERIC_ERROR }; func = (QGAVSSRequesterFunc)GetProcAddress(provider_lib, func_name); if (!func) { - error_setg_win32(err, GetLastError(), "failed to load %s from %s", + error_setg_win32(errp, GetLastError(), "failed to load %s from %s", func_name, QGA_VSS_DLL); return; } diff --git a/qga/vss-win32.h b/qga/vss-win32.h index db8fbe5208..298927dfa5 100644 --- a/qga/vss-win32.h +++ b/qga/vss-win32.h @@ -22,6 +22,6 @@ bool vss_initialized(void); int ga_install_vss_provider(void); void ga_uninstall_vss_provider(void); -void qga_vss_fsfreeze(int *nr_volume, Error **err, bool freeze); +void qga_vss_fsfreeze(int *nr_volume, Error **errp, bool freeze); #endif diff --git a/qmp-commands.hx b/qmp-commands.hx index f437937e2c..cae890ee5d 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -1165,19 +1165,19 @@ Example: -> { "execute": "transaction", "arguments": { "actions": [ - { 'type': 'blockdev-snapshot-sync', 'data' : { "device": "ide-hd0", + { "type": "blockdev-snapshot-sync", "data" : { "device": "ide-hd0", "snapshot-file": "/some/place/my-image", "format": "qcow2" } }, - { 'type': 'blockdev-snapshot-sync', 'data' : { "node-name": "myfile", + { "type": "blockdev-snapshot-sync", "data" : { "node-name": "myfile", "snapshot-file": "/some/place/my-image2", "snapshot-node-name": "node3432", "mode": "existing", "format": "qcow2" } }, - { 'type': 'blockdev-snapshot-sync', 'data' : { "device": "ide-hd1", + { "type": "blockdev-snapshot-sync", "data" : { "device": "ide-hd1", "snapshot-file": "/some/place/my-image2", "mode": "existing", "format": "qcow2" } }, - { 'type': 'blockdev-snapshot-internal-sync', 'data' : { + { "type": "blockdev-snapshot-internal-sync", "data" : { "device": "ide-hd2", "name": "snapshot0" } } ] } } <- { "return": {} } @@ -41,7 +41,7 @@ NameInfo *qmp_query_name(Error **errp) return info; } -VersionInfo *qmp_query_version(Error **err) +VersionInfo *qmp_query_version(Error **errp) { VersionInfo *info = g_malloc0(sizeof(*info)); const char *version = QEMU_VERSION; @@ -82,7 +82,7 @@ UuidInfo *qmp_query_uuid(Error **errp) return info; } -void qmp_quit(Error **err) +void qmp_quit(Error **errp) { no_shutdown = 0; qemu_system_shutdown_request(); @@ -146,24 +146,9 @@ SpiceInfo *qmp_query_spice(Error **errp) }; #endif -static void iostatus_bdrv_it(void *opaque, BlockDriverState *bs) -{ - bdrv_iostatus_reset(bs); -} - -static void encrypted_bdrv_it(void *opaque, BlockDriverState *bs) -{ - Error **err = opaque; - - if (!error_is_set(err) && bdrv_key_required(bs)) { - error_set(err, QERR_DEVICE_ENCRYPTED, bdrv_get_device_name(bs), - bdrv_get_encrypted_filename(bs)); - } -} - void qmp_cont(Error **errp) { - Error *local_err = NULL; + BlockDriverState *bs; if (runstate_needs_reset()) { error_setg(errp, "Resetting the Virtual Machine is required"); @@ -172,11 +157,16 @@ void qmp_cont(Error **errp) return; } - bdrv_iterate(iostatus_bdrv_it, NULL); - bdrv_iterate(encrypted_bdrv_it, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; + for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) { + bdrv_iostatus_reset(bs); + } + for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) { + if (bdrv_key_required(bs)) { + error_set(errp, QERR_DEVICE_ENCRYPTED, + bdrv_get_device_name(bs), + bdrv_get_encrypted_filename(bs)); + return; + } } if (runstate_check(RUN_STATE_INMIGRATE)) { @@ -405,12 +395,12 @@ static void qmp_change_vnc(const char *target, bool has_arg, const char *arg, #endif /* !CONFIG_VNC */ void qmp_change(const char *device, const char *target, - bool has_arg, const char *arg, Error **err) + bool has_arg, const char *arg, Error **errp) { if (strcmp(device, "vnc") == 0) { - qmp_change_vnc(target, has_arg, arg, err); + qmp_change_vnc(target, has_arg, arg, errp); } else { - qmp_change_blockdev(device, target, arg, err); + qmp_change_blockdev(device, target, arg, errp); } } diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 9734ab0a53..8d9096f65c 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -369,9 +369,10 @@ def gen_command_def_prologue(prefix="", proxy=False): try: - opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:m", + opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:i:o:m", ["source", "header", "prefix=", - "output-dir=", "type=", "middle"]) + "input-file=", "output-dir=", + "type=", "middle"]) except getopt.GetoptError, err: print str(err) sys.exit(1) @@ -389,6 +390,8 @@ do_h = False for o, a in opts: if o in ("-p", "--prefix"): prefix = a + elif o in ("-i", "--input-file"): + input_file = a elif o in ("-o", "--output-dir"): output_dir = a + "/" elif o in ("-t", "--type"): @@ -420,7 +423,7 @@ except os.error, e: if e.errno != errno.EEXIST: raise -exprs = parse_schema(sys.stdin) +exprs = parse_schema(input_file) commands = filter(lambda expr: expr.has_key('command'), exprs) commands = filter(lambda expr: not expr.has_key('gen'), commands) diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 10864efc58..b4632324a7 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -279,14 +279,15 @@ void qapi_free_%(type)s(%(c_type)s obj) try: - opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:", + opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:i:o:", ["source", "header", "builtins", - "prefix=", "output-dir="]) + "prefix=", "input-file=", "output-dir="]) except getopt.GetoptError, err: print str(err) sys.exit(1) output_dir = "" +input_file = "" prefix = "" c_file = 'qapi-types.c' h_file = 'qapi-types.h' @@ -298,6 +299,8 @@ do_builtins = False for o, a in opts: if o in ("-p", "--prefix"): prefix = a + elif o in ("-i", "--input-file"): + input_file = a elif o in ("-o", "--output-dir"): output_dir = a + "/" elif o in ("-c", "--source"): @@ -378,7 +381,7 @@ fdecl.write(mcgen(''' ''', guard=guardname(h_file))) -exprs = parse_schema(sys.stdin) +exprs = parse_schema(input_file) exprs = filter(lambda expr: not expr.has_key('gen'), exprs) fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL")) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 45ce3a957a..c6579beed5 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -397,13 +397,14 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **e name=name) try: - opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:", + opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:i:o:", ["source", "header", "builtins", "prefix=", - "output-dir="]) + "input-file=", "output-dir="]) except getopt.GetoptError, err: print str(err) sys.exit(1) +input_file = "" output_dir = "" prefix = "" c_file = 'qapi-visit.c' @@ -416,6 +417,8 @@ do_builtins = False for o, a in opts: if o in ("-p", "--prefix"): prefix = a + elif o in ("-i", "--input-file"): + input_file = a elif o in ("-o", "--output-dir"): output_dir = a + "/" elif o in ("-c", "--source"): @@ -494,7 +497,7 @@ fdecl.write(mcgen(''' ''', prefix=prefix, guard=guardname(h_file))) -exprs = parse_schema(sys.stdin) +exprs = parse_schema(input_file) # to avoid header dependency hell, we always generate declarations # for built-in types in our header files and simply guard them diff --git a/scripts/qapi.py b/scripts/qapi.py index b474c39558..ec806aabeb 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -11,7 +11,9 @@ # This work is licensed under the terms of the GNU GPL, version 2. # See the COPYING file in the top-level directory. +import re from ordereddict import OrderedDict +import os import sys builtin_types = [ @@ -35,9 +37,17 @@ builtin_type_qtypes = { 'uint64': 'QTYPE_QINT', } +def error_path(parent): + res = "" + while parent: + res = ("In file included from %s:%d:\n" % (parent['file'], + parent['line'])) + res + parent = parent['parent'] + return res + class QAPISchemaError(Exception): def __init__(self, schema, msg): - self.fp = schema.fp + self.input_file = schema.input_file self.msg = msg self.col = 1 self.line = schema.line @@ -46,23 +56,31 @@ class QAPISchemaError(Exception): self.col = (self.col + 7) % 8 + 1 else: self.col += 1 + self.info = schema.parent_info def __str__(self): - return "%s:%s:%s: %s" % (self.fp.name, self.line, self.col, self.msg) + return error_path(self.info) + \ + "%s:%d:%d: %s" % (self.input_file, self.line, self.col, self.msg) class QAPIExprError(Exception): def __init__(self, expr_info, msg): - self.fp = expr_info['fp'] - self.line = expr_info['line'] + self.info = expr_info self.msg = msg def __str__(self): - return "%s:%s: %s" % (self.fp.name, self.line, self.msg) + return error_path(self.info['parent']) + \ + "%s:%d: %s" % (self.info['file'], self.info['line'], self.msg) class QAPISchema: - def __init__(self, fp): - self.fp = fp + def __init__(self, fp, input_relname=None, include_hist=[], parent_info=None): + 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)] + self.parent_info = parent_info self.src = fp.read() if self.src == '' or self.src[-1] != '\n': self.src += '\n' @@ -73,10 +91,33 @@ class QAPISchema: self.accept() while self.tok != None: - expr_info = {'fp': fp, 'line': self.line} - expr_elem = {'expr': self.get_expr(False), - 'info': expr_info} - self.exprs.append(expr_elem) + expr_info = {'file': input_relname, 'line': self.line, 'parent': self.parent_info} + expr = self.get_expr(False) + if isinstance(expr, dict) and "include" in expr: + if len(expr) != 1: + raise QAPIExprError(expr_info, "Invalid 'include' directive") + include = expr["include"] + if not isinstance(include, str): + raise QAPIExprError(expr_info, + 'Expected a file name (string), got: %s' + % include) + include_path = os.path.join(self.input_dir, include) + if any(include_path == elem[1] + for elem in self.include_hist): + raise QAPIExprError(expr_info, "Inclusion loop for %s" + % include) + 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) + self.exprs.extend(exprs_include.exprs) + else: + expr_elem = {'expr': expr, + 'info': expr_info} + self.exprs.append(expr_elem) def accept(self): while True: @@ -263,10 +304,10 @@ def check_exprs(schema): if expr.has_key('union'): check_union(expr, expr_elem['info']) -def parse_schema(fp): +def parse_schema(input_file): try: - schema = QAPISchema(fp) - except QAPISchemaError, e: + schema = QAPISchema(open(input_file, "r")) + except (QAPISchemaError, QAPIExprError), e: print >>sys.stderr, e exit(1) diff --git a/tests/Makefile b/tests/Makefile index 14ecf05b5d..6b8b6f273a 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -190,7 +190,10 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \ duplicate-key.json union-invalid-base.json flat-union-no-base.json \ flat-union-invalid-discriminator.json \ flat-union-invalid-branch-key.json flat-union-reverse-define.json \ - flat-union-string-discriminator.json) + 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) GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h @@ -242,13 +245,19 @@ tests/test-vmstate$(EXESUF): tests/test-vmstate.o \ tests/test-qapi-types.c tests/test-qapi-types.h :\ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@") + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ + $(gen-out-type) -o tests -p "test-" -i $<, \ + " GEN $@") tests/test-qapi-visit.c tests/test-qapi-visit.h :\ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-visit.py - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@") + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \ + $(gen-out-type) -o tests -p "test-" -i $<, \ + " GEN $@") tests/test-qmp-commands.h tests/test-qmp-marshal.c :\ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@") + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \ + $(gen-out-type) -o tests -p "test-" -i $<, \ + " GEN $@") tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a @@ -400,9 +409,14 @@ check-tests/test-qapi.py: tests/test-qapi.py .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y)) $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json - $(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py <$^ >$*.test.out 2>$*.test.err; echo $$? >$*.test.exit, " TEST $*.out") + $(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \ + $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \ + $^ >$*.test.out 2>$*.test.err; \ + echo $$? >$*.test.exit, \ + " TEST $*.out") @diff -q $(SRC_PATH)/$*.out $*.test.out - @diff -q $(SRC_PATH)/$*.err $*.test.err + @# Sanitize error messages (make them independent of build directory) + @perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff -q $(SRC_PATH)/$*.err - @diff -q $(SRC_PATH)/$*.exit $*.test.exit # Consolidated targets diff --git a/tests/qapi-schema/duplicate-key.err b/tests/qapi-schema/duplicate-key.err index 0801c6a9bb..768b276f80 100644 --- a/tests/qapi-schema/duplicate-key.err +++ b/tests/qapi-schema/duplicate-key.err @@ -1 +1 @@ -<stdin>:2:10: Duplicate key "key" +tests/qapi-schema/duplicate-key.json:2:10: Duplicate key "key" diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.err b/tests/qapi-schema/flat-union-invalid-branch-key.err index 1125caf5db..ccf72d2dfe 100644 --- a/tests/qapi-schema/flat-union-invalid-branch-key.err +++ b/tests/qapi-schema/flat-union-invalid-branch-key.err @@ -1 +1 @@ -<stdin>:13: Discriminator value 'value_wrong' is not found in enum 'TestEnum' +tests/qapi-schema/flat-union-invalid-branch-key.json:13: Discriminator value 'value_wrong' is not found in enum 'TestEnum' diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.err b/tests/qapi-schema/flat-union-invalid-discriminator.err index cad9dbf225..790b6759b8 100644 --- a/tests/qapi-schema/flat-union-invalid-discriminator.err +++ b/tests/qapi-schema/flat-union-invalid-discriminator.err @@ -1 +1 @@ -<stdin>:13: Discriminator 'enum_wrong' is not a member of base type 'TestBase' +tests/qapi-schema/flat-union-invalid-discriminator.json:13: Discriminator 'enum_wrong' is not a member of base type 'TestBase' diff --git a/tests/qapi-schema/flat-union-no-base.err b/tests/qapi-schema/flat-union-no-base.err index e2d7443a3b..a59749eb84 100644 --- a/tests/qapi-schema/flat-union-no-base.err +++ b/tests/qapi-schema/flat-union-no-base.err @@ -1 +1 @@ -<stdin>:7: Flat union 'TestUnion' must have a base field +tests/qapi-schema/flat-union-no-base.json:7: Flat union 'TestUnion' must have a base field diff --git a/tests/qapi-schema/flat-union-string-discriminator.err b/tests/qapi-schema/flat-union-string-discriminator.err index 87482704ec..200016bd5c 100644 --- a/tests/qapi-schema/flat-union-string-discriminator.err +++ b/tests/qapi-schema/flat-union-string-discriminator.err @@ -1 +1 @@ -<stdin>:13: Discriminator 'kind' must be of enumeration type +tests/qapi-schema/flat-union-string-discriminator.json:13: Discriminator 'kind' must be of enumeration type diff --git a/tests/qapi-schema/funny-char.err b/tests/qapi-schema/funny-char.err index d3dd293faf..bfc890cd9f 100644 --- a/tests/qapi-schema/funny-char.err +++ b/tests/qapi-schema/funny-char.err @@ -1 +1 @@ -<stdin>:2:36: Stray ";" +tests/qapi-schema/funny-char.json:2:36: Stray ";" diff --git a/tests/qapi-schema/include-before-err.err b/tests/qapi-schema/include-before-err.err new file mode 100644 index 0000000000..55652751e1 --- /dev/null +++ b/tests/qapi-schema/include-before-err.err @@ -0,0 +1 @@ +tests/qapi-schema/include-before-err.json:2:13: Expected ":" diff --git a/tests/qapi-schema/include-before-err.exit b/tests/qapi-schema/include-before-err.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/include-before-err.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/include-before-err.json b/tests/qapi-schema/include-before-err.json new file mode 100644 index 0000000000..afb6cb63c4 --- /dev/null +++ b/tests/qapi-schema/include-before-err.json @@ -0,0 +1,2 @@ +{ 'include': 'include-simple-sub.json' } +{ 'command' 'missing-colon' } diff --git a/tests/qapi-schema/include-before-err.out b/tests/qapi-schema/include-before-err.out new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/tests/qapi-schema/include-before-err.out diff --git a/tests/qapi-schema/include-cycle-b.json b/tests/qapi-schema/include-cycle-b.json new file mode 100644 index 0000000000..4fa985dcd5 --- /dev/null +++ b/tests/qapi-schema/include-cycle-b.json @@ -0,0 +1 @@ +{ 'include': 'include-cycle-c.json' } diff --git a/tests/qapi-schema/include-cycle-c.json b/tests/qapi-schema/include-cycle-c.json new file mode 100644 index 0000000000..d12b5924a3 --- /dev/null +++ b/tests/qapi-schema/include-cycle-c.json @@ -0,0 +1 @@ +{ 'include': 'include-cycle.json' } diff --git a/tests/qapi-schema/include-cycle.err b/tests/qapi-schema/include-cycle.err new file mode 100644 index 0000000000..602cf62329 --- /dev/null +++ b/tests/qapi-schema/include-cycle.err @@ -0,0 +1,3 @@ +In file included from tests/qapi-schema/include-cycle.json:1: +In file included from include-cycle-b.json:1: +include-cycle-c.json:1: Inclusion loop for include-cycle.json diff --git a/tests/qapi-schema/include-cycle.exit b/tests/qapi-schema/include-cycle.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/include-cycle.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/include-cycle.json b/tests/qapi-schema/include-cycle.json new file mode 100644 index 0000000000..6fcf1ebaac --- /dev/null +++ b/tests/qapi-schema/include-cycle.json @@ -0,0 +1 @@ +{ 'include': 'include-cycle-b.json' } diff --git a/tests/qapi-schema/include-cycle.out b/tests/qapi-schema/include-cycle.out new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/tests/qapi-schema/include-cycle.out diff --git a/tests/qapi-schema/include-format-err.err b/tests/qapi-schema/include-format-err.err new file mode 100644 index 0000000000..721ff4eccc --- /dev/null +++ b/tests/qapi-schema/include-format-err.err @@ -0,0 +1 @@ +tests/qapi-schema/include-format-err.json:1: Invalid 'include' directive diff --git a/tests/qapi-schema/include-format-err.exit b/tests/qapi-schema/include-format-err.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/include-format-err.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/include-format-err.json b/tests/qapi-schema/include-format-err.json new file mode 100644 index 0000000000..44980f026f --- /dev/null +++ b/tests/qapi-schema/include-format-err.json @@ -0,0 +1,2 @@ +{ 'include': 'include-simple-sub.json', + 'foo': 'bar' } diff --git a/tests/qapi-schema/include-format-err.out b/tests/qapi-schema/include-format-err.out new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/tests/qapi-schema/include-format-err.out diff --git a/tests/qapi-schema/include-nested-err.err b/tests/qapi-schema/include-nested-err.err new file mode 100644 index 0000000000..1dacbda3be --- /dev/null +++ b/tests/qapi-schema/include-nested-err.err @@ -0,0 +1,2 @@ +In file included from tests/qapi-schema/include-nested-err.json:1: +missing-colon.json:1:10: Expected ":" diff --git a/tests/qapi-schema/include-nested-err.exit b/tests/qapi-schema/include-nested-err.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/include-nested-err.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/include-nested-err.json b/tests/qapi-schema/include-nested-err.json new file mode 100644 index 0000000000..5631e56ea0 --- /dev/null +++ b/tests/qapi-schema/include-nested-err.json @@ -0,0 +1 @@ +{ 'include': 'missing-colon.json' } diff --git a/tests/qapi-schema/include-nested-err.out b/tests/qapi-schema/include-nested-err.out new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/tests/qapi-schema/include-nested-err.out diff --git a/tests/qapi-schema/include-no-file.err b/tests/qapi-schema/include-no-file.err new file mode 100644 index 0000000000..d5b9b22d85 --- /dev/null +++ b/tests/qapi-schema/include-no-file.err @@ -0,0 +1 @@ +tests/qapi-schema/include-no-file.json:1: No such file or directory: include-no-file-sub.json diff --git a/tests/qapi-schema/include-no-file.exit b/tests/qapi-schema/include-no-file.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/include-no-file.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/include-no-file.json b/tests/qapi-schema/include-no-file.json new file mode 100644 index 0000000000..9249ebd50c --- /dev/null +++ b/tests/qapi-schema/include-no-file.json @@ -0,0 +1 @@ +{ 'include': 'include-no-file-sub.json' } diff --git a/tests/qapi-schema/include-no-file.out b/tests/qapi-schema/include-no-file.out new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/tests/qapi-schema/include-no-file.out diff --git a/tests/qapi-schema/include-non-file.err b/tests/qapi-schema/include-non-file.err new file mode 100644 index 0000000000..9658c78801 --- /dev/null +++ b/tests/qapi-schema/include-non-file.err @@ -0,0 +1 @@ +tests/qapi-schema/include-non-file.json:1: Expected a file name (string), got: ['foo', 'bar'] diff --git a/tests/qapi-schema/include-non-file.exit b/tests/qapi-schema/include-non-file.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/include-non-file.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/include-non-file.json b/tests/qapi-schema/include-non-file.json new file mode 100644 index 0000000000..cd43c3f9db --- /dev/null +++ b/tests/qapi-schema/include-non-file.json @@ -0,0 +1 @@ +{ 'include': [ 'foo', 'bar' ] } diff --git a/tests/qapi-schema/include-non-file.out b/tests/qapi-schema/include-non-file.out new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/tests/qapi-schema/include-non-file.out diff --git a/tests/qapi-schema/include-relpath-sub.json b/tests/qapi-schema/include-relpath-sub.json new file mode 100644 index 0000000000..4bd4af4162 --- /dev/null +++ b/tests/qapi-schema/include-relpath-sub.json @@ -0,0 +1,2 @@ +{ 'enum': 'Status', + 'data': [ 'good', 'bad', 'ugly' ] } diff --git a/tests/qapi-schema/include-relpath.err b/tests/qapi-schema/include-relpath.err new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/tests/qapi-schema/include-relpath.err diff --git a/tests/qapi-schema/include-relpath.exit b/tests/qapi-schema/include-relpath.exit new file mode 100644 index 0000000000..573541ac97 --- /dev/null +++ b/tests/qapi-schema/include-relpath.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/include-relpath.json b/tests/qapi-schema/include-relpath.json new file mode 100644 index 0000000000..05018f3908 --- /dev/null +++ b/tests/qapi-schema/include-relpath.json @@ -0,0 +1 @@ +{ 'include': 'include/relpath.json' } diff --git a/tests/qapi-schema/include-relpath.out b/tests/qapi-schema/include-relpath.out new file mode 100644 index 0000000000..4ce3dcf12f --- /dev/null +++ b/tests/qapi-schema/include-relpath.out @@ -0,0 +1,3 @@ +[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])] +[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}] +[] diff --git a/tests/qapi-schema/include-self-cycle.err b/tests/qapi-schema/include-self-cycle.err new file mode 100644 index 0000000000..981742ae5f --- /dev/null +++ b/tests/qapi-schema/include-self-cycle.err @@ -0,0 +1 @@ +tests/qapi-schema/include-self-cycle.json:1: Inclusion loop for include-self-cycle.json diff --git a/tests/qapi-schema/include-self-cycle.exit b/tests/qapi-schema/include-self-cycle.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/include-self-cycle.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/include-self-cycle.json b/tests/qapi-schema/include-self-cycle.json new file mode 100644 index 0000000000..55fb1b596f --- /dev/null +++ b/tests/qapi-schema/include-self-cycle.json @@ -0,0 +1 @@ +{ 'include': 'include-self-cycle.json' } diff --git a/tests/qapi-schema/include-self-cycle.out b/tests/qapi-schema/include-self-cycle.out new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/tests/qapi-schema/include-self-cycle.out diff --git a/tests/qapi-schema/include-simple-sub.json b/tests/qapi-schema/include-simple-sub.json new file mode 100644 index 0000000000..4bd4af4162 --- /dev/null +++ b/tests/qapi-schema/include-simple-sub.json @@ -0,0 +1,2 @@ +{ 'enum': 'Status', + 'data': [ 'good', 'bad', 'ugly' ] } diff --git a/tests/qapi-schema/include-simple.err b/tests/qapi-schema/include-simple.err new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/tests/qapi-schema/include-simple.err diff --git a/tests/qapi-schema/include-simple.exit b/tests/qapi-schema/include-simple.exit new file mode 100644 index 0000000000..573541ac97 --- /dev/null +++ b/tests/qapi-schema/include-simple.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/include-simple.json b/tests/qapi-schema/include-simple.json new file mode 100644 index 0000000000..1dd391a592 --- /dev/null +++ b/tests/qapi-schema/include-simple.json @@ -0,0 +1 @@ +{ 'include': 'include-simple-sub.json' } diff --git a/tests/qapi-schema/include-simple.out b/tests/qapi-schema/include-simple.out new file mode 100644 index 0000000000..4ce3dcf12f --- /dev/null +++ b/tests/qapi-schema/include-simple.out @@ -0,0 +1,3 @@ +[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])] +[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}] +[] diff --git a/tests/qapi-schema/include/relpath.json b/tests/qapi-schema/include/relpath.json new file mode 100644 index 0000000000..45dee24704 --- /dev/null +++ b/tests/qapi-schema/include/relpath.json @@ -0,0 +1 @@ +{ 'include': '../include-relpath-sub.json' } diff --git a/tests/qapi-schema/missing-colon.err b/tests/qapi-schema/missing-colon.err index 9f2a35515c..d9d66b377a 100644 --- a/tests/qapi-schema/missing-colon.err +++ b/tests/qapi-schema/missing-colon.err @@ -1 +1 @@ -<stdin>:1:10: Expected ":" +tests/qapi-schema/missing-colon.json:1:10: Expected ":" diff --git a/tests/qapi-schema/missing-comma-list.err b/tests/qapi-schema/missing-comma-list.err index 4fe0700195..e73d2770d6 100644 --- a/tests/qapi-schema/missing-comma-list.err +++ b/tests/qapi-schema/missing-comma-list.err @@ -1 +1 @@ -<stdin>:2:20: Expected "," or "]" +tests/qapi-schema/missing-comma-list.json:2:20: Expected "," or "]" diff --git a/tests/qapi-schema/missing-comma-object.err b/tests/qapi-schema/missing-comma-object.err index b0121b5f3a..52b3a8a1ec 100644 --- a/tests/qapi-schema/missing-comma-object.err +++ b/tests/qapi-schema/missing-comma-object.err @@ -1 +1 @@ -<stdin>:2:3: Expected "," or "}" +tests/qapi-schema/missing-comma-object.json:2:3: Expected "," or "}" diff --git a/tests/qapi-schema/non-objects.err b/tests/qapi-schema/non-objects.err index a6c2dc26a6..334f0c91ae 100644 --- a/tests/qapi-schema/non-objects.err +++ b/tests/qapi-schema/non-objects.err @@ -1 +1 @@ -<stdin>:1:1: Expected "{" +tests/qapi-schema/non-objects.json:1:1: Expected "{" diff --git a/tests/qapi-schema/quoted-structural-chars.err b/tests/qapi-schema/quoted-structural-chars.err index a6c2dc26a6..9b183841dd 100644 --- a/tests/qapi-schema/quoted-structural-chars.err +++ b/tests/qapi-schema/quoted-structural-chars.err @@ -1 +1 @@ -<stdin>:1:1: Expected "{" +tests/qapi-schema/quoted-structural-chars.json:1:1: Expected "{" diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py index b3d1e1dbce..634ef2d00a 100644 --- a/tests/qapi-schema/test-qapi.py +++ b/tests/qapi-schema/test-qapi.py @@ -12,15 +12,13 @@ from qapi import * from pprint import pprint +import os import sys try: - exprs = parse_schema(sys.stdin) + exprs = parse_schema(sys.argv[1]) except SystemExit: raise -except: - print >>sys.stderr, "Crashed:", sys.exc_info()[0] - exit(1) pprint(exprs) pprint(enum_types) diff --git a/tests/qapi-schema/trailing-comma-list.err b/tests/qapi-schema/trailing-comma-list.err index ff839a34e9..24c24b0108 100644 --- a/tests/qapi-schema/trailing-comma-list.err +++ b/tests/qapi-schema/trailing-comma-list.err @@ -1 +1 @@ -<stdin>:2:36: Expected "{", "[" or string +tests/qapi-schema/trailing-comma-list.json:2:36: Expected "{", "[" or string diff --git a/tests/qapi-schema/trailing-comma-object.err b/tests/qapi-schema/trailing-comma-object.err index f5409627da..30bce5e194 100644 --- a/tests/qapi-schema/trailing-comma-object.err +++ b/tests/qapi-schema/trailing-comma-object.err @@ -1 +1 @@ -<stdin>:2:38: Expected string +tests/qapi-schema/trailing-comma-object.json:2:38: Expected string diff --git a/tests/qapi-schema/unclosed-list.err b/tests/qapi-schema/unclosed-list.err index 0e837a7fad..fb41a86abd 100644 --- a/tests/qapi-schema/unclosed-list.err +++ b/tests/qapi-schema/unclosed-list.err @@ -1 +1 @@ -<stdin>:1:20: Expected "," or "]" +tests/qapi-schema/unclosed-list.json:1:20: Expected "," or "]" diff --git a/tests/qapi-schema/unclosed-object.err b/tests/qapi-schema/unclosed-object.err index e6dc9501dc..db3deedd63 100644 --- a/tests/qapi-schema/unclosed-object.err +++ b/tests/qapi-schema/unclosed-object.err @@ -1 +1 @@ -<stdin>:1:21: Expected "," or "}" +tests/qapi-schema/unclosed-object.json:1:21: Expected "," or "}" diff --git a/tests/qapi-schema/unclosed-string.err b/tests/qapi-schema/unclosed-string.err index 948d88339d..12b187074e 100644 --- a/tests/qapi-schema/unclosed-string.err +++ b/tests/qapi-schema/unclosed-string.err @@ -1 +1 @@ -<stdin>:1:11: Missing terminating "'" +tests/qapi-schema/unclosed-string.json:1:11: Missing terminating "'" diff --git a/tests/qapi-schema/union-invalid-base.err b/tests/qapi-schema/union-invalid-base.err index dd8e3d1b3b..938f96962b 100644 --- a/tests/qapi-schema/union-invalid-base.err +++ b/tests/qapi-schema/union-invalid-base.err @@ -1 +1 @@ -<stdin>:7: Base 'TestBaseWrong' is not a valid type +tests/qapi-schema/union-invalid-base.json:7: Base 'TestBaseWrong' is not a valid type diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c index f03353b755..449d285e56 100644 --- a/tests/test-qmp-input-strict.c +++ b/tests/test-qmp-input-strict.c @@ -86,13 +86,13 @@ static void test_validate_struct(TestInputVisitorData *data, const void *unused) { TestStruct *p = NULL; - Error *errp = NULL; + Error *err = NULL; Visitor *v; v = validate_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo' }"); - visit_type_TestStruct(v, &p, NULL, &errp); - g_assert(!errp); + visit_type_TestStruct(v, &p, NULL, &err); + g_assert(!err); g_free(p->string); g_free(p); } @@ -101,13 +101,13 @@ static void test_validate_struct_nested(TestInputVisitorData *data, const void *unused) { UserDefNested *udp = NULL; - Error *errp = NULL; + Error *err = NULL; Visitor *v; v = validate_test_init(data, "{ 'string0': 'string0', 'dict1': { 'string1': 'string1', 'dict2': { 'userdef1': { 'integer': 42, 'string': 'string' }, 'string2': 'string2'}}}"); - visit_type_UserDefNested(v, &udp, NULL, &errp); - g_assert(!errp); + visit_type_UserDefNested(v, &udp, NULL, &err); + g_assert(!err); qapi_free_UserDefNested(udp); } @@ -115,13 +115,13 @@ static void test_validate_list(TestInputVisitorData *data, const void *unused) { UserDefOneList *head = NULL; - Error *errp = NULL; + Error *err = NULL; Visitor *v; v = validate_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44 } ]"); - visit_type_UserDefOneList(v, &head, NULL, &errp); - g_assert(!errp); + visit_type_UserDefOneList(v, &head, NULL, &err); + g_assert(!err); qapi_free_UserDefOneList(head); } @@ -130,12 +130,12 @@ static void test_validate_union(TestInputVisitorData *data, { UserDefUnion *tmp = NULL; Visitor *v; - Error *errp = NULL; + Error *err = NULL; v = validate_test_init(data, "{ 'type': 'b', 'integer': 41, 'data' : { 'integer': 42 } }"); - visit_type_UserDefUnion(v, &tmp, NULL, &errp); - g_assert(!errp); + visit_type_UserDefUnion(v, &tmp, NULL, &err); + g_assert(!err); qapi_free_UserDefUnion(tmp); } @@ -144,7 +144,7 @@ static void test_validate_union_flat(TestInputVisitorData *data, { UserDefFlatUnion *tmp = NULL; Visitor *v; - Error *errp = NULL; + Error *err = NULL; v = validate_test_init(data, "{ 'enum1': 'value1', " @@ -152,8 +152,8 @@ static void test_validate_union_flat(TestInputVisitorData *data, "'boolean': true }"); /* TODO when generator bug is fixed, add 'integer': 41 */ - visit_type_UserDefFlatUnion(v, &tmp, NULL, &errp); - g_assert(!errp); + visit_type_UserDefFlatUnion(v, &tmp, NULL, &err); + g_assert(!err); qapi_free_UserDefFlatUnion(tmp); } @@ -162,12 +162,12 @@ static void test_validate_union_anon(TestInputVisitorData *data, { UserDefAnonUnion *tmp = NULL; Visitor *v; - Error *errp = NULL; + Error *err = NULL; v = validate_test_init(data, "42"); - visit_type_UserDefAnonUnion(v, &tmp, NULL, &errp); - g_assert(!errp); + visit_type_UserDefAnonUnion(v, &tmp, NULL, &err); + g_assert(!err); qapi_free_UserDefAnonUnion(tmp); } @@ -175,13 +175,13 @@ static void test_validate_fail_struct(TestInputVisitorData *data, const void *unused) { TestStruct *p = NULL; - Error *errp = NULL; + Error *err = NULL; Visitor *v; v = validate_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo', 'extra': 42 }"); - visit_type_TestStruct(v, &p, NULL, &errp); - g_assert(errp); + visit_type_TestStruct(v, &p, NULL, &err); + g_assert(err); if (p) { g_free(p->string); } @@ -192,13 +192,13 @@ static void test_validate_fail_struct_nested(TestInputVisitorData *data, const void *unused) { UserDefNested *udp = NULL; - Error *errp = NULL; + Error *err = NULL; Visitor *v; v = validate_test_init(data, "{ 'string0': 'string0', 'dict1': { 'string1': 'string1', 'dict2': { 'userdef1': { 'integer': 42, 'string': 'string', 'extra': [42, 23, {'foo':'bar'}] }, 'string2': 'string2'}}}"); - visit_type_UserDefNested(v, &udp, NULL, &errp); - g_assert(errp); + visit_type_UserDefNested(v, &udp, NULL, &err); + g_assert(err); qapi_free_UserDefNested(udp); } @@ -206,13 +206,13 @@ static void test_validate_fail_list(TestInputVisitorData *data, const void *unused) { UserDefOneList *head = NULL; - Error *errp = NULL; + Error *err = NULL; Visitor *v; v = validate_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44, 'extra': 'ggg' } ]"); - visit_type_UserDefOneList(v, &head, NULL, &errp); - g_assert(errp); + visit_type_UserDefOneList(v, &head, NULL, &err); + g_assert(err); qapi_free_UserDefOneList(head); } @@ -220,13 +220,13 @@ static void test_validate_fail_union(TestInputVisitorData *data, const void *unused) { UserDefUnion *tmp = NULL; - Error *errp = NULL; + Error *err = NULL; Visitor *v; v = validate_test_init(data, "{ 'type': 'b', 'data' : { 'integer': 42 } }"); - visit_type_UserDefUnion(v, &tmp, NULL, &errp); - g_assert(errp); + visit_type_UserDefUnion(v, &tmp, NULL, &err); + g_assert(err); qapi_free_UserDefUnion(tmp); } @@ -234,13 +234,13 @@ static void test_validate_fail_union_flat(TestInputVisitorData *data, const void *unused) { UserDefFlatUnion *tmp = NULL; - Error *errp = NULL; + Error *err = NULL; Visitor *v; v = validate_test_init(data, "{ 'string': 'c', 'integer': 41, 'boolean': true }"); - visit_type_UserDefFlatUnion(v, &tmp, NULL, &errp); - g_assert(errp); + visit_type_UserDefFlatUnion(v, &tmp, NULL, &err); + g_assert(err); qapi_free_UserDefFlatUnion(tmp); } @@ -249,12 +249,12 @@ static void test_validate_fail_union_anon(TestInputVisitorData *data, { UserDefAnonUnion *tmp = NULL; Visitor *v; - Error *errp = NULL; + Error *err = NULL; v = validate_test_init(data, "3.14"); - visit_type_UserDefAnonUnion(v, &tmp, NULL, &errp); - g_assert(errp); + visit_type_UserDefAnonUnion(v, &tmp, NULL, &err); + g_assert(err); qapi_free_UserDefAnonUnion(tmp); } diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c index 1729667a6f..a58a3e6fdb 100644 --- a/tests/test-qmp-input-visitor.c +++ b/tests/test-qmp-input-visitor.c @@ -90,13 +90,13 @@ static void test_visitor_in_int(TestInputVisitorData *data, const void *unused) { int64_t res = 0, value = -42; - Error *errp = NULL; + Error *err = NULL; Visitor *v; v = visitor_input_test_init(data, "%" PRId64, value); - visit_type_int(v, &res, NULL, &errp); - g_assert(!errp); + visit_type_int(v, &res, NULL, &err); + g_assert(!err); g_assert_cmpint(res, ==, value); } @@ -104,7 +104,7 @@ static void test_visitor_in_int_overflow(TestInputVisitorData *data, const void *unused) { int64_t res = 0; - Error *errp = NULL; + Error *err = NULL; Visitor *v; /* this will overflow a Qint/int64, so should be deserialized into @@ -113,22 +113,22 @@ static void test_visitor_in_int_overflow(TestInputVisitorData *data, */ v = visitor_input_test_init(data, "%f", DBL_MAX); - visit_type_int(v, &res, NULL, &errp); - g_assert(errp); - error_free(errp); + visit_type_int(v, &res, NULL, &err); + g_assert(err); + error_free(err); } static void test_visitor_in_bool(TestInputVisitorData *data, const void *unused) { - Error *errp = NULL; + Error *err = NULL; bool res = false; Visitor *v; v = visitor_input_test_init(data, "true"); - visit_type_bool(v, &res, NULL, &errp); - g_assert(!errp); + visit_type_bool(v, &res, NULL, &err); + g_assert(!err); g_assert_cmpint(res, ==, true); } @@ -136,13 +136,13 @@ static void test_visitor_in_number(TestInputVisitorData *data, const void *unused) { double res = 0, value = 3.14; - Error *errp = NULL; + Error *err = NULL; Visitor *v; v = visitor_input_test_init(data, "%f", value); - visit_type_number(v, &res, NULL, &errp); - g_assert(!errp); + visit_type_number(v, &res, NULL, &err); + g_assert(!err); g_assert_cmpfloat(res, ==, value); } @@ -150,13 +150,13 @@ static void test_visitor_in_string(TestInputVisitorData *data, const void *unused) { char *res = NULL, *value = (char *) "Q E M U"; - Error *errp = NULL; + Error *err = NULL; Visitor *v; v = visitor_input_test_init(data, "%s", value); - visit_type_str(v, &res, NULL, &errp); - g_assert(!errp); + visit_type_str(v, &res, NULL, &err); + g_assert(!err); g_assert_cmpstr(res, ==, value); g_free(res); @@ -165,7 +165,7 @@ static void test_visitor_in_string(TestInputVisitorData *data, static void test_visitor_in_enum(TestInputVisitorData *data, const void *unused) { - Error *errp = NULL; + Error *err = NULL; Visitor *v; EnumOne i; @@ -174,8 +174,8 @@ static void test_visitor_in_enum(TestInputVisitorData *data, v = visitor_input_test_init(data, "%s", EnumOne_lookup[i]); - visit_type_EnumOne(v, &res, NULL, &errp); - g_assert(!errp); + visit_type_EnumOne(v, &res, NULL, &err); + g_assert(!err); g_assert_cmpint(i, ==, res); visitor_input_teardown(data, NULL); @@ -196,34 +196,33 @@ static void visit_type_TestStruct(Visitor *v, TestStruct **obj, const char *name, Error **errp) { Error *err = NULL; - if (!error_is_set(errp)) { - 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); - } + + 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); } + error_propagate(errp, err); } static void test_visitor_in_struct(TestInputVisitorData *data, const void *unused) { TestStruct *p = NULL; - Error *errp = NULL; + Error *err = NULL; Visitor *v; v = visitor_input_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo' }"); - visit_type_TestStruct(v, &p, NULL, &errp); - g_assert(!errp); + visit_type_TestStruct(v, &p, NULL, &err); + g_assert(!err); g_assert_cmpint(p->integer, ==, -42); g_assert(p->boolean == true); g_assert_cmpstr(p->string, ==, "foo"); @@ -242,13 +241,13 @@ static void test_visitor_in_struct_nested(TestInputVisitorData *data, const void *unused) { UserDefNested *udp = NULL; - Error *errp = NULL; + Error *err = NULL; Visitor *v; v = visitor_input_test_init(data, "{ 'string0': 'string0', 'dict1': { 'string1': 'string1', 'dict2': { 'userdef1': { 'integer': 42, 'string': 'string' }, 'string2': 'string2'}}}"); - visit_type_UserDefNested(v, &udp, NULL, &errp); - g_assert(!errp); + visit_type_UserDefNested(v, &udp, NULL, &err); + g_assert(!err); check_and_free_str(udp->string0, "string0"); check_and_free_str(udp->dict1.string1, "string1"); @@ -265,14 +264,14 @@ static void test_visitor_in_list(TestInputVisitorData *data, const void *unused) { UserDefOneList *item, *head = NULL; - Error *errp = NULL; + Error *err = NULL; Visitor *v; int i; v = visitor_input_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44 } ]"); - visit_type_UserDefOneList(v, &head, NULL, &errp); - g_assert(!errp); + visit_type_UserDefOneList(v, &head, NULL, &err); + g_assert(!err); g_assert(head != NULL); for (i = 0, item = head; item; item = item->next, i++) { @@ -634,16 +633,16 @@ static void test_visitor_in_errors(TestInputVisitorData *data, const void *unused) { TestStruct *p = NULL; - Error *errp = NULL; + Error *err = NULL; Visitor *v; v = visitor_input_test_init(data, "{ 'integer': false, 'boolean': 'foo', 'string': -42 }"); - visit_type_TestStruct(v, &p, NULL, &errp); - g_assert(errp); + visit_type_TestStruct(v, &p, NULL, &err); + g_assert(err); g_assert(p->string == NULL); - error_free(errp); + error_free(err); g_free(p->string); g_free(p); } diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c index da279713f2..2580f3debf 100644 --- a/tests/test-qmp-output-visitor.c +++ b/tests/test-qmp-output-visitor.c @@ -45,11 +45,11 @@ static void test_visitor_out_int(TestOutputVisitorData *data, const void *unused) { int64_t value = -42; - Error *errp = NULL; + Error *err = NULL; QObject *obj; - visit_type_int(data->ov, &value, NULL, &errp); - g_assert(!errp); + visit_type_int(data->ov, &value, NULL, &err); + g_assert(!err); obj = qmp_output_get_qobject(data->qov); g_assert(obj != NULL); @@ -62,12 +62,12 @@ static void test_visitor_out_int(TestOutputVisitorData *data, static void test_visitor_out_bool(TestOutputVisitorData *data, const void *unused) { - Error *errp = NULL; + Error *err = NULL; bool value = true; QObject *obj; - visit_type_bool(data->ov, &value, NULL, &errp); - g_assert(!errp); + visit_type_bool(data->ov, &value, NULL, &err); + g_assert(!err); obj = qmp_output_get_qobject(data->qov); g_assert(obj != NULL); @@ -81,11 +81,11 @@ static void test_visitor_out_number(TestOutputVisitorData *data, const void *unused) { double value = 3.14; - Error *errp = NULL; + Error *err = NULL; QObject *obj; - visit_type_number(data->ov, &value, NULL, &errp); - g_assert(!errp); + visit_type_number(data->ov, &value, NULL, &err); + g_assert(!err); obj = qmp_output_get_qobject(data->qov); g_assert(obj != NULL); @@ -99,11 +99,11 @@ static void test_visitor_out_string(TestOutputVisitorData *data, const void *unused) { char *string = (char *) "Q E M U"; - Error *errp = NULL; + Error *err = NULL; QObject *obj; - visit_type_str(data->ov, &string, NULL, &errp); - g_assert(!errp); + visit_type_str(data->ov, &string, NULL, &err); + g_assert(!err); obj = qmp_output_get_qobject(data->qov); g_assert(obj != NULL); @@ -117,12 +117,12 @@ static void test_visitor_out_no_string(TestOutputVisitorData *data, const void *unused) { char *string = NULL; - Error *errp = NULL; + Error *err = NULL; QObject *obj; /* A null string should return "" */ - visit_type_str(data->ov, &string, NULL, &errp); - g_assert(!errp); + visit_type_str(data->ov, &string, NULL, &err); + g_assert(!err); obj = qmp_output_get_qobject(data->qov); g_assert(obj != NULL); @@ -135,13 +135,13 @@ static void test_visitor_out_no_string(TestOutputVisitorData *data, static void test_visitor_out_enum(TestOutputVisitorData *data, const void *unused) { - Error *errp = NULL; + Error *err = NULL; QObject *obj; EnumOne i; for (i = 0; i < ENUM_ONE_MAX; i++) { - visit_type_EnumOne(data->ov, &i, "unused", &errp); - g_assert(!errp); + visit_type_EnumOne(data->ov, &i, "unused", &err); + g_assert(!err); obj = qmp_output_get_qobject(data->qov); g_assert(obj != NULL); @@ -156,13 +156,13 @@ static void test_visitor_out_enum_errors(TestOutputVisitorData *data, const void *unused) { EnumOne i, bad_values[] = { ENUM_ONE_MAX, -1 }; - Error *errp; + Error *err; for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) { - errp = NULL; - visit_type_EnumOne(data->ov, &bad_values[i], "unused", &errp); - g_assert(errp); - error_free(errp); + err = NULL; + visit_type_EnumOne(data->ov, &bad_values[i], "unused", &err); + g_assert(err); + error_free(err); } } @@ -193,12 +193,12 @@ static void test_visitor_out_struct(TestOutputVisitorData *data, .boolean = false, .string = (char *) "foo"}; TestStruct *p = &test_struct; - Error *errp = NULL; + Error *err = NULL; QObject *obj; QDict *qdict; - visit_type_TestStruct(data->ov, &p, NULL, &errp); - g_assert(!errp); + visit_type_TestStruct(data->ov, &p, NULL, &err); + g_assert(!err); obj = qmp_output_get_qobject(data->qov); g_assert(obj != NULL); @@ -217,7 +217,7 @@ static void test_visitor_out_struct_nested(TestOutputVisitorData *data, const void *unused) { int64_t value = 42; - Error *errp = NULL; + Error *err = NULL; UserDefNested *ud2; QObject *obj; QDict *qdict, *dict1, *dict2, *dict3, *userdef; @@ -242,8 +242,8 @@ static void test_visitor_out_struct_nested(TestOutputVisitorData *data, ud2->dict1.dict3.userdef2->base->integer = value; ud2->dict1.dict3.string3 = g_strdup(strings[3]); - visit_type_UserDefNested(data->ov, &ud2, "unused", &errp); - g_assert(!errp); + visit_type_UserDefNested(data->ov, &ud2, "unused", &err); + g_assert(!err); obj = qmp_output_get_qobject(data->qov); g_assert(obj != NULL); @@ -283,16 +283,16 @@ static void test_visitor_out_struct_errors(TestOutputVisitorData *data, EnumOne bad_values[] = { ENUM_ONE_MAX, -1 }; UserDefZero b; UserDefOne u = { .base = &b }, *pu = &u; - Error *errp; + Error *err; int i; for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) { - errp = NULL; + err = NULL; u.has_enum1 = true; u.enum1 = bad_values[i]; - visit_type_UserDefOne(data->ov, &pu, "unused", &errp); - g_assert(errp); - error_free(errp); + visit_type_UserDefOne(data->ov, &pu, "unused", &err); + g_assert(err); + error_free(err); } } @@ -328,7 +328,7 @@ static void test_visitor_out_list(TestOutputVisitorData *data, const int max_items = 10; bool value_bool = true; int value_int = 10; - Error *errp = NULL; + Error *err = NULL; QListEntry *entry; QObject *obj; QList *qlist; @@ -345,8 +345,8 @@ static void test_visitor_out_list(TestOutputVisitorData *data, head = p; } - visit_type_TestStructList(data->ov, &head, NULL, &errp); - g_assert(!errp); + visit_type_TestStructList(data->ov, &head, NULL, &err); + g_assert(!err); obj = qmp_output_get_qobject(data->qov); g_assert(obj != NULL); diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c index d406263aee..877e737714 100644 --- a/tests/test-string-input-visitor.c +++ b/tests/test-string-input-visitor.c @@ -54,62 +54,62 @@ static void test_visitor_in_int(TestInputVisitorData *data, const void *unused) { int64_t res = 0, value = -42; - Error *errp = NULL; + Error *err = NULL; Visitor *v; v = visitor_input_test_init(data, "-42"); - visit_type_int(v, &res, NULL, &errp); - g_assert(!errp); + visit_type_int(v, &res, NULL, &err); + g_assert(!err); g_assert_cmpint(res, ==, value); } static void test_visitor_in_bool(TestInputVisitorData *data, const void *unused) { - Error *errp = NULL; + Error *err = NULL; bool res = false; Visitor *v; v = visitor_input_test_init(data, "true"); - visit_type_bool(v, &res, NULL, &errp); - g_assert(!errp); + visit_type_bool(v, &res, NULL, &err); + g_assert(!err); g_assert_cmpint(res, ==, true); visitor_input_teardown(data, unused); v = visitor_input_test_init(data, "yes"); - visit_type_bool(v, &res, NULL, &errp); - g_assert(!errp); + visit_type_bool(v, &res, NULL, &err); + g_assert(!err); g_assert_cmpint(res, ==, true); visitor_input_teardown(data, unused); v = visitor_input_test_init(data, "on"); - visit_type_bool(v, &res, NULL, &errp); - g_assert(!errp); + visit_type_bool(v, &res, NULL, &err); + g_assert(!err); g_assert_cmpint(res, ==, true); visitor_input_teardown(data, unused); v = visitor_input_test_init(data, "false"); - visit_type_bool(v, &res, NULL, &errp); - g_assert(!errp); + visit_type_bool(v, &res, NULL, &err); + g_assert(!err); g_assert_cmpint(res, ==, false); visitor_input_teardown(data, unused); v = visitor_input_test_init(data, "no"); - visit_type_bool(v, &res, NULL, &errp); - g_assert(!errp); + visit_type_bool(v, &res, NULL, &err); + g_assert(!err); g_assert_cmpint(res, ==, false); visitor_input_teardown(data, unused); v = visitor_input_test_init(data, "off"); - visit_type_bool(v, &res, NULL, &errp); - g_assert(!errp); + visit_type_bool(v, &res, NULL, &err); + g_assert(!err); g_assert_cmpint(res, ==, false); } @@ -117,13 +117,13 @@ static void test_visitor_in_number(TestInputVisitorData *data, const void *unused) { double res = 0, value = 3.14; - Error *errp = NULL; + Error *err = NULL; Visitor *v; v = visitor_input_test_init(data, "3.14"); - visit_type_number(v, &res, NULL, &errp); - g_assert(!errp); + visit_type_number(v, &res, NULL, &err); + g_assert(!err); g_assert_cmpfloat(res, ==, value); } @@ -131,13 +131,13 @@ static void test_visitor_in_string(TestInputVisitorData *data, const void *unused) { char *res = NULL, *value = (char *) "Q E M U"; - Error *errp = NULL; + Error *err = NULL; Visitor *v; v = visitor_input_test_init(data, value); - visit_type_str(v, &res, NULL, &errp); - g_assert(!errp); + visit_type_str(v, &res, NULL, &err); + g_assert(!err); g_assert_cmpstr(res, ==, value); g_free(res); @@ -146,7 +146,7 @@ static void test_visitor_in_string(TestInputVisitorData *data, static void test_visitor_in_enum(TestInputVisitorData *data, const void *unused) { - Error *errp = NULL; + Error *err = NULL; Visitor *v; EnumOne i; @@ -155,8 +155,8 @@ static void test_visitor_in_enum(TestInputVisitorData *data, v = visitor_input_test_init(data, EnumOne_lookup[i]); - visit_type_EnumOne(v, &res, NULL, &errp); - g_assert(!errp); + visit_type_EnumOne(v, &res, NULL, &err); + g_assert(!err); g_assert_cmpint(i, ==, res); visitor_input_teardown(data, NULL); diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c index 22363d100f..2af5a21ab5 100644 --- a/tests/test-string-output-visitor.c +++ b/tests/test-string-output-visitor.c @@ -45,11 +45,11 @@ static void test_visitor_out_int(TestOutputVisitorData *data, const void *unused) { int64_t value = -42; - Error *errp = NULL; + Error *err = NULL; char *str; - visit_type_int(data->ov, &value, NULL, &errp); - g_assert(!errp); + visit_type_int(data->ov, &value, NULL, &err); + g_assert(!err); str = string_output_get_string(data->sov); g_assert(str != NULL); @@ -60,12 +60,12 @@ static void test_visitor_out_int(TestOutputVisitorData *data, static void test_visitor_out_bool(TestOutputVisitorData *data, const void *unused) { - Error *errp = NULL; + Error *err = NULL; bool value = true; char *str; - visit_type_bool(data->ov, &value, NULL, &errp); - g_assert(!errp); + visit_type_bool(data->ov, &value, NULL, &err); + g_assert(!err); str = string_output_get_string(data->sov); g_assert(str != NULL); @@ -77,11 +77,11 @@ static void test_visitor_out_number(TestOutputVisitorData *data, const void *unused) { double value = 3.14; - Error *errp = NULL; + Error *err = NULL; char *str; - visit_type_number(data->ov, &value, NULL, &errp); - g_assert(!errp); + visit_type_number(data->ov, &value, NULL, &err); + g_assert(!err); str = string_output_get_string(data->sov); g_assert(str != NULL); @@ -93,11 +93,11 @@ static void test_visitor_out_string(TestOutputVisitorData *data, const void *unused) { char *string = (char *) "Q E M U"; - Error *errp = NULL; + Error *err = NULL; char *str; - visit_type_str(data->ov, &string, NULL, &errp); - g_assert(!errp); + visit_type_str(data->ov, &string, NULL, &err); + g_assert(!err); str = string_output_get_string(data->sov); g_assert(str != NULL); @@ -109,12 +109,12 @@ static void test_visitor_out_no_string(TestOutputVisitorData *data, const void *unused) { char *string = NULL; - Error *errp = NULL; + Error *err = NULL; char *str; /* A null string should return "" */ - visit_type_str(data->ov, &string, NULL, &errp); - g_assert(!errp); + visit_type_str(data->ov, &string, NULL, &err); + g_assert(!err); str = string_output_get_string(data->sov); g_assert(str != NULL); @@ -125,13 +125,13 @@ static void test_visitor_out_no_string(TestOutputVisitorData *data, static void test_visitor_out_enum(TestOutputVisitorData *data, const void *unused) { - Error *errp = NULL; + Error *err = NULL; char *str; EnumOne i; for (i = 0; i < ENUM_ONE_MAX; i++) { - visit_type_EnumOne(data->ov, &i, "unused", &errp); - g_assert(!errp); + visit_type_EnumOne(data->ov, &i, "unused", &err); + g_assert(!err); str = string_output_get_string(data->sov); g_assert(str != NULL); @@ -144,13 +144,13 @@ static void test_visitor_out_enum_errors(TestOutputVisitorData *data, const void *unused) { EnumOne i, bad_values[] = { ENUM_ONE_MAX, -1 }; - Error *errp; + Error *err; for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) { - errp = NULL; - visit_type_EnumOne(data->ov, &bad_values[i], "unused", &errp); - g_assert(errp); - error_free(errp); + err = NULL; + visit_type_EnumOne(data->ov, &bad_values[i], "unused", &err); + g_assert(err); + error_free(err); } } diff --git a/util/cutils.c b/util/cutils.c index b337293239..dbe7412bd8 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -24,6 +24,8 @@ #include "qemu-common.h" #include "qemu/host-utils.h" #include <math.h> +#include <limits.h> +#include <errno.h> #include "qemu/sockets.h" #include "qemu/iov.h" @@ -457,11 +459,16 @@ int parse_uint_full(const char *s, unsigned long long *value, int base) int qemu_parse_fd(const char *param) { - int fd; - char *endptr = NULL; + long fd; + char *endptr; + errno = 0; fd = strtol(param, &endptr, 10); - if (*endptr || (fd == 0 && param == endptr)) { + if (param == endptr /* no conversion performed */ || + errno != 0 /* not representable as long; possibly others */ || + *endptr != '\0' /* final string not empty */ || + fd < 0 /* invalid as file descriptor */ || + fd > INT_MAX /* not representable as int */) { return -1; } return fd; diff --git a/util/error.c b/util/error.c index 2bb42e1c4b..66245ccd1f 100644 --- a/util/error.c +++ b/util/error.c @@ -165,13 +165,13 @@ void error_free(Error *err) } } -void error_propagate(Error **dst_err, Error *local_err) +void error_propagate(Error **dst_errp, Error *local_err) { - if (local_err && dst_err == &error_abort) { + if (local_err && dst_errp == &error_abort) { error_report("%s", error_get_pretty(local_err)); abort(); - } else if (dst_err && !*dst_err) { - *dst_err = local_err; + } else if (dst_errp && !*dst_errp) { + *dst_errp = local_err; } else if (local_err) { error_free(local_err); } diff --git a/util/qemu-option.c b/util/qemu-option.c index 8bbc3ad4a3..324e4c59f7 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -1036,7 +1036,7 @@ static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque) const char *value; int n; - if (!strcmp(key, "id") || error_is_set(state->errp)) { + if (!strcmp(key, "id") || *state->errp) { return; } |