[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED er
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message |
Date: |
Fri, 27 Aug 2010 11:15:07 -0300 |
On Fri, 27 Aug 2010 15:59:21 +0200
Markus Armbruster <address@hidden> wrote:
> "Daniel P. Berrange" <address@hidden> writes:
>
> > On Fri, Aug 27, 2010 at 07:39:37AM -0500, Anthony Liguori wrote:
> >> On 08/27/2010 04:29 AM, Daniel P. Berrange wrote:
> >> >On Fri, Aug 27, 2010 at 10:57:10AM +0530, Amit Shah wrote:
> >> >
> >> >>This error message denotes some command was not successful in completing
> >> >>as the guest was unresponsive.
> >> >>
> >> >>Use it in the virtio-balloon code when showing older, cached data.
> >> >>
> >> >>Signed-off-by: Amit Shah<address@hidden>
> >> >>---
> >> >> hw/virtio-balloon.c | 1 +
> >> >> qerror.c | 4 ++++
> >> >> qerror.h | 3 +++
> >> >> 3 files changed, 8 insertions(+), 0 deletions(-)
> >> >>
> >> >>diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
> >> >>index d6c66cf..309c343 100644
> >> >>--- a/hw/virtio-balloon.c
> >> >>+++ b/hw/virtio-balloon.c
> >> >>@@ -140,6 +140,7 @@ static void complete_stats_request(VirtIOBalloon *vb)
> >> >>
> >> >> static void show_old_stats(void *opaque)
> >> >> {
> >> >>+ qerror_report(QERR_MACHINE_STOPPED);
> >> >> complete_stats_request(opaque);
> >> >> }
> >> >>
> >> >
> >> >NACK. It has always been allowed& valid to call query-balloon
> >> >to get the current balloon level. We must not throw an error
> >> >just because the recently added mem stats can't be refreshed.
> >>
> >> I think that's a fair comment but why even bother fixing the command.
> >> Let's introduce a new command that just gets a single piece of
> >> information instead of having a command return lots of information.
> >
> > The existing query-balloon command that has been around for years &
> > is used by all current apps has a significant regression since we added
> > the memstats code to it: a guest can now trivially inflict a DOS on the
> > mgmt app if it crashes or is malicious. IMHO we need to fix that regression
> > for 0.13 so that existing apps don't suffer[1]. Adding a timeout to silently
> > skip the stats refresh if the guest doesn't respond, but without raising
> > an error seems the best tradeoff we can do here.
>
> I agree.
>
> Adding a roundtrip through the guest to an existing command was a
> mistake.
I wondered if we could drop it for now to make it right in 0.14, but I
believe it's already part of the user monitor for some time and libvirt
uses the stats, right?
I think we need testing/unstable namespace in QMP, where commands can be
tested for while so that we reduce the risk of nasty surprises like this one.
>
> > Beyond fixing that regression, I agree that this command is terminally
> > flawed & we need to deprecate it & provide better specified new
> > replacement(s). This seems like 0.14 work to me though.
>
> Yup.
>
> > Regards,
> > Daniel
> >
> > [1] I know that they could already suffer if there was a bug in qemu
> > that prevented it responding, even if the guest was not being
> > malicious/crashed.
>
- [Qemu-devel] [PATCH v3 0/3] virtio-balloon: Don't wait indefinitely for guest response, Amit Shah, 2010/08/27
- [Qemu-devel] [PATCH v3 1/3] balloon: Don't try fetching info if guest is unresponsive, Amit Shah, 2010/08/27
- [Qemu-devel] [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message, Amit Shah, 2010/08/27
- [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message, Daniel P. Berrange, 2010/08/27
- [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message, Anthony Liguori, 2010/08/27
- [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message, Daniel P. Berrange, 2010/08/27
- Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message, Markus Armbruster, 2010/08/27
- Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message,
Luiz Capitulino <=
- Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message, Anthony Liguori, 2010/08/27
- Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message, Daniel P. Berrange, 2010/08/27
- Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message, Anthony Liguori, 2010/08/27
- Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message, Luiz Capitulino, 2010/08/27
- Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message, Anthony Liguori, 2010/08/27
- Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message, Luiz Capitulino, 2010/08/27
- Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message, Anthony Liguori, 2010/08/27
- Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message, Luiz Capitulino, 2010/08/27
- Should QMP be RPC to internal C interfaces? (was: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message), Markus Armbruster, 2010/08/30
- [Qemu-devel] Re: Should QMP be RPC to internal C interfaces?, Anthony Liguori, 2010/08/30