All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Halil Pasic <pasic@linux.ibm.com>
Cc: "Peter Xu" <peterx@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	qemu-devel@nongnu.org, "Philippe Mathieu-Daudé" <f4bug@amsat.org>
Subject: Re: [Qemu-devel] [PATCH v4 1/2] qemu-error: introduce {error|warn}_report_once
Date: Wed, 30 May 2018 18:19:18 +0300	[thread overview]
Message-ID: <20180530181734-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <4bee280d-f047-5395-895f-0c96bce398d7@linux.ibm.com>

On Wed, May 30, 2018 at 05:15:19PM +0200, Halil Pasic wrote:
> 
> 
> On 05/30/2018 06:47 AM, Michael S. Tsirkin wrote:
> > On Thu, May 24, 2018 at 12:44:53PM +0800, Peter Xu wrote:
> > > There are many error_report()s that can be used in frequently called
> > > functions, especially on IO paths.  That can be unideal in that
> > > malicious guest can try to trigger the error tons of time which might
> > > use up the log space on the host (e.g., libvirt can capture the stderr
> > > of QEMU and put it persistently onto disk).
> > 
> > I think the problem is real enough but I think the API
> > isn't great as it stresses the mechanism. Which fundamentally does
> > not matter - we can print once or 10 times, or whatever.
> > 
> > What happens here is a guest bug as opposed to hypervisor
> > bug. So I think a better name would be guest_error.
> 
> I don't agree with your argument against the name report_once
> Michael. In my reading the commit message describes one of use
> cases for which the infrastructure introduced by this patch is
> a supposed to be a good fit. But report_once is not restricted
> to this example.

All I'm saying is that we should distinguish between
guest and host errors at code level.



> In my previous life in the userspace I had to debug problems
> where the original error message got log-rotated away because of an
> onslaught of error messages that were a consequence of the original
> one, and not very helpful.
> 
> IMHO raising the issue of guest_error is a very sane thing to do,
> but it is a different problem. I think guest_error is about how and
> to whom the error is to be reported. IMHO report the error to the
> ones that are affected by it and to the ones that can do something
> about it (e.g. fix it) is a good rule of thumb. The latter may be
> different for hypervisor and for guest bugs.
> 
> In my understanding this is really about spamming the log problem.
> Of course one can try to solve/mitigate the problem at different
> levels. It could be declared
> 1) a problem to be solved in the logging library more or less
> transparently
> 2) a problem to be solved by the environment and it's admin (e.g.
> log aggregation, filtering, and rotation)
> 3) a problem that the client code of the logging library has to
> explicitly deal with
> 
> The once and rate_limited are 3).
> 
> To sum it up guest error or not and once or not are orthogonal
> problems in my view.
> 
> Regards,
> Halil

Right. But as long as we are changing this code, I'd like
to see guest errors reported in a way that makes it
easy to distinguish them from host errors.

> > 
> > Internally we can still have something similar to this
> > mechanism.
> > 
> > Another idea is to reset these guest error counters on guest reset.
> > Device reset too? I'm not 100% sure as guest can trigger device resets.
> > 
> 
> 
> [..]

  reply	other threads:[~2018-05-30 15:19 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-24  4:44 [Qemu-devel] [PATCH v4 0/2] error-report: introduce {error|warn}_report_once Peter Xu
2018-05-24  4:44 ` [Qemu-devel] [PATCH v4 1/2] qemu-error: " Peter Xu
2018-05-29  9:30   ` Cornelia Huck
2018-05-30  3:30     ` Peter Xu
2018-05-30 13:36       ` Cornelia Huck
2018-06-13  7:56         ` Markus Armbruster
2018-05-30  4:47   ` Michael S. Tsirkin
2018-05-30  6:39     ` Peter Xu
2018-05-30 13:53       ` Cornelia Huck
2018-06-13  7:59         ` Markus Armbruster
2018-05-30 15:15     ` Halil Pasic
2018-05-30 15:19       ` Michael S. Tsirkin [this message]
2018-05-30 15:30         ` Halil Pasic
2018-06-13  8:01   ` Markus Armbruster
2018-06-13  9:08     ` Peter Xu
2018-05-24  4:44 ` [Qemu-devel] [PATCH v4 2/2] intel-iommu: start to use error_report_once Peter Xu
2018-06-13  8:05   ` Markus Armbruster
2018-06-13  9:36     ` Auger Eric
2018-06-14 12:51       ` Markus Armbruster
2018-06-14 12:56         ` Peter Maydell
2018-08-15  5:58 ` [Qemu-devel] [PATCH v4 0/2] error-report: introduce {error|warn}_report_once Markus Armbruster
2018-08-15  6:10   ` Peter Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180530181734-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=armbru@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=jasowang@redhat.com \
    --cc=pasic@linux.ibm.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.