git.proxmox.com Git - mirror

error: Eliminate error_propagate() with Coccinelle, part 1

When all we do with an Error we receive into a local variable is
propagating to somewhere else, we can just as well receive it there
right away.  Convert

    if (!foo(..., &err)) {
        ...
        error_propagate(errp, err);
        ...
        return ...
    }

to

    if (!foo(..., errp)) {
        ...
        ...
        return ...
    }

where nothing else needs @err.  Coccinelle script:

    @rule1 forall@
    identifier fun, err, errp, lbl;
    expression list args, args2;
    binary operator op;
    constant c1, c2;
    symbol false;
    @@
         if (
    (
    -        fun(args, &err, args2)
    +        fun(args, errp, args2)
    |
    -        !fun(args, &err, args2)
    +        !fun(args, errp, args2)
    |
    -        fun(args, &err, args2) op c1
    +        fun(args, errp, args2) op c1
    )
            )
         {
             ... when != err
                 when != lbl:
                 when strict
    -        error_propagate(errp, err);
             ... when != err
    (
             return;
    |
             return c2;
    |
             return false;
    )
         }

    @rule2 forall@
    identifier fun, err, errp, lbl;
    expression list args, args2;
    expression var;
    binary operator op;
    constant c1, c2;
    symbol false;
    @@
    -    var = fun(args, &err, args2);
    +    var = fun(args, errp, args2);
         ... when != err
         if (
    (
             var
    |
             !var
    |
             var op c1
    )
            )
         {
             ... when != err
                 when != lbl:
                 when strict
    -        error_propagate(errp, err);
             ... when != err
    (
             return;
    |
             return c2;
    |
             return false;
    |
             return var;
    )
         }

    @depends on rule1 || rule2@
    identifier err;
    @@
    -    Error *err = NULL;
         ... when != err

Not exactly elegant, I'm afraid.

The "when != lbl:" is necessary to avoid transforming

         if (fun(args, &err)) {
             goto out
         }
         ...
     out:
         error_propagate(errp, err);

even though other paths to label out still need the error_propagate().
For an actual example, see sclp_realize().

Without the "when strict", Coccinelle transforms vfio_msix_setup(),
incorrectly.  I don't know what exactly "when strict" does, only that
it helps here.

The match of return is narrower than what I want, but I can't figure
out how to express "return where the operand doesn't use @err".  For
an example where it's too narrow, see vfio_intx_enable().

Silently fails to convert hw/arm/armsse.c, because Coccinelle gets
confused by ARMSSE being used both as typedef and function-like macro
there.  Converted manually.

Line breaks tidied up manually.  One nested declaration of @local_err
deleted manually.  Preexisting unwanted blank line dropped in
hw/riscv/sifive_e.c.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200707160613.848843-35-armbru@redhat.com>

error: Avoid unnecessary error_propagate() after error_setg()

Replace

    error_setg(&err, ...);
    error_propagate(errp, err);

by

    error_setg(errp, ...);

Related pattern:

    if (...) {
        error_setg(&err, ...);
        goto out;
    }
    ...
out:
    error_propagate(errp, err);
    return;

When all paths to label out are that way, replace by

    if (...) {
        error_setg(errp, ...);
        return;
    }

and delete the label along with the error_propagate().

When we have at most one other path that actually needs to propagate,
and maybe one at the end that where propagation is unnecessary, e.g.

    foo(..., &err);
    if (err) {
        goto out;
    }
    ...
    bar(..., &err);
out:
    error_propagate(errp, err);
    return;

move the error_propagate() to where it's needed, like

    if (...) {
        foo(..., &err);
        error_propagate(errp, err);
        return;
    }
    ...
    bar(..., errp);
    return;

and transform the error_setg() as above.

In some places, the transformation results in obviously unnecessary
error_propagate().  The next few commits will eliminate them.

Bonus: the elimination of gotos will make later patches in this series
easier to review.

Candidates for conversion tracked down with this Coccinelle script:

    @@
    identifier err, errp;
    expression list args;
    @@
    -    error_setg(&err, args);
    +    error_setg(errp, args);
         ... when != err
         error_propagate(errp, err);

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200707160613.848843-34-armbru@redhat.com>

qdev: Use returned bool to check for failure, Coccinelle part

The previous commit enables conversion of

    qdev_prop_set_drive_err(..., &err);
    if (err) {
    ...
    }

to

    if (!qdev_prop_set_drive_err(..., errp)) {
    ...
    }

Coccinelle script:

    @@
    identifier fun = qdev_prop_set_drive_err;
    expression list args;
    typedef Error;
    Error *err;
    @@
    -    fun(args, &err);
    -    if (err)
    +    if (!fun(args, &err))
         {
             ...
         }

One line break tidied up manually.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200707160613.848843-33-armbru@redhat.com>

qdev: Make functions taking Error ** return bool, not void

See recent commit "error: Document Error API usage rules" for
rationale.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-32-armbru@redhat.com>

qom: Make functions taking Error ** return bool, not 0/-1

Just for consistency. Also fix the example in object_set_props()'s
documentation.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-31-armbru@redhat.com>

qom: Use returned bool to check for failure, manual part

The previous commit used Coccinelle to convert from checking the Error
object to checking the return value. Convert a few more manually.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-30-armbru@redhat.com>

qom: Use returned bool to check for failure, Coccinelle part

The previous commit enables conversion of

    foo(..., &err);
    if (err) {
        ...
    }

to

    if (!foo(..., errp)) {
        ...
    }

for QOM functions that now return true / false on success / error.
Coccinelle script:

    @@
    identifier fun = {
        object_apply_global_props, object_initialize_child_with_props,
        object_initialize_child_with_propsv, object_property_get,
        object_property_get_bool, object_property_parse, object_property_set,
        object_property_set_bool, object_property_set_int,
        object_property_set_link, object_property_set_qobject,
        object_property_set_str, object_property_set_uint, object_set_props,
        object_set_propv, user_creatable_add_dict,
        user_creatable_complete, user_creatable_del
    };
    expression list args, args2;
    typedef Error;
    Error *err;
    @@
    -    fun(args, &err, args2);
    -    if (err)
    +    if (!fun(args, &err, args2))
         {
             ...
         }

Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by
ARMSSE being used both as typedef and function-like macro there.
Convert manually.

Line breaks tidied up manually.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-29-armbru@redhat.com>

qom: Make functions taking Error ** return bool, not void

See recent commit "error: Document Error API usage rules" for
rationale.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-28-armbru@redhat.com>

qom: Put name parameter before value / visitor parameter

The object_property_set_FOO() setters take property name and value in
an unusual order:

    void object_property_set_FOO(Object *obj, FOO_TYPE value,
                                 const char *name, Error **errp)

Having to pass value before name feels grating.  Swap them.

Same for object_property_set(), object_property_get(), and
object_property_parse().

Convert callers with this Coccinelle script:

    @@
    identifier fun = {
        object_property_get, object_property_parse, object_property_set_str,
        object_property_set_link, object_property_set_bool,
        object_property_set_int, object_property_set_uint, object_property_set,
        object_property_set_qobject
    };
    expression obj, v, name, errp;
    @@
    -    fun(obj, v, name, errp)
    +    fun(obj, name, v, errp)

Chokes on hw/arm/musicpal.c's lcd_refresh() with the unhelpful error
message "no position information".  Convert that one manually.

Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by
ARMSSE being used both as typedef and function-like macro there.
Convert manually.

Fails to convert hw/rx/rx-gdbsim.c, because Coccinelle gets confused
by RXCPU being used both as typedef and function-like macro there.
Convert manually.  The other files using RXCPU that way don't need
conversion.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-27-armbru@redhat.com>
[Straightforwad conflict with commit 2336172d9b "audio: set default
value for pcspk.iobase property" resolved]

qom: Use return values to check for error where that's simpler

When using the Error object to check for error, we need to receive it
into a local variable, then propagate() it to @errp.

Using the return value permits allows receiving it straight to @errp.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-26-armbru@redhat.com>

qom: Don't handle impossible object_property_get_link() failure

Don't handle object_property_get_link() failure that can't happen
unless the programmer screwed up, pass &error_abort.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200707160613.848843-25-armbru@redhat.com>

qom: Crash more nicely on object_property_get_link() failure

Pass &error_abort instead of NULL where the returned value is
dereferenced or asserted to be non-null. Drop a now redundant
assertion.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-24-armbru@redhat.com>

qom: Rename qdev_get_type() to object_get_type()

Commit 2f262e06f0 lifted qdev_get_type() from qdev to object without
renaming it accordingly. Do that now.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-23-armbru@redhat.com>

qom: Use error_reportf_err() instead of g_printerr() in examples

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-22-armbru@redhat.com>

s390x/pci: Fix harmless mistake in zpci's property fid's setter

s390_pci_set_fid() sets zpci->fid_defined to true even when
visit_type_uint32() failed. Reproducer: "-device zpci,fid=junk".
Harmless in practice, because qdev_device_add() then fails, throwing
away @zpci. Fix it anyway.

Cc: Matthew Rosato <mjrosato@linux.ibm.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Message-Id: <20200707160613.848843-21-armbru@redhat.com>

qapi: Use returned bool to check for failure, manual part

The previous commit used Coccinelle to convert from checking the Error
object to checking the return value. Convert a few more manually.
Also tweak control flow in places to conform to the conventional "if
error bail out" pattern.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-20-armbru@redhat.com>

qapi: Use returned bool to check for failure, Coccinelle part

The previous commit enables conversion of

    visit_foo(..., &err);
    if (err) {
        ...
    }

to

    if (!visit_foo(..., errp)) {
        ...
    }

for visitor functions that now return true / false on success / error.
Coccinelle script:

    @@
    identifier fun =~ "check_list|input_type_enum|lv_start_struct|lv_type_bool|lv_type_int64|lv_type_str|lv_type_uint64|output_type_enum|parse_type_bool|parse_type_int64|parse_type_null|parse_type_number|parse_type_size|parse_type_str|parse_type_uint64|print_type_bool|print_type_int64|print_type_null|print_type_number|print_type_size|print_type_str|print_type_uint64|qapi_clone_start_alternate|qapi_clone_start_list|qapi_clone_start_struct|qapi_clone_type_bool|qapi_clone_type_int64|qapi_clone_type_null|qapi_clone_type_number|qapi_clone_type_str|qapi_clone_type_uint64|qapi_dealloc_start_list|qapi_dealloc_start_struct|qapi_dealloc_type_anything|qapi_dealloc_type_bool|qapi_dealloc_type_int64|qapi_dealloc_type_null|qapi_dealloc_type_number|qapi_dealloc_type_str|qapi_dealloc_type_uint64|qobject_input_check_list|qobject_input_check_struct|qobject_input_start_alternate|qobject_input_start_list|qobject_input_start_struct|qobject_input_type_any|qobject_input_type_bool|qobject_input_type_bool_keyval|qobject_input_type_int64|qobject_input_type_int64_keyval|qobject_input_type_null|qobject_input_type_number|qobject_input_type_number_keyval|qobject_input_type_size_keyval|qobject_input_type_str|qobject_input_type_str_keyval|qobject_input_type_uint64|qobject_input_type_uint64_keyval|qobject_output_start_list|qobject_output_start_struct|qobject_output_type_any|qobject_output_type_bool|qobject_output_type_int64|qobject_output_type_null|qobject_output_type_number|qobject_output_type_str|qobject_output_type_uint64|start_list|visit_check_list|visit_check_struct|visit_start_alternate|visit_start_list|visit_start_struct|visit_type_.*";
    expression list args;
    typedef Error;
    Error *err;
    @@
    -    fun(args, &err);
    -    if (err)
    +    if (!fun(args, &err))
         {
             ...
         }

A few line breaks tidied up manually.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-19-armbru@redhat.com>

qapi: Make visitor functions taking Error ** return bool, not void

See recent commit "error: Document Error API usage rules" for
rationale.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-18-armbru@redhat.com>

hmp: Eliminate a variable in hmp_migrate_set_parameter()

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-17-armbru@redhat.com>

block: Avoid error accumulation in bdrv_img_create()

When creating an image fails because the format doesn't support option
"backing_file" or "backing_fmt", bdrv_img_create() first has
qemu_opt_set() put a generic error into @local_err, then puts the real
error into @errp with error_setg(), and then propagates the former to
the latter, which throws away the generic error. A bit complicated,
but works.

Now that qemu_opt_set() returns a useful value, we can simply ignore
the generic error instead.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-16-armbru@redhat.com>

qemu-option: Use returned bool to check for failure

The previous commit enables conversion of

    foo(..., &err);
    if (err) {
        ...
    }

to

    if (!foo(..., &err)) {
        ...
    }

for QemuOpts functions that now return true / false on success /
error.  Coccinelle script:

    @@
    identifier fun = {
        opts_do_parse, parse_option_bool, parse_option_number,
        parse_option_size, qemu_opt_parse, qemu_opt_rename, qemu_opt_set,
        qemu_opt_set_bool, qemu_opt_set_number, qemu_opts_absorb_qdict,
        qemu_opts_do_parse, qemu_opts_from_qdict_entry, qemu_opts_set,
        qemu_opts_validate
    };
    expression list args, args2;
    typedef Error;
    Error *err;
    @@
    -    fun(args, &err, args2);
    -    if (err)
    +    if (!fun(args, &err, args2))
         {
             ...
         }

A few line breaks tidied up manually.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-15-armbru@redhat.com>
[Conflict with commit 0b6786a9c1 "block/amend: refactor qcow2 amend
options" resolved by rerunning Coccinelle on master's version]

qemu-option: Make functions taking Error ** return bool, not void

See recent commit "error: Document Error API usage rules" for
rationale.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-14-armbru@redhat.com>

qemu-option: Replace opt_set() by cleaner opt_validate()

opt_set() frees its argument @value on failure.  Slightly unclean;
functions ideally do nothing on failure.

To tidy this up, move opt_create() from opt_set() into its callers,
along with the cleanup.  Rename opt_set() to opt_validate(), noting
its similarity to qemu_opts_validate().  Drop redundant parameter
@opts; use opt->opts instead.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-13-armbru@redhat.com>

qemu-option: Factor out helper opt_create()

There is just one use so far. The next commit will add more.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-12-armbru@redhat.com>

qemu-option: Simplify around find_default_by_name()

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-11-armbru@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>

qemu-option: Factor out helper find_default_by_name()

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <20200707160613.848843-10-armbru@redhat.com>

qemu-option: Make uses of find_desc_by_name() more similar

This is to make the next commit easier to review.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <20200707160613.848843-9-armbru@redhat.com>

qemu-option: Check return value instead of @err where convenient

Convert uses like

    opts = qemu_opts_create(..., &err);
    if (err) {
        ...
    }

to

    opts = qemu_opts_create(..., errp);
    if (!opts) {
        ...
    }

Eliminate error_propagate() that are now unnecessary.  Delete @err
that are now unused.

Note that we can't drop parallels_open()'s error_propagate() here.  We
continue to execute it even in the converted case.  It's a no-op then:
local_err is null.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <20200707160613.848843-8-armbru@redhat.com>

virtio-crypto-pci: Tidy up virtio_crypto_pci_realize()

virtio_crypto_pci_realize() continues after realization of its
"virtio-crypto-device" fails. Only an object_property_set_link()
follows; looks harmless to me. Tidy up anyway: return after failure,
just like virtio_rng_pci_realize() does.

Cc: "Gonglei (Arei)" <arei.gonglei@huawei.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Gonglei < arei.gonglei@huawei.com>
Message-Id: <20200707160613.848843-7-armbru@redhat.com>

macio: Tidy up error handling in macio_newworld_realize()

macio_newworld_realize() effectively ignores ns->gpio realization
errors, leaking the Error object. Fortunately, macio_gpio_realize()
can't actually fail. Tidy up.

Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <20200707160613.848843-6-armbru@redhat.com>

qdev: Use returned bool to check for qdev_realize() etc. failure

Convert

    foo(..., &err);
    if (err) {
        ...
    }

to

    if (!foo(..., &err)) {
        ...
    }

for qdev_realize(), qdev_realize_and_unref(), qbus_realize() and their
wrappers isa_realize_and_unref(), pci_realize_and_unref(),
sysbus_realize(), sysbus_realize_and_unref(), usb_realize_and_unref().
Coccinelle script:

    @@
    identifier fun = {
        isa_realize_and_unref, pci_realize_and_unref, qbus_realize,
        qdev_realize, qdev_realize_and_unref, sysbus_realize,
        sysbus_realize_and_unref, usb_realize_and_unref
    };
    expression list args, args2;
    typedef Error;
    Error *err;
    @@
    -    fun(args, &err, args2);
    -    if (err)
    +    if (!fun(args, &err, args2))
         {
             ...
         }

Chokes on hw/arm/musicpal.c's lcd_refresh() with the unhelpful error
message "no position information".  Nothing to convert there; skipped.

Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by
ARMSSE being used both as typedef and function-like macro there.
Converted manually.

A few line breaks tidied up manually.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <20200707160613.848843-5-armbru@redhat.com>

error: Document Error API usage rules

This merely codifies existing practice, with one exception: the rule
advising against returning void, where existing practice is mixed.

When the Error API was created, we adopted the (unwritten) rule to
return void when the function returns no useful value on success,
unlike GError, which recommends to return true on success and false on
error then.

When a function returns a distinct error value, say false, a checked
call that passes the error up looks like

    if (!frobnicate(..., errp)) {
        handle the error...
    }

When it returns void, we need

    Error *err = NULL;

    frobnicate(..., &err);
    if (err) {
        handle the error...
        error_propagate(errp, err);
    }

Not only is this more verbose, it also creates an Error object even
when @errp is null, &error_abort or &error_fatal.

People got tired of the additional boilerplate, and started to ignore
the unwritten rule.  The result is confusion among developers about
the preferred usage.

Make the rule advising against returning void official by putting it
in writing.  This will hopefully reduce confusion.

Update the examples accordingly.

The remainder of this series will update a substantial amount of code
to honor the rule.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <20200707160613.848843-4-armbru@redhat.com>
[Tweak prose as per advice from Eric]

error: Improve error.h's big comment

Add headlines to the big comment.

Explain examples for NULL, &error_abort and &error_fatal argument
better.

Tweak rationale for error_propagate_prepend().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200707160613.848843-3-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>

error: Fix examples in error.h's big comment

Mark a bad example more clearly. Fix the error_propagate_prepend()
example. Add a missing declaration and a second error pileup example.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <20200707160613.848843-2-armbru@redhat.com>

Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' into staging

Pull request

Fix for a LTTng Userspace Tracer header problem.

# gpg: Signature made Tue 07 Jul 2020 16:10:04 BST
# gpg:                using RSA key 8695A8BFD3F97CDAAC35775A9CA4ABB381AB73C8
# gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>" [full]
# gpg:                 aka "Stefan Hajnoczi <stefanha@gmail.com>" [full]
# Primary key fingerprint: 8695 A8BF D3F9 7CDA AC35  775A 9CA4 ABB3 81AB 73C8

* remotes/stefanha/tags/tracing-pull-request:
  tracetool: work around ust <sys/sdt.h> include conflict

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Merge remote-tracking branch 'remotes/philmd-gitlab/tags/fw_cfg-20200704' into staging

firmware (and crypto) patches

- add the tls-cipher-suites object,
- add the ability to QOM objects to produce data consumable
  by the fw_cfg device,
- let the tls-cipher-suites object implement the
  FW_CFG_DATA_GENERATOR interface.

This is required by EDK2 'HTTPS Boot' feature of OVMF to tell
the guest which TLS ciphers it can use.

CI jobs results:
  https://travis-ci.org/github/philmd/qemu/builds/704724619
  https://gitlab.com/philmd/qemu/-/pipelines/162938106
  https://cirrus-ci.com/build/4682977303068672

# gpg: Signature made Sat 04 Jul 2020 17:37:08 BST
# gpg:                using RSA key FAABE75E12917221DCFD6BB2E3E32C2CDEADC0DE
# gpg: Good signature from "Philippe Mathieu-Daudé (F4BUG) <f4bug@amsat.org>" [full]
# Primary key fingerprint: FAAB E75E 1291 7221 DCFD  6BB2 E3E3 2C2C DEAD C0DE

* remotes/philmd-gitlab/tags/fw_cfg-20200704:
  crypto/tls-cipher-suites: Produce fw_cfg consumable blob
  softmmu/vl: Allow -fw_cfg 'gen_id' option to use the 'etc/' namespace
  softmmu/vl: Let -fw_cfg option take a 'gen_id' argument
  hw/nvram/fw_cfg: Add the FW_CFG_DATA_GENERATOR interface
  crypto: Add tls-cipher-suites object

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Merge remote-tracking branch 'remotes/kraxel/tags/modules-20200707-pull-request' into staging

qom: add support for qom objects in modules.
build some devices (qxl, virtio-gpu, ccid, usb-redir) as modules.
build braille chardev as module.

v2: more verbose comment for "build: fix device module builds" patch.

note: qemu doesn't rebuild objects on cflags changes (specifically
      -fPIC being added when code is switched from builtin to module).
      Workaround for resulting build errors: "make clean", rebuild.

# gpg: Signature made Tue 07 Jul 2020 14:42:16 BST
# gpg:                using RSA key 4CB6D8EED3E87138
# gpg: Good signature from "Gerd Hoffmann (work) <kraxel@redhat.com>" [full]
# gpg:                 aka "Gerd Hoffmann <gerd@kraxel.org>" [full]
# gpg:                 aka "Gerd Hoffmann (private) <kraxel@gmail.com>" [full]
# Primary key fingerprint: A032 8CFF B93A 17A7 9901  FE7D 4CB6 D8EE D3E8 7138

* remotes/kraxel/tags/modules-20200707-pull-request:
  chardev: enable modules, use for braille
  vga: build virtio-gpu as module
  vga: build virtio-gpu only once
  vga: build qxl as module
  usb: build usb-redir as module
  ccid: build smartcard as module
  build: fix device module builds
  qdev: device module support
  object: qom module support
  module: qom module support

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Merge remote-tracking branch 'remotes/vivier2/tags/trivial-branch-for-5.1-pull-request' into staging

trivial branch patches 20200707

# gpg: Signature made Tue 07 Jul 2020 11:52:06 BST
# gpg:                using RSA key CD2F75DDC8E3A4DC2E4F5173F30C38BD3F2FBE3C
# gpg:                issuer "laurent@vivier.eu"
# gpg: Good signature from "Laurent Vivier <lvivier@redhat.com>" [full]
# gpg:                 aka "Laurent Vivier <laurent@vivier.eu>" [full]
# gpg:                 aka "Laurent Vivier (Red Hat) <lvivier@redhat.com>" [full]
# Primary key fingerprint: CD2F 75DD C8E3 A4DC 2E4F  5173 F30C 38BD 3F2F BE3C

* remotes/vivier2/tags/trivial-branch-for-5.1-pull-request:
  net/tap-solaris.c: Include qemu-common.h for TFR macro
  intel_iommu: "aw-bits" error message still refers to "x-aw-bits"
  util/qemu-option: Document the get_opt_value() function
  MAINTAINERS: Update Radoslaw Biernacki email address
  .mailmap: Update Alexander Graf email address
  trivial: Respect alphabetical order of .o files in Makefile.objs
  fix the prototype of muls64/mulu64

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Merge remote-tracking branch 'remotes/vivier/tags/m68k-next-pull-request' into staging

m68k pull-request 20200706

disable floatx80_invalid_encoding() for m68k
fix m68k_cpu_get_phys_page_debug()

# gpg: Signature made Mon 06 Jul 2020 21:05:33 BST
# gpg:                using RSA key CD2F75DDC8E3A4DC2E4F5173F30C38BD3F2FBE3C
# gpg:                issuer "laurent@vivier.eu"
# gpg: Good signature from "Laurent Vivier <lvivier@redhat.com>" [full]
# gpg:                 aka "Laurent Vivier <laurent@vivier.eu>" [full]
# gpg:                 aka "Laurent Vivier (Red Hat) <lvivier@redhat.com>" [full]
# Primary key fingerprint: CD2F 75DD C8E3 A4DC 2E4F  5173 F30C 38BD 3F2F BE3C

* remotes/vivier/tags/m68k-next-pull-request:
  softfloat,m68k: disable floatx80_invalid_encoding() for m68k
  target/m68k: consolidate physical translation offset into get_physical_address()
  target/m68k: fix physical address translation in m68k_cpu_get_phys_page_debug()

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Merge remote-tracking branch 'remotes/kraxel/tags/audio-20200706-pull-request' into staging

audio: deprecate -soundhw

# gpg: Signature made Mon 06 Jul 2020 20:29:07 BST
# gpg:                using RSA key 4CB6D8EED3E87138
# gpg: Good signature from "Gerd Hoffmann (work) <kraxel@redhat.com>" [full]
# gpg:                 aka "Gerd Hoffmann <gerd@kraxel.org>" [full]
# gpg:                 aka "Gerd Hoffmann (private) <kraxel@gmail.com>" [full]
# Primary key fingerprint: A032 8CFF B93A 17A7 9901  FE7D 4CB6 D8EE D3E8 7138

* remotes/kraxel/tags/audio-20200706-pull-request:
  audio: set default value for pcspk.iobase property
  pcspk: update docs/system/target-i386-desc.rst.inc
  audio: add soundhw deprecation notice
  audio: deprecate -soundhw pcspk
  audio: create pcspk device early
  audio: rework pcspk_init()
  softmmu: initialize spice and audio earlier
  pc_basic_device_init: drop no_vmport arg
  pc_basic_device_init: drop has_pit arg
  pc_basic_device_init: pass PCMachineState
  audio: deprecate -soundhw hda
  audio: deprecate -soundhw sb16
  audio: deprecate -soundhw gus
  audio: deprecate -soundhw cs4231a
  audio: deprecate -soundhw adlib
  audio: deprecate -soundhw es1370
  audio: deprecate -soundhw ac97
  audio: add deprecated_register_soundhw
  stubs: add pci_create_simple
  stubs: add isa_create_simple

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20200706' into staging

Fix for ppc shifts
Fix for non-parallel atomic ops

# gpg: Signature made Mon 06 Jul 2020 19:49:08 BST
# gpg:                using RSA key 7A481E78868B4DB6A85A05C064DF38E8AF7E215F
# gpg:                issuer "richard.henderson@linaro.org"
# gpg: Good signature from "Richard Henderson <richard.henderson@linaro.org>" [full]
# Primary key fingerprint: 7A48 1E78 868B 4DB6 A85A  05C0 64DF 38E8 AF7E 215F

* remotes/rth/tags/pull-tcg-20200706:
  tcg: Fix do_nonatomic_op_* vs signed operations
  tcg/ppc: Sanitize immediate shifts

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2020-07-06' into staging

Block patches for 5.1:
- LUKS keyslot amendment
  (+ patches to make the iotests pass on non-Linux systems, and to keep
     the tests passing for qcow v1, and to skip LUKS tests (including
     qcow2 LUKS) when the built qemu does not support it)
- Refactoring in the block layer: Drop the basically unnecessary
  unallocated_blocks_are_zero field from BlockDriverInfo
- Fix qcow2 preallocation when the image size is not a multiple of the
  cluster size
- Fix in block-copy code

# gpg: Signature made Mon 06 Jul 2020 11:02:53 BST
# gpg:                using RSA key 91BEB60A30DB3E8857D11829F407DB0061D5CF40
# gpg:                issuer "mreitz@redhat.com"
# gpg: Good signature from "Max Reitz <mreitz@redhat.com>" [full]
# Primary key fingerprint: 91BE B60A 30DB 3E88 57D1  1829 F407 DB00 61D5 CF40

* remotes/maxreitz/tags/pull-block-2020-07-06: (31 commits)
  qed: Simplify backing reads
  block: drop unallocated_blocks_are_zero
  block/vhdx: drop unallocated_blocks_are_zero
  block/file-posix: drop unallocated_blocks_are_zero
  block/iscsi: drop unallocated_blocks_are_zero
  block/crypto: drop unallocated_blocks_are_zero
  block/vpc: return ZERO block-status when appropriate
  block/vdi: return ZERO block-status when appropriate
  block: inline bdrv_unallocated_blocks_are_zero()
  qemu-img: convert: don't use unallocated_blocks_are_zero
  iotests: add tests for blockdev-amend
  block/qcow2: implement blockdev-amend
  block/crypto: implement blockdev-amend
  block/core: add generic infrastructure for x-blockdev-amend qmp command
  iotests: qemu-img tests for luks key management
  block/qcow2: extend qemu-img amend interface with crypto options
  block/crypto: implement the encryption key management
  block/crypto: rename two functions
  block/amend: refactor qcow2 amend options
  block/amend: separate amend and create options for qemu-img
  ...

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging

virtio,acpi: features, fixes, cleanups.

vdpa support
virtio-mem support
a handy script for disassembling acpi tables
misc fixes and cleanups

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
# gpg: Signature made Tue 07 Jul 2020 13:00:35 BST
# gpg:                using RSA key 5D09FD0871C8F85B94CA8A0D281F0DB8D28D5469
# gpg:                issuer "mst@redhat.com"
# gpg: Good signature from "Michael S. Tsirkin <mst@kernel.org>" [full]
# gpg:                 aka "Michael S. Tsirkin <mst@redhat.com>" [full]
# Primary key fingerprint: 0270 606B 6F3C DF3D 0B17  0970 C350 3912 AFBE 8E67
#      Subkey fingerprint: 5D09 FD08 71C8 F85B 94CA  8A0D 281F 0DB8 D28D 5469

* remotes/mst/tags/for_upstream: (41 commits)
  vhost-vdpa: introduce vhost-vdpa net client
  vhost-vdpa: introduce vhost-vdpa backend
  vhost_net: introduce set_config & get_config
  vhost: implement vhost_force_iommu method
  vhost: introduce new VhostOps vhost_force_iommu
  vhost: implement vhost_vq_get_addr method
  vhost: introduce new VhostOps vhost_vq_get_addr
  vhost: implement vhost_dev_start method
  vhost: introduce new VhostOps vhost_dev_start
  vhost: check the existence of vhost_set_iotlb_callback
  virtio-pci: implement queue_enabled method
  virtio-bus: introduce queue_enabled method
  vhost_net: use the function qemu_get_peer
  net: introduce qemu_get_peer
  MAINTAINERS: add VT-d entry
  docs: vhost-user: add Virtio status protocol feature
  tests/acpi: remove stale allowed tables
  numa: Auto-enable NUMA when any memory devices are possible
  virtio-mem: Exclude unplugged memory during migration
  virtio-mem: Add trace events
  ...

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
# Conflicts:
# hw/arm/virt.c
# hw/virtio/trace-events

tracetool: work around ust <sys/sdt.h> include conflict

Both the dtrace and ust backends may include <sys/sdt.h> but LTTng
Userspace Tracer 2.11 and later requires SDT_USE_VARIADIC to be defined
before including the header file.

This is a classic problem with C header files included from different
parts of a program. If the same header is included twice within the same
compilation unit then the first inclusion determines the macro
environment.

Work around this by defining SDT_USE_VARIADIC in the dtrace backend too.
It doesn't hurt and fixes a missing STAP_PROBEV() compiler error when
the ust backend is enabled together with the dtrace backend.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20200625140757.237012-1-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

chardev: enable modules, use for braille

Removes brlapi library dependency from core qemu.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200624131045.14512-11-kraxel@redhat.com

vga: build virtio-gpu as module

Drops libvirglrenderer.so dependency from core qemu.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200624131045.14512-10-kraxel@redhat.com

vga: build virtio-gpu only once

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20200624131045.14512-9-kraxel@redhat.com

vga: build qxl as module

First step in making spice support modular.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200624131045.14512-8-kraxel@redhat.com

usb: build usb-redir as module

Drops libusbredirparser.so dependency from core qemu.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200624131045.14512-7-kraxel@redhat.com

ccid: build smartcard as module

Drops libcacard.so dependency from core qemu.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200624131045.14512-6-kraxel@redhat.com

build: fix device module builds

Slightly hackish workaround, works ok as long as we don't
have target-specific modules. meson will obsolete this.

See comment in the patch for the --verbose description.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200624131045.14512-5-kraxel@redhat.com

[ kraxel: updated comment from discussions ]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

qdev: device module support

Hook module loading into the places where we
need it when building devices as modules.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200624131045.14512-4-kraxel@redhat.com

object: qom module support

Little helper function to load modules on demand. In most cases adding
module loading support for devices and other objects is just
s/object_class_by_name/module_object_class_by_name/ in the right spot.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200624131045.14512-3-kraxel@redhat.com

module: qom module support

Add support for qom types provided by modules. For starters use a
manually maintained list which maps qom type to module and prefix.

Two load functions are added: One to load the module for a specific
type, and one to load all modules (needed for object/device lists as
printed by -- for example -- qemu -device help).

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200624131045.14512-2-kraxel@redhat.com

vhost-vdpa: introduce vhost-vdpa net client

This patch set introduces a new net client type: vhost-vdpa.
vhost-vdpa net client will set up a vDPA device which is specified
by a "vhostdev" parameter.

Signed-off-by: Lingshan Zhu <lingshan.zhu@intel.com>
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Signed-off-by: Cindy Lu <lulu@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
Message-Id: <20200701145538.22333-15-lulu@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>

vhost-vdpa: introduce vhost-vdpa backend

Currently we have 2 types of vhost backends in QEMU: vhost kernel and
vhost-user. The above patch provides a generic device for vDPA purpose,
this vDPA device exposes to user space a non-vendor-specific configuration
interface for setting up a vhost HW accelerator, this patch set introduces
a third vhost backend called vhost-vdpa based on the vDPA interface.

Vhost-vdpa usage:

qemu-system-x86_64 -cpu host -enable-kvm \
    ......
    -netdev type=vhost-vdpa,vhostdev=/dev/vhost-vdpa-id,id=vhost-vdpa0 \
    -device virtio-net-pci,netdev=vhost-vdpa0,page-per-vq=on \

Signed-off-by: Lingshan zhu <lingshan.zhu@intel.com>
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Signed-off-by: Cindy Lu <lulu@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
Message-Id: <20200701145538.22333-14-lulu@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>

Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2020-07-06' into staging

* Fuzzer fixes from Alexander
* Clean-up patches for qtests, configure and mcf5206
* Sparc64 sun4u acceptance test

# gpg: Signature made Mon 06 Jul 2020 08:34:14 BST
# gpg:                using RSA key 27B88847EEE0250118F3EAB92ED9D774FE702DB5
# gpg:                issuer "thuth@redhat.com"
# gpg: Good signature from "Thomas Huth <th.huth@gmx.de>" [full]
# gpg:                 aka "Thomas Huth <thuth@redhat.com>" [full]
# gpg:                 aka "Thomas Huth <huth@tuxfamily.org>" [full]
# gpg:                 aka "Thomas Huth <th.huth@posteo.de>" [unknown]
# Primary key fingerprint: 27B8 8847 EEE0 2501 18F3  EAB9 2ED9 D774 FE70 2DB5

* remotes/huth-gitlab/tags/pull-request-2020-07-06:
  tests/acceptance: Add a test for the sun4u sparc64 machine
  hw/m68k/mcf5206: Replace remaining hw_error()s by qemu_log_mask()
  configure / util: Auto-detect the availability of openpty()
  tests/qtest: Unify the test for the xenfv and xenpv machines
  fuzz: do not use POSIX shm for coverage bitmap
  fuzz: fix broken qtest check at rcu_disable_atfork

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

net/tap-solaris.c: Include qemu-common.h for TFR macro

In commit a8d2532645cf5ce4 we cleaned up usage of the qemu-common.h header
so that it was always included from .c files and never from other .h files.
We missed adding it to net/tap-solaris.c (which previously was pulling it
in via tap-int.h), which broke building on Solaris hosts.

Fixes: a8d2532645cf5ce4
Reported-by: Michele Denber <denber@mindspring.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Tested-by: Michele Denber <denber@mindspring.com>
Message-Id: <20200704092317.12943-1-peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>

intel_iommu: "aw-bits" error message still refers to "x-aw-bits"

Commit 4b49b586c4 ('intel_iommu: remove "x-" prefix for "aw-bits"')
removed the "x-" prefix but but didn't update the error message
accordingly.

Signed-off-by: Menno Lageman <menno.lageman@oracle.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20200625155258.1452425-1-menno.lageman@oracle.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>

util/qemu-option: Document the get_opt_value() function

Coverity noticed commit 950c4e6c94 introduced a dereference before
null check in get_opt_value (CID1391003):

In get_opt_value: All paths that lead to this null pointer
comparison already dereference the pointer earlier (CWE-476)

We fixed this in commit 6e3ad3f0e31, but relaxed the check in commit
0c2f6e7ee99 because "No callers of get_opt_value() pass in a NULL
for the 'value' parameter".

Since this function is publicly exposed, it risks new users to do
the same error again. Avoid that documenting the 'value' argument
must not be NULL.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20200629070858.19850-1-philmd@redhat.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>

MAINTAINERS: Update Radoslaw Biernacki email address

My Linaro account is no longer active and stop forwarding emails to me.
Changing it to my current employer domain.

Signed-off-by: Radoslaw Biernacki <rad@semihalf.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: Leif Lindholm <leif@nuviainc.com>
Message-Id: <20200512170704.9290-1-rad@semihalf.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>

.mailmap: Update Alexander Graf email address

Update Alexander Graf email address to avoid emails bouncing.

Suggested-by: Alexander Graf <agraf@csgraf.de>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alexander Graf <agraf@csgraf.de>
Message-Id: <20200702173818.14651-2-f4bug@amsat.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>

trivial: Respect alphabetical order of .o files in Makefile.objs

The vmgenid.o is the only file that is not in alphabetical order.

Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200629094934.2081180-1-dinechin@redhat.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>

softfloat,m68k: disable floatx80_invalid_encoding() for m68k

According to the comment, this definition of invalid encoding is given
by intel developer's manual, and doesn't comply with 680x0 FPU.

With m68k, the explicit integer bit can be zero in the case of:
- zeros                (exp == 0, mantissa == 0)
- denormalized numbers (exp == 0, mantissa != 0)
- unnormalized numbers (exp != 0, exp < 0x7FFF)
- infinities           (exp == 0x7FFF, mantissa == 0)
- not-a-numbers        (exp == 0x7FFF, mantissa != 0)

For infinities and NaNs, the explicit integer bit can be either one or
zero.

The IEEE 754 standard does not define a zero integer bit. Such a number
is an unnormalized number. Hardware does not directly support
denormalized and unnormalized numbers, but implicitly supports them by
trapping them as unimplemented data types, allowing efficient conversion
in software.

See "M68000 FAMILY PROGRAMMER’S REFERENCE MANUAL",
    "1.6 FLOATING-POINT DATA TYPES"

We will implement in the m68k TCG emulator the FP_UNIMP exception to
trap into the kernel to normalize the number. In case of linux-user,
the number will be normalized by QEMU.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200612140400.2130118-1-laurent@vivier.eu>

target/m68k: consolidate physical translation offset into get_physical_address()

Since all callers to get_physical_address() now apply the same page offset to
the translation result, move the logic into get_physical_address() itself to
avoid duplication.

Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20200701201531.13828-3-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>

target/m68k: fix physical address translation in m68k_cpu_get_phys_page_debug()

The result of the get_physical_address() function should be combined with the
offset of the original page access before being returned. Otherwise the
m68k_cpu_get_phys_page_debug() function can round to the wrong page causing
incorrect lookups in gdbstub and various "Disassembler disagrees with
translator over instruction decoding" warnings to appear at translation time.

Fixes: 88b2fef6c3 ("target/m68k: add MC68040 MMU")
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20200701201531.13828-2-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>

tcg: Fix do_nonatomic_op_* vs signed operations

The smin/smax/umin/umax operations require the operands to be
properly sign extended. Do not drop the MO_SIGN bit from the
load, and additionally extend the val input.

Reviewed-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
Reported-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20200701165646.1901320-1-richard.henderson@linaro.org>

tcg/ppc: Sanitize immediate shifts

Sanitize shift constants so that shift operations with
large constants don't generate invalid instructions.

Signed-off-by: Catherine A. Frederick <chocola@animebitch.es>
Message-Id: <20200607211100.22858-1-agrecascino123@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

fix the prototype of muls64/mulu64

The prototypes of muls64/mulu64 in host-utils.h should match the
definitions in host-utils.c

Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
Message-Id: <20200701234344.91843-10-ljp@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>

audio: set default value for pcspk.iobase property

Allows dropping the explicit qdev_prop_set_uint32 call in pcspk_init.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-21-kraxel@redhat.com

pcspk: update docs/system/target-i386-desc.rst.inc

Add PC speaker with config hints.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-20-kraxel@redhat.com

audio: add soundhw deprecation notice

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-19-kraxel@redhat.com

audio: deprecate -soundhw pcspk

Add deprecation message to the audio init function.

Factor out audio initialization and call that from
both audio init and realize, so setting the audiodev
property is enough to properly initialize pcspk.

Add a property alias to the machine type to set the
audio device, so pcspk can be initialized using:
"-machine pcspk-audiodev=<name>"

Using "-global isa-pcspk.audiodev=<name>" works too but
is not recommended.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-18-kraxel@redhat.com

audio: create pcspk device early

Create the pcspk device early, so it exists at
machine type initialization time.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-17-kraxel@redhat.com

audio: rework pcspk_init()

Instead of creating and returning the pc speaker accept it as argument.
That allows to rework the initialization workflow in followup patches.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-16-kraxel@redhat.com

softmmu: initialize spice and audio earlier

audiodev must be initialized before machine_set_property
so the machine can have audiodev property aliases.

spice must initialize before audiodev because the default
audiodev is spice only in case spice is actually enabled.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-15-kraxel@redhat.com

pc_basic_device_init: drop no_vmport arg

Now that we pass pcms anyway, we don't need the no_vmport arg any more.
No functional change.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-14-kraxel@redhat.com

pc_basic_device_init: drop has_pit arg

Now that we pass pcms anyway, we don't need the has_pit arg any more.
No functional change.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-13-kraxel@redhat.com

pc_basic_device_init: pass PCMachineState

Need access to pcms for pcspk initialization.
Just preparation, no functional change.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-12-kraxel@redhat.com

audio: deprecate -soundhw hda

Add deprecation message to the audio init function.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-11-kraxel@redhat.com

audio: deprecate -soundhw sb16

Switch to deprecated_register_soundhw().
Remove the now obsolete init function.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-10-kraxel@redhat.com

audio: deprecate -soundhw gus

Switch to deprecated_register_soundhw().
Remove the now obsolete init function.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-9-kraxel@redhat.com

audio: deprecate -soundhw cs4231a

Switch to deprecated_register_soundhw().
Remove the now obsolete init function.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-8-kraxel@redhat.com

audio: deprecate -soundhw adlib

Switch to deprecated_register_soundhw().
Remove the now obsolete init function.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-7-kraxel@redhat.com

audio: deprecate -soundhw es1370

Switch to deprecated_register_soundhw(). Remove the now obsolete init
function. Add an alias so both es1370 and ES1370 are working with
-device.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-6-kraxel@redhat.com

audio: deprecate -soundhw ac97

Switch to deprecated_register_soundhw(). Remove the now obsolete init
function. Add an alias so both ac97 and AC97 are working with -device.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-5-kraxel@redhat.com

audio: add deprecated_register_soundhw

Add helper function for -soundhw deprecation. It can replace the
simple init functions which just call {isa,pci}_create_simple()
with a hardcoded type. It also prints a deprecation message.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200702132525.6849-4-kraxel@redhat.com

stubs: add pci_create_simple

Needed for -soundhw cleanup.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20200702132525.6849-3-kraxel@redhat.com

stubs: add isa_create_simple

Needed for -soundhw cleanup.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20200702132525.6849-2-kraxel@redhat.com

Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-5.1-pull-request' into staging

linux-user pull request 2020-07-02

Update linux-user maintainer
Improve strace output for some syscalls
Display contents of ioctl() parameters
Fix sparc64 flushw operation

# gpg: Signature made Sat 04 Jul 2020 17:25:21 BST
# gpg:                using RSA key CD2F75DDC8E3A4DC2E4F5173F30C38BD3F2FBE3C
# gpg:                issuer "laurent@vivier.eu"
# gpg: Good signature from "Laurent Vivier <lvivier@redhat.com>" [full]
# gpg:                 aka "Laurent Vivier <laurent@vivier.eu>" [full]
# gpg:                 aka "Laurent Vivier (Red Hat) <lvivier@redhat.com>" [full]
# Primary key fingerprint: CD2F 75DD C8E3 A4DC 2E4F  5173 F30C 38BD 3F2F BE3C

* remotes/vivier2/tags/linux-user-for-5.1-pull-request:
  MAINTAINERS: update linux-user maintainer
  linux-user: Add strace support for printing arguments of ioctl()
  linux-user: Add thunk argument types for SIOCGSTAMP and SIOCGSTAMPNS
  linux-user: Add strace support for printing arguments of fallocate()
  linux-user: Add strace support for printing arguments of chown()/lchown()
  linux-user: Add strace support for printing arguments of lseek()
  linux-user: Add strace support for printing argument of syscalls used for extended attributes
  linux-user: Add strace support for a group of syscalls
  linux-user: Extend strace support to enable argument printing after syscall execution
  linux-user: syscall: ioctls: support DRM_IOCTL_VERSION
  linux-user/sparc64: Fix the handling of window spill trap
  target/sparc: Translate flushw opcode

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

qed: Simplify backing reads

The other four drivers that support backing files (qcow, qcow2,
parallels, vmdk) all rely on the block layer to populate zeroes when
reading beyond EOF of a short backing file. We can simplify the qed
code by doing likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200528094405.145708-11-vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>

block: drop unallocated_blocks_are_zero

Currently this field only set by qed and qcow2. But in fact, all
backing-supporting formats (parallels, qcow, qcow2, qed, vmdk) share
these semantics: on unallocated blocks, if there is no backing file they
just memset the buffer with zeroes.

So, document this behavior for .supports_backing and drop
.unallocated_blocks_are_zero

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200528094405.145708-10-vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>

block/vhdx: drop unallocated_blocks_are_zero

vhdx doesn't have .bdrv_co_block_status handler, so DATA|ALLOCATED is
always assumed for it in bdrv_co_block_status().
unallocated_blocks_are_zero is useless (it doesn't affect the only user
of the field: bdrv_co_block_status()), drop it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200528094405.145708-9-vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>

block/file-posix: drop unallocated_blocks_are_zero

raw_co_block_status() in block/file-posix.c never returns 0, so
unallocated_blocks_are_zero is useless (it doesn't affect the only user
of the field: bdrv_co_block_status()). Drop it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200528094405.145708-8-vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>

block/iscsi: drop unallocated_blocks_are_zero

We set bdi->unallocated_blocks_are_zero = iscsilun->lbprz, but
iscsi_co_block_status doesn't return 0 in case of iscsilun->lbprz, it
returns ZERO when appropriate. So actually unallocated_blocks_are_zero
is useless (it doesn't affect the only user of the field:
bdrv_co_block_status()). Drop it now.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200528094405.145708-7-vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>

block/crypto: drop unallocated_blocks_are_zero

It's false by default, no needs to set it. We are going to drop this
variable at all, so drop it now here, it doesn't hurt.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200528094405.145708-6-vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>

block/vpc: return ZERO block-status when appropriate

In case when get_image_offset() returns -1, we do zero out the
corresponding chunk of qiov. So, this should be reported as ZERO.

Note that this changes visible output of "qemu-img map --output=json"
and "qemu-io -c map" commands. For qemu-img map, the change is obvious:
we just mark as zero what is really zero. For qemu-io it's less
obvious: what was unallocated now is allocated.

There is an inconsistency in understanding of unallocated regions in
Qemu: backing-supporting format-drivers return 0 block-status to report
go-to-backing logic for this area. Some protocol-drivers (iscsi) return
0 to report fs-unallocated-non-zero status (i.e., don't occupy space on
disk, read result is undefined).

BDRV_BLOCK_ALLOCATED is defined as something more close to
go-to-backing logic. Still it is calculated as ZERO | DATA, so 0 from
iscsi is treated as unallocated. It doesn't influence backing-chain
behavior, as iscsi can't have backing file. But it does influence
"qemu-io -c map".

We should solve this inconsistency at some future point. Now, let's
just make backing-not-supporting format drivers (vdi in the previous
patch and vpc now) to behave more like backing-supporting drivers
and not report 0 block-status. More over, returning ZERO status is
absolutely valid thing, and again, corresponds to how the other
format-drivers (backing-supporting) work.

After block-status update, it never reports 0, so setting
unallocated_blocks_are_zero doesn't make sense (as the only user of it
is bdrv_co_block_status and it checks unallocated_blocks_are_zero only
for unallocated areas). Drop it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200528094405.145708-5-vsementsov@virtuozzo.com>
[mreitz: qemu-io -c map as used by iotest 146 now reports everything as
allocated; in order to make the test do something useful, we
use qemu-img map --output=json now]
Signed-off-by: Max Reitz <mreitz@redhat.com>

tests/acceptance: Add a test for the sun4u sparc64 machine

We can use the image from the advent calendar 2018 to test the sun4u
machine. It's not using the "QEMU advent calendar" string, so we can
not use the do_test_advcal_2018() from boot_linux_console.py, thus
let's also put it into a separate file to also be able to add an
entry to the MAINTAINERS file.

Message-Id: <20200704173519.26087-1-thuth@redhat.com>
Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>

hw/m68k/mcf5206: Replace remaining hw_error()s by qemu_log_mask()

hw_error() dumps the CPU state and exits QEMU. This is ok during initial
code development (to see where the guest code is currently executing),
but it is certainly not the desired behavior that we want to present to
normal users, and it can also cause trouble when e.g. fuzzing devices.
Thus let's replace these hw_error()s by qemu_log_mask()s instead.

Message-Id: <20200611055807.15921-1-huth@tuxfamily.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Thomas Huth <huth@tuxfamily.org>

block/vdi: return ZERO block-status when appropriate

In case of !VDI_IS_ALLOCATED[], we do zero out the corresponding chunk
of qiov. So, this should be reported as ZERO.

Note that this changes visible output of "qemu-img map --output=json"
and "qemu-io -c map" commands. For qemu-img map, the change is obvious:
we just mark as zero what is really zero. For qemu-io it's less
obvious: what was unallocated now is allocated.

There is an inconsistency in understanding of unallocated regions in
Qemu: backing-supporting format-drivers return 0 block-status to report
go-to-backing logic for this area. Some protocol-drivers (iscsi) return
0 to report fs-unallocated-non-zero status (i.e., don't occupy space on
disk, read result is undefined).

BDRV_BLOCK_ALLOCATED is defined as something more close to
go-to-backing logic. Still it is calculated as ZERO | DATA, so 0 from
iscsi is treated as unallocated. It doesn't influence backing-chain
behavior, as iscsi can't have backing file. But it does influence
"qemu-io -c map".

We should solve this inconsistency at some future point. Now, let's
just make backing-not-supporting format drivers (vdi at this patch and
vpc with the following) to behave more like backing-supporting drivers
and not report 0 block-status. More over, returning ZERO status is
absolutely valid thing, and again, corresponds to how the other
format-drivers (backing-supporting) work.

After block-status update, it never reports 0, so setting
unallocated_blocks_are_zero doesn't make sense (as the only user of it
is bdrv_co_block_status and it checks unallocated_blocks_are_zero only
for unallocated areas). Drop it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200528094405.145708-4-vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>