[Qemu-devel] [PATCH RFC] qemu-error: introduce error_report_once

[Qemu-devel] [PATCH RFC] qemu-error: introduce error_report_once

[Peter Xu]

I stole the printk_once() macro.

I always wanted to be able to print some error directly if there is a
buffer to dump, however we can't use error_report() really quite often
when there can be any DDOS attack.  To avoid that, we can introduce a
print-once function for it.

CC: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
We can for sure introduce similar functions for the rest of the
error_*() functions, it's just an idea to see whether we'd like it
in general.
---
 include/qemu/error-report.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
index e1c8ae1a52..efebb80e2c 100644
--- a/include/qemu/error-report.h
+++ b/include/qemu/error-report.h
@@ -44,6 +44,18 @@ void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 void warn_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 void info_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 
+#define error_report_once(fmt, ...)             \
+    ({                                          \
+        static bool __print_once;               \
+        bool __ret_print_once = !__print_once;  \
+                                                \
+        if (!__print_once) {					\
+            __print_once = true;				\
+            error_report(fmt, ##__VA_ARGS__);   \
+        }                                       \
+        unlikely(__ret_print_once);				\
+    })
+
 const char *error_get_progname(void);
 extern bool enable_timestamp_msg;
 
-- 
2.17.0

Re: [Qemu-devel] [PATCH RFC] qemu-error: introduce error_report_once

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180515091356.24106-1-peterx@redhat.com
Subject: [Qemu-devel] [PATCH RFC] qemu-error: introduce error_report_once

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   fbd3a489df..c691c87616  master     -> master
 t [tag update]            patchew/20180515082319.30723-1-kraxel@redhat.com -> patchew/20180515082319.30723-1-kraxel@redhat.com
 * [new tag]               patchew/20180515091356.24106-1-peterx@redhat.com -> patchew/20180515091356.24106-1-peterx@redhat.com
Switched to a new branch 'test'
ad1159c23b qemu-error: introduce error_report_once

=== OUTPUT BEGIN ===
Checking PATCH 1/1: qemu-error: introduce error_report_once...
ERROR: code indent should never use tabs
#30: FILE: include/qemu/error-report.h:52:
+        if (!__print_once) {^I^I^I^I^I\$

ERROR: code indent should never use tabs
#31: FILE: include/qemu/error-report.h:53:
+            __print_once = true;^I^I^I^I\$

ERROR: code indent should never use tabs
#34: FILE: include/qemu/error-report.h:56:
+        unlikely(__ret_print_once);^I^I^I^I\$

total: 3 errors, 0 warnings, 18 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH RFC] qemu-error: introduce error_report_once

[Markus Armbruster]

Peter Xu <peterx@redhat.com> writes:

> I stole the printk_once() macro.
>
> I always wanted to be able to print some error directly if there is a
> buffer to dump, however we can't use error_report() really quite often
> when there can be any DDOS attack.

Got an example?

>                                     To avoid that, we can introduce a
> print-once function for it.
>
> CC: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> We can for sure introduce similar functions for the rest of the
> error_*() functions, it's just an idea to see whether we'd like it
> in general.
> ---
>  include/qemu/error-report.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
> index e1c8ae1a52..efebb80e2c 100644
> --- a/include/qemu/error-report.h
> +++ b/include/qemu/error-report.h
> @@ -44,6 +44,18 @@ void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>  void warn_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>  void info_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>  
> +#define error_report_once(fmt, ...)             \
> +    ({                                          \
> +        static bool __print_once;               \
> +        bool __ret_print_once = !__print_once;  \
> +                                                \
> +        if (!__print_once) {					\
> +            __print_once = true;				\
> +            error_report(fmt, ##__VA_ARGS__);   \
> +        }                                       \
> +        unlikely(__ret_print_once);				\
> +    })
> +
>  const char *error_get_progname(void);
>  extern bool enable_timestamp_msg;

Ignorant question: what's the return value's intended use?

Re: [Qemu-devel] [PATCH RFC] qemu-error: introduce error_report_once

[Peter Xu]

On Tue, May 15, 2018 at 02:02:55PM +0200, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > I stole the printk_once() macro.
> >
> > I always wanted to be able to print some error directly if there is a
> > buffer to dump, however we can't use error_report() really quite often
> > when there can be any DDOS attack.
> 
> Got an example?

There aren't much, but there are still some error_report()s that can
be used in frequently called functions, especially on IO paths.  For
example:


*** hw/usb/dev-storage.c:
usb_msd_handle_data[422]       error_report("usb-msd: Bad CBW size");
usb_msd_handle_data[427]       error_report("usb-msd: Bad signature %08x",
usb_msd_handle_data[434]       error_report("usb-msd: Bad LUN %d", cbw.lun);

*** hw/usb/dev-uas.c:
usb_uas_handle_control[656]    error_report("%s: unhandled control request (req 0x%x, val 0x%x, idx 0x%x",
usb_uas_handle_data[823]       error_report("%s: unknown command iu: id 0x%x",
usb_uas_handle_data[870]       error_report("%s: no inflight request", __func__);
usb_uas_handle_data[888]       error_report("%s: invalid endpoint %d", __func__, p->ep->nr);

*** hw/virtio/vhost-user.c:
vhost_user_read[208]           error_report("Failed to read msg header. Read %d instead of %d."
vhost_user_read[215]           error_report("Failed to read msg header."
vhost_user_read[223]           error_report("Failed to read msg header."
vhost_user_read[234]           error_report("Failed to read msg payload."
process_message_reply[260]     error_report("Received unexpected msg type."
vhost_user_write[302]          error_report("Failed to set msg fds.");
vhost_user_write[308]          error_report("Failed to write msg."
...
slave_read[859]                error_report("Failed to read from slave.");
slave_read[864]                error_report("Failed to read msg header."
slave_read[873]                error_report("Failed to read payload from slave.");
slave_read[885]                error_report("Received unexpected msg type.");
slave_read[910]                error_report("Failed to send msg reply to slave.");
...

I only picked some of the callers, they might not all suite in this
case, but just to show what I mean.

Another example is in VT-d emulation code, we have trace_vtd_error()
tracer.  AFAIU all those places can be replaced by something like
error_report() but trace points are mostly used to avoid DDOS attack,
say, an evil guest might trigger this error tons of times to even use
up a host's disk space (e.g., in libvirt qemu log, since libvirt might
capture that).  However using trace points mean that errors are not
dumped if trace not enabled.

So this print_once() thing will make sure (1) it's on by deffault, so
we can even get something without turning the trace on and
reproducing, and (2) it won't be affected by DDOS attack.

> 
> >                                     To avoid that, we can introduce a
> > print-once function for it.
> >
> > CC: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> > We can for sure introduce similar functions for the rest of the
> > error_*() functions, it's just an idea to see whether we'd like it
> > in general.
> > ---
> >  include/qemu/error-report.h | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
> > index e1c8ae1a52..efebb80e2c 100644
> > --- a/include/qemu/error-report.h
> > +++ b/include/qemu/error-report.h
> > @@ -44,6 +44,18 @@ void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
> >  void warn_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
> >  void info_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
> >  
> > +#define error_report_once(fmt, ...)             \
> > +    ({                                          \
> > +        static bool __print_once;               \
> > +        bool __ret_print_once = !__print_once;  \
> > +                                                \
> > +        if (!__print_once) {					\
> > +            __print_once = true;				\
> > +            error_report(fmt, ##__VA_ARGS__);   \
> > +        }                                       \
> > +        unlikely(__ret_print_once);				\
> > +    })
> > +
> >  const char *error_get_progname(void);
> >  extern bool enable_timestamp_msg;
> 
> Ignorant question: what's the return value's intended use?

For me I don't need it, I just didn't see a reason to remove it - it's
quite cheap as a stack variable (comparing to the heap variable
__print_once which will really consume some byte in the binaries) and
basically it works just like we exported that __print_once when
needed, so I kept it in case people might use it.

One example I can think of is that we can keep some error environment
when the first error happens:

  if (something_wrong_happened) {
     if (error_report_once("blablabla")) {
       /* only cache the first error */
       error_cmd = xxx;
       error_param = xxx;
       ...
     }
  }

Regards,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH RFC] qemu-error: introduce error_report_once

[Markus Armbruster]

Peter Xu <peterx@redhat.com> writes:

> On Tue, May 15, 2018 at 02:02:55PM +0200, Markus Armbruster wrote:
>> Peter Xu <peterx@redhat.com> writes:
>> 
>> > I stole the printk_once() macro.
>> >
>> > I always wanted to be able to print some error directly if there is a
>> > buffer to dump, however we can't use error_report() really quite often
>> > when there can be any DDOS attack.
>> 
>> Got an example?
>
> There aren't much, but there are still some error_report()s that can
> be used in frequently called functions, especially on IO paths.  For
> example:
>
>
> *** hw/usb/dev-storage.c:
> usb_msd_handle_data[422]       error_report("usb-msd: Bad CBW size");
> usb_msd_handle_data[427]       error_report("usb-msd: Bad signature %08x",
> usb_msd_handle_data[434]       error_report("usb-msd: Bad LUN %d", cbw.lun);
>
> *** hw/usb/dev-uas.c:
> usb_uas_handle_control[656]    error_report("%s: unhandled control request (req 0x%x, val 0x%x, idx 0x%x",
> usb_uas_handle_data[823]       error_report("%s: unknown command iu: id 0x%x",
> usb_uas_handle_data[870]       error_report("%s: no inflight request", __func__);
> usb_uas_handle_data[888]       error_report("%s: invalid endpoint %d", __func__, p->ep->nr);
>
> *** hw/virtio/vhost-user.c:
> vhost_user_read[208]           error_report("Failed to read msg header. Read %d instead of %d."
> vhost_user_read[215]           error_report("Failed to read msg header."
> vhost_user_read[223]           error_report("Failed to read msg header."
> vhost_user_read[234]           error_report("Failed to read msg payload."
> process_message_reply[260]     error_report("Received unexpected msg type."
> vhost_user_write[302]          error_report("Failed to set msg fds.");
> vhost_user_write[308]          error_report("Failed to write msg."
> ...
> slave_read[859]                error_report("Failed to read from slave.");
> slave_read[864]                error_report("Failed to read msg header."
> slave_read[873]                error_report("Failed to read payload from slave.");
> slave_read[885]                error_report("Received unexpected msg type.");
> slave_read[910]                error_report("Failed to send msg reply to slave.");
> ...
>
> I only picked some of the callers, they might not all suite in this
> case, but just to show what I mean.
>
> Another example is in VT-d emulation code, we have trace_vtd_error()
> tracer.  AFAIU all those places can be replaced by something like
> error_report() but trace points are mostly used to avoid DDOS attack,
> say, an evil guest might trigger this error tons of times to even use
> up a host's disk space (e.g., in libvirt qemu log, since libvirt might
> capture that).  However using trace points mean that errors are not
> dumped if trace not enabled.
>
> So this print_once() thing will make sure (1) it's on by deffault, so
> we can even get something without turning the trace on and
> reproducing, and (2) it won't be affected by DDOS attack.
>
>> 
>> >                                     To avoid that, we can introduce a
>> > print-once function for it.
>> >
>> > CC: Markus Armbruster <armbru@redhat.com>
>> > Signed-off-by: Peter Xu <peterx@redhat.com>
>> > ---
>> > We can for sure introduce similar functions for the rest of the
>> > error_*() functions, it's just an idea to see whether we'd like it
>> > in general.
>> > ---
>> >  include/qemu/error-report.h | 12 ++++++++++++
>> >  1 file changed, 12 insertions(+)
>> >
>> > diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
>> > index e1c8ae1a52..efebb80e2c 100644
>> > --- a/include/qemu/error-report.h
>> > +++ b/include/qemu/error-report.h
>> > @@ -44,6 +44,18 @@ void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>> >  void warn_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>> >  void info_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>> >  
>> > +#define error_report_once(fmt, ...)             \
>> > +    ({                                          \
>> > +        static bool __print_once;               \
>> > +        bool __ret_print_once = !__print_once;  \
>> > +                                                \
>> > +        if (!__print_once) {					\
>> > +            __print_once = true;				\
>> > +            error_report(fmt, ##__VA_ARGS__);   \
>> > +        }                                       \
>> > +        unlikely(__ret_print_once);				\
>> > +    })
>> > +
>> >  const char *error_get_progname(void);
>> >  extern bool enable_timestamp_msg;
>> 
>> Ignorant question: what's the return value's intended use?
>
> For me I don't need it, I just didn't see a reason to remove it - it's
> quite cheap as a stack variable (comparing to the heap variable
> __print_once which will really consume some byte in the binaries) and
> basically it works just like we exported that __print_once when
> needed, so I kept it in case people might use it.
>
> One example I can think of is that we can keep some error environment
> when the first error happens:
>
>   if (something_wrong_happened) {
>      if (error_report_once("blablabla")) {
>        /* only cache the first error */
>        error_cmd = xxx;
>        error_param = xxx;
>        ...
>      }
>   }

I see.

Add a contract comment (suggest to start with the one next to
error_report()), expand the tabs, replace the reserved identifiers
(caught by patchew; you can use foo_ instead of __foo), throw in at
least one use to serve as a showcase (ideally one demonstrating the
difference to trace points), and consider working some of your
additional rationale into your commit message.

Re: [Qemu-devel] [PATCH RFC] qemu-error: introduce error_report_once

[Eric Blake]

On 05/15/2018 10:56 AM, Markus Armbruster wrote:

>> One example I can think of is that we can keep some error environment
>> when the first error happens:
>>
>>    if (something_wrong_happened) {
>>       if (error_report_once("blablabla")) {
>>         /* only cache the first error */
>>         error_cmd = xxx;
>>         error_param = xxx;
>>         ...
>>       }
>>    }
> 
> I see.
> 
> Add a contract comment (suggest to start with the one next to
> error_report()), expand the tabs, replace the reserved identifiers
> (caught by patchew; you can use foo_ instead of __foo),

Actually, patchew doesn't flag use of leading __ (although maybe it 
should); it was complaining about the use of TAB instead of space.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH RFC] qemu-error: introduce error_report_once

[Peter Xu]

On Tue, May 15, 2018 at 05:56:34PM +0200, Markus Armbruster wrote:

[...]

> I see.
> 
> Add a contract comment (suggest to start with the one next to
> error_report()), expand the tabs, replace the reserved identifiers
> (caught by patchew; you can use foo_ instead of __foo), throw in at
> least one use to serve as a showcase (ideally one demonstrating the
> difference to trace points), and consider working some of your
> additional rationale into your commit message.

Will do.  Thanks Markus.

-- 
Peter Xu

Re: [Qemu-devel] [PATCH RFC] qemu-error: introduce error_report_once

[Eric Blake]

On 05/15/2018 04:13 AM, Peter Xu wrote:
> I stole the printk_once() macro.
> 
> I always wanted to be able to print some error directly if there is a
> buffer to dump, however we can't use error_report() really quite often
> when there can be any DDOS attack.  To avoid that, we can introduce a
> print-once function for it.
> 
> CC: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> We can for sure introduce similar functions for the rest of the
> error_*() functions, it's just an idea to see whether we'd like it
> in general.
> ---
>   include/qemu/error-report.h | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
> index e1c8ae1a52..efebb80e2c 100644
> --- a/include/qemu/error-report.h
> +++ b/include/qemu/error-report.h
> @@ -44,6 +44,18 @@ void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>   void warn_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>   void info_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>   
> +#define error_report_once(fmt, ...)             \
> +    ({                                          \
> +        static bool __print_once;               \

Double-underscore names are reserved for the compiler's use, not ours. 
Better would be naming this:

static bool print_once_;

with a trailing underscore, or at most a single leading underscore.

> +        bool __ret_print_once = !__print_once;  \

Same comment for this variable.

> +                                                \
> +        if (!__print_once) {					\
> +            __print_once = true;				\
> +            error_report(fmt, ##__VA_ARGS__);   \
> +        }                                       \
> +        unlikely(__ret_print_once);				\
> +    })
> +
>   const char *error_get_progname(void);
>   extern bool enable_timestamp_msg;
>   
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH RFC] qemu-error: introduce error_report_once

[Peter Xu]

On Tue, May 15, 2018 at 10:29:39AM -0500, Eric Blake wrote:
> On 05/15/2018 04:13 AM, Peter Xu wrote:
> > I stole the printk_once() macro.
> > 
> > I always wanted to be able to print some error directly if there is a
> > buffer to dump, however we can't use error_report() really quite often
> > when there can be any DDOS attack.  To avoid that, we can introduce a
> > print-once function for it.
> > 
> > CC: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> > We can for sure introduce similar functions for the rest of the
> > error_*() functions, it's just an idea to see whether we'd like it
> > in general.
> > ---
> >   include/qemu/error-report.h | 12 ++++++++++++
> >   1 file changed, 12 insertions(+)
> > 
> > diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
> > index e1c8ae1a52..efebb80e2c 100644
> > --- a/include/qemu/error-report.h
> > +++ b/include/qemu/error-report.h
> > @@ -44,6 +44,18 @@ void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
> >   void warn_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
> >   void info_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
> > +#define error_report_once(fmt, ...)             \
> > +    ({                                          \
> > +        static bool __print_once;               \
> 
> Double-underscore names are reserved for the compiler's use, not ours.
> Better would be naming this:
> 
> static bool print_once_;
> 
> with a trailing underscore, or at most a single leading underscore.
> 
> > +        bool __ret_print_once = !__print_once;  \
> 
> Same comment for this variable.

Sure!

(I am wondering why Linux is always using that way to name lots of
 variables, and I'm surprised that I got 385350 after I run this under
 the Linux repo: 'git grep "__[a-z][a-z]" | wc -l', even considering
 some false positives)

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH RFC] qemu-error: introduce error_report_once

[Eric Blake]

On 05/15/2018 10:07 PM, Peter Xu wrote:

>>> +        static bool __print_once;               \
>>
>> Double-underscore names are reserved for the compiler's use, not ours.
>> Better would be naming this:
>>
>> static bool print_once_;
>>
>> with a trailing underscore, or at most a single leading underscore.
>>
>>> +        bool __ret_print_once = !__print_once;  \
>>
>> Same comment for this variable.
> 
> Sure!
> 
> (I am wondering why Linux is always using that way to name lots of
>   variables, and I'm surprised that I got 385350 after I run this under
>   the Linux repo: 'git grep "__[a-z][a-z]" | wc -l', even considering
>   some false positives)

git grep "\b_[_A-Z]" might be a more precise grep for use of reserved 
identifiers.  The Linux kernel can get away with some uses that qemu 
does not, because it is a monolithic low-level project that is closely 
tied to rather specific compiler behaviors and does not have to port to 
other systems; rather than a user-space application that aims to be 
portable to multiple operating systems, compilers, and libc 
implementations.  Also, grepping for leading double-underscore will have 
hits even in qemu, where we ARE taking advantage of a compiler feature 
(an obvious example: anywhere we #define a macro wrapper around an 
__attribute__ tag - __attribute__ belongs to the namespace reserved for 
the compiler, so it makes sense that turning on that compiler feature 
requires using the compiler's namespace).  Or put another way, grepping 
for the use of reserved identifiers is easy, but grepping for where we 
are inappropriately declaring something that may collide (rather than 
using something that already exists) is a bit harder.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH RFC] qemu-error: introduce error_report_once

[Peter Xu]

On Wed, May 16, 2018 at 09:02:42AM -0500, Eric Blake wrote:
> On 05/15/2018 10:07 PM, Peter Xu wrote:
> 
> > > > +        static bool __print_once;               \
> > > 
> > > Double-underscore names are reserved for the compiler's use, not ours.
> > > Better would be naming this:
> > > 
> > > static bool print_once_;
> > > 
> > > with a trailing underscore, or at most a single leading underscore.
> > > 
> > > > +        bool __ret_print_once = !__print_once;  \
> > > 
> > > Same comment for this variable.
> > 
> > Sure!
> > 
> > (I am wondering why Linux is always using that way to name lots of
> >   variables, and I'm surprised that I got 385350 after I run this under
> >   the Linux repo: 'git grep "__[a-z][a-z]" | wc -l', even considering
> >   some false positives)
> 
> git grep "\b_[_A-Z]" might be a more precise grep for use of reserved
> identifiers.  The Linux kernel can get away with some uses that qemu does
> not, because it is a monolithic low-level project that is closely tied to
> rather specific compiler behaviors and does not have to port to other
> systems; rather than a user-space application that aims to be portable to
> multiple operating systems, compilers, and libc implementations.  Also,
> grepping for leading double-underscore will have hits even in qemu, where we
> ARE taking advantage of a compiler feature (an obvious example: anywhere we
> #define a macro wrapper around an __attribute__ tag - __attribute__ belongs
> to the namespace reserved for the compiler, so it makes sense that turning
> on that compiler feature requires using the compiler's namespace).  Or put
> another way, grepping for the use of reserved identifiers is easy, but
> grepping for where we are inappropriately declaring something that may
> collide (rather than using something that already exists) is a bit harder.

Ah yes, I suppose it'll at least need a whitelist of existing symbols
that are used in compiler's namespace (like, __attribute__) to make
the number more accurate, which does not really suite for a oneliner
any more.

Good to know the reason finally.  Thanks Eric!

-- 
Peter Xu

Re: [Qemu-devel] [PATCH RFC] qemu-error: introduce error_report_once

[Stefan Hajnoczi]

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

On Tue, May 15, 2018 at 05:13:56PM +0800, Peter Xu wrote:
> I stole the printk_once() macro.
> 
> I always wanted to be able to print some error directly if there is a
> buffer to dump, however we can't use error_report() really quite often
> when there can be any DDOS attack.  To avoid that, we can introduce a
> print-once function for it.

I like the idea.  It solves the problem of guest-triggerable error
messages that we should know about for troubleshooting but can't due to
DoS.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH RFC] qemu-error: introduce error_report_once

[Peter Xu]

On Thu, May 31, 2018 at 12:03:17PM +0100, Stefan Hajnoczi wrote:
> On Tue, May 15, 2018 at 05:13:56PM +0800, Peter Xu wrote:
> > I stole the printk_once() macro.
> > 
> > I always wanted to be able to print some error directly if there is a
> > buffer to dump, however we can't use error_report() really quite often
> > when there can be any DDOS attack.  To avoid that, we can introduce a
> > print-once function for it.
> 
> I like the idea.  It solves the problem of guest-triggerable error
> messages that we should know about for troubleshooting but can't due to
> DoS.
> 
> Stefan

Thanks for the positive feedback.  Please feel free to have a look on
the latest version:

Subject: [PATCH v4 0/2] error-report: introduce {error|warn}_report_once
Message-Id: <20180524044454.11792-1-peterx@redhat.com>

Regards,

-- 
Peter Xu