From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:55612) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TimPP-0001EK-2Y for qemu-devel@nongnu.org; Wed, 12 Dec 2012 08:31:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TimPI-0007aZ-Fy for qemu-devel@nongnu.org; Wed, 12 Dec 2012 08:31:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55560) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TimPI-0007aV-7G for qemu-devel@nongnu.org; Wed, 12 Dec 2012 08:31:28 -0500 Date: Wed, 12 Dec 2012 11:29:30 -0200 From: Luiz Capitulino Message-ID: <20121212112930.7fa8effd@doriath.home> In-Reply-To: <24E144B8C0207547AD09C467A8259F75578AC74C@lisa.maurer-it.com> References: <1355168173-14571-1-git-send-email-lcapitulino@redhat.com> <24E144B8C0207547AD09C467A8259F7557870AEE@lisa.maurer-it.com> <20121211094513.29fa32bd@doriath.home> <24E144B8C0207547AD09C467A8259F755787AC78@lisa.maurer-it.com> <20121211105947.1b92f411@doriath.home> <24E144B8C0207547AD09C467A8259F755787E011@lisa.maurer-it.com> <20121211173808.GH30686@vm> <20121211162840.4b01701e@doriath.home> <24E144B8C0207547AD09C467A8259F755788040D@lisa.maurer-it.com> <20121211172355.3ee96cd4@doriath.home> <24E144B8C0207547AD09C467A8259F755788045F@lisa.maurer-it.com> <20121211174156.6e3e4d78@doriath.home> <24E144B8C0207547AD09C467A8259F75578AC74C@lisa.maurer-it.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/3] re-enable balloon stats List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Dietmar Maurer Cc: "qemu-devel@nongnu.org" , "aliguori@us.ibm.com" , mdroth , "agl@us.ibm.com" On Wed, 12 Dec 2012 13:17:15 +0000 Dietmar Maurer wrote: > > > > > Why don't we raise the error when we query the values? > > You already raise an error here, so that works out of the box: > > if (s->stats[i] == -1) { > error_setg(errp, > "timer hasn't been enabled or guest doesn't support '%s'", name); > > > > > > > > > Hmm, that's a good idea. > > > > > > > > Only small nit is that, today old stats will remain available to be > > > > queried even if you disable the timer. If we do what you suggest, > > > > old stats won't be available if the module is unloaded (well, I > > > > *guess* the guest will unset the feature bit on module removal). > > > > > > What is the problem with that? It would be no problem to simulate the > > > old behavior, but why do you want that? > > > > I actually just thought that it's a nice to have. > > The following patch seems to work for me: Yeah, that's what I'll do and the doc has to be updated too. > > Index: new/hw/virtio-balloon.c > =================================================================== > --- new.orig/hw/virtio-balloon.c 2012-12-12 14:05:56.000000000 +0100 > +++ new/hw/virtio-balloon.c 2012-12-12 14:07:43.000000000 +0100 > @@ -111,6 +111,10 @@ > { > VirtIOBalloon *s = opaque; > > + if (!balloon_stats_supported(s) || !runstate_is_running()) { > + return; > + } > + > virtqueue_push(s->svq, &s->stats_vq_elem, s->stats_vq_offset); > virtio_notify(&s->vdev, s->svq); > } > @@ -164,11 +168,6 @@ > VirtIOBalloon *s = opaque; > int64_t value; > > - if (!balloon_stats_supported(s)) { > - error_setg(errp, "guest doesn\'t support balloon stats"); > - return; > - } > - > visit_type_int(v, &value, name, errp); > if (error_is_set(errp)) { > return; > > >