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<amit.s...@redhat.com> > >>--- > >> 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. 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. 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. -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|