Skip to content

Commit

Permalink
qom: -object error messages lost location, restore it
Browse files Browse the repository at this point in the history
qemu_opts_foreach() runs its callback with the error location set to
the option's location.  Any errors the callback reports use the
option's location automatically.

Commit 90998d5 moved the actual error reporting from "inside"
qemu_opts_foreach() to after it.  Here's a typical hunk:

	 if (qemu_opts_foreach(qemu_find_opts("object"),
    -                          object_create,
    -                          object_create_initial, NULL)) {
    +                          user_creatable_add_opts_foreach,
    +                          object_create_initial, &err)) {
    +        error_report_err(err);
	     exit(1);
	 }

Before, object_create() reports from within qemu_opts_foreach(), using
the option's location.  Afterwards, we do it after
qemu_opts_foreach(), using whatever location happens to be current
there.  Commonly a "none" location.

This is because Error objects don't have location information.
Problematic.

Reproducer:

    $ qemu-system-x86_64 -nodefaults -display none -object secret,id=foo,foo=bar
    qemu-system-x86_64: Property '.foo' not found

Note no location.  This commit restores it:

    qemu-system-x86_64: -object secret,id=foo,foo=bar: Property '.foo' not found

Note that the qemu_opts_foreach() bug just fixed could mask the bug
here: if the location it leaves dangling hasn't been clobbered, yet,
it's the correct one.

Reported-by: Eric Blake <[email protected]>
Cc: Daniel P. Berrange <[email protected]>
Signed-off-by: Markus Armbruster <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Daniel P. Berrange <[email protected]>
Reviewed-by: Eric Blake <[email protected]>
[Paragraph on Error added to commit message]
  • Loading branch information
Markus Armbruster committed Apr 28, 2016
1 parent d9d3aae commit 51b9b47
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 39 deletions.
5 changes: 3 additions & 2 deletions include/qom/object_interfaces.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,16 +140,17 @@ typedef bool (*user_creatable_add_opts_predicate)(const char *type);
* user_creatable_add_opts_foreach:
* @opaque: a user_creatable_add_opts_predicate callback or NULL
* @opts: options to create
* @errp: if an error occurs, a pointer to an area to store the error
* @errp: unused
*
* An iterator callback to be used in conjunction with
* the qemu_opts_foreach() method for creating a list of
* objects from a set of QemuOpts
*
* The @opaque parameter can be passed a user_creatable_add_opts_predicate
* callback to filter which types of object are created during iteration.
* When it fails, report the error.
*
* Returns: 0 on success, -1 on error
* Returns: 0 on success, -1 when an error was reported.
*/
int user_creatable_add_opts_foreach(void *opaque,
QemuOpts *opts, Error **errp);
Expand Down
39 changes: 11 additions & 28 deletions qemu-img.c
Original file line number Diff line number Diff line change
Expand Up @@ -435,8 +435,7 @@ static int img_create(int argc, char **argv)

if (qemu_opts_foreach(&qemu_object_opts,
user_creatable_add_opts_foreach,
NULL, &local_err)) {
error_report_err(local_err);
NULL, NULL)) {
goto fail;
}

Expand Down Expand Up @@ -598,7 +597,6 @@ static int img_check(int argc, char **argv)
bool writethrough;
ImageCheck *check;
bool quiet = false;
Error *local_err = NULL;
bool image_opts = false;

fmt = NULL;
Expand Down Expand Up @@ -679,8 +677,7 @@ static int img_check(int argc, char **argv)

if (qemu_opts_foreach(&qemu_object_opts,
user_creatable_add_opts_foreach,
NULL, &local_err)) {
error_report_err(local_err);
NULL, NULL)) {
return 1;
}

Expand Down Expand Up @@ -871,8 +868,7 @@ static int img_commit(int argc, char **argv)

if (qemu_opts_foreach(&qemu_object_opts,
user_creatable_add_opts_foreach,
NULL, &local_err)) {
error_report_err(local_err);
NULL, NULL)) {
return 1;
}

Expand Down Expand Up @@ -1133,7 +1129,6 @@ static int img_compare(int argc, char **argv)
int64_t nb_sectors;
int c, pnum;
uint64_t progress_base;
Error *local_err = NULL;
bool image_opts = false;

cache = BDRV_DEFAULT_CACHE;
Expand Down Expand Up @@ -1201,8 +1196,7 @@ static int img_compare(int argc, char **argv)

if (qemu_opts_foreach(&qemu_object_opts,
user_creatable_add_opts_foreach,
NULL, &local_err)) {
error_report_err(local_err);
NULL, NULL)) {
ret = 2;
goto out4;
}
Expand Down Expand Up @@ -1864,8 +1858,7 @@ static int img_convert(int argc, char **argv)

if (qemu_opts_foreach(&qemu_object_opts,
user_creatable_add_opts_foreach,
NULL, &local_err)) {
error_report_err(local_err);
NULL, NULL)) {
goto fail_getopt;
}

Expand Down Expand Up @@ -2299,7 +2292,6 @@ static int img_info(int argc, char **argv)
bool chain = false;
const char *filename, *fmt, *output;
ImageInfoList *list;
Error *local_err = NULL;
bool image_opts = false;

fmt = NULL;
Expand Down Expand Up @@ -2363,8 +2355,7 @@ static int img_info(int argc, char **argv)

if (qemu_opts_foreach(&qemu_object_opts,
user_creatable_add_opts_foreach,
NULL, &local_err)) {
error_report_err(local_err);
NULL, NULL)) {
return 1;
}

Expand Down Expand Up @@ -2513,7 +2504,6 @@ static int img_map(int argc, char **argv)
int64_t length;
MapEntry curr = { .length = 0 }, next;
int ret = 0;
Error *local_err = NULL;
bool image_opts = false;

fmt = NULL;
Expand Down Expand Up @@ -2573,8 +2563,7 @@ static int img_map(int argc, char **argv)

if (qemu_opts_foreach(&qemu_object_opts,
user_creatable_add_opts_foreach,
NULL, &local_err)) {
error_report_err(local_err);
NULL, NULL)) {
return 1;
}

Expand Down Expand Up @@ -2717,8 +2706,7 @@ static int img_snapshot(int argc, char **argv)

if (qemu_opts_foreach(&qemu_object_opts,
user_creatable_add_opts_foreach,
NULL, &err)) {
error_report_err(err);
NULL, NULL)) {
return 1;
}

Expand Down Expand Up @@ -2867,8 +2855,7 @@ static int img_rebase(int argc, char **argv)

if (qemu_opts_foreach(&qemu_object_opts,
user_creatable_add_opts_foreach,
NULL, &local_err)) {
error_report_err(local_err);
NULL, NULL)) {
return 1;
}

Expand Down Expand Up @@ -3133,7 +3120,6 @@ static int img_resize(int argc, char **argv)
bool quiet = false;
BlockBackend *blk = NULL;
QemuOpts *param;
Error *local_err = NULL;

static QemuOptsList resize_options = {
.name = "resize_options",
Expand Down Expand Up @@ -3204,8 +3190,7 @@ static int img_resize(int argc, char **argv)

if (qemu_opts_foreach(&qemu_object_opts,
user_creatable_add_opts_foreach,
NULL, &local_err)) {
error_report_err(local_err);
NULL, NULL)) {
return 1;
}

Expand Down Expand Up @@ -3297,7 +3282,6 @@ static int img_amend(int argc, char **argv)
bool quiet = false, progress = false;
BlockBackend *blk = NULL;
BlockDriverState *bs = NULL;
Error *local_err = NULL;
bool image_opts = false;

cache = BDRV_DEFAULT_CACHE;
Expand Down Expand Up @@ -3365,8 +3349,7 @@ static int img_amend(int argc, char **argv)

if (qemu_opts_foreach(&qemu_object_opts,
user_creatable_add_opts_foreach,
NULL, &local_err)) {
error_report_err(local_err);
NULL, NULL)) {
ret = -1;
goto out_no_progress;
}
Expand Down
3 changes: 1 addition & 2 deletions qemu-io.c
Original file line number Diff line number Diff line change
Expand Up @@ -534,8 +534,7 @@ int main(int argc, char **argv)

if (qemu_opts_foreach(&qemu_object_opts,
user_creatable_add_opts_foreach,
NULL, &local_error)) {
error_report_err(local_error);
NULL, NULL)) {
exit(1);
}

Expand Down
3 changes: 1 addition & 2 deletions qemu-nbd.c
Original file line number Diff line number Diff line change
Expand Up @@ -711,8 +711,7 @@ int main(int argc, char **argv)

if (qemu_opts_foreach(&qemu_object_opts,
user_creatable_add_opts_foreach,
NULL, &local_err)) {
error_report_err(local_err);
NULL, NULL)) {
exit(EXIT_FAILURE);
}

Expand Down
4 changes: 3 additions & 1 deletion qom/object_interfaces.c
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp)
{
bool (*type_predicate)(const char *) = opaque;
Object *obj = NULL;
Error *err = NULL;
const char *type;

type = qemu_opt_get(opts, "qom-type");
Expand All @@ -178,8 +179,9 @@ int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp)
return 0;
}

obj = user_creatable_add_opts(opts, errp);
obj = user_creatable_add_opts(opts, &err);
if (!obj) {
error_report_err(err);
return -1;
}
object_unref(obj);
Expand Down
6 changes: 2 additions & 4 deletions vl.c
Original file line number Diff line number Diff line change
Expand Up @@ -4291,8 +4291,7 @@ int main(int argc, char **argv, char **envp)

if (qemu_opts_foreach(qemu_find_opts("object"),
user_creatable_add_opts_foreach,
object_create_initial, &err)) {
error_report_err(err);
object_create_initial, NULL)) {
exit(1);
}

Expand Down Expand Up @@ -4410,8 +4409,7 @@ int main(int argc, char **argv, char **envp)

if (qemu_opts_foreach(qemu_find_opts("object"),
user_creatable_add_opts_foreach,
object_create_delayed, &err)) {
error_report_err(err);
object_create_delayed, NULL)) {
exit(1);
}

Expand Down

0 comments on commit 51b9b47

Please sign in to comment.