Re: [Qemu-devel] [PATCH 05/11] qerror: Clean up QERR_ macros to expand into a single string

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3587 bytes --]

On 06/13/2015 08:20 AM, Markus Armbruster wrote:
> These macros expand into error class enumeration constant, comma,
> string.  Unclean.  Has been that way since commit 13f59ae.
> 
> The error class is always ERROR_CLASS_GENERIC_ERROR since the previous
> commit.
> 
> Clean up as follows:
> 
> * Prepend every use of a QERR_ macro by ERROR_CLASS_GENERIC_ERROR, and
>   delete it from the QERR_ macro.  No change after preprocessing.
> 
> * Rewrite error_set(ERROR_CLASS_GENERIC_ERROR, ...) into
>   error_setg(...).  Again, no change after preprocessing.

Were these conversions done via coccinelle? If so, it would be nice to
mention the script used in the commit message.  But that doesn't affect
the correctness of the patch itself.

> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

>  util/qemu-option.c               | 22 ++++++++------
>  53 files changed, 362 insertions(+), 358 deletions(-)

Close to 1:1 lineup; difference is due to altered line wraps in a few spots.

> 
> diff --git a/backends/rng-egd.c b/backends/rng-egd.c
> index 849bd7a..225221e 100644
> --- a/backends/rng-egd.c
> +++ b/backends/rng-egd.c
> @@ -140,8 +140,8 @@ static void rng_egd_opened(RngBackend *b, Error **errp)
>      RngEgd *s = RNG_EGD(b);
>  
>      if (s->chr_name == NULL) {
> -        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
> -                  "chardev", "a valid character device");
> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "chardev",
> +                   "a valid character device");

Interesting line rewrap.

> +++ b/backends/rng-random.c
> @@ -74,8 +74,8 @@ static void rng_random_opened(RngBackend *b, Error **errp)
>      RndRandom *s = RNG_RANDOM(b);
>  
>      if (s->filename == NULL) {
> -        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
> -                  "filename", "a valid filename");
> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "filename",
> +                   "a valid filename");

and again


> +++ b/block/quorum.c
> @@ -800,8 +800,8 @@ static int quorum_valid_threshold(int threshold, int num_children, Error **errp)
>  {
>  
>      if (threshold < 1) {
> -        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
> -                  "vote-threshold", "value >= 1");
> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "vote-threshold",
> +                   "value >= 1");

looks like there are several of them. They don't hurt, so I'll quit
pointing them out.


>  
>  #define QERR_PROPERTY_VALUE_OUT_OF_RANGE \
> -    ERROR_CLASS_GENERIC_ERROR, "Property %s.%s doesn't take value %" PRId64 " (minimum: %" PRId64 ", maximum: %" PRId64 ")"
> +    "Property %s.%s doesn't take value %" PRId64 " (minimum: %" PRId64 ", maximum: %" PRId64 ")"

Still a long line; worth wrapping with another \ mid-string?

> +++ b/tpm.c
> @@ -140,20 +140,22 @@ static int configure_tpm(QemuOpts *opts)
>  
>      id = qemu_opts_id(opts);
>      if (id == NULL) {
> -        qerror_report(QERR_MISSING_PARAMETER, "id");
> +        qerror_report(ERROR_CLASS_GENERIC_ERROR, QERR_MISSING_PARAMETER, "id");
>          return 1;

Not quite the same s/error_set/error_setg/ change as everywhere else,
but still correct.

There may be additional merge conflicts depending on timing of getting
this in, but it is mechanical resolution, and the sooner we get this in,
the better.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]