From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:38906) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SUjVY-0005Rw-7B for qemu-devel@nongnu.org; Wed, 16 May 2012 15:03:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SUjVW-0002qt-2P for qemu-devel@nongnu.org; Wed, 16 May 2012 15:03:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51443) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SUjVV-0002qb-R1 for qemu-devel@nongnu.org; Wed, 16 May 2012 15:03:34 -0400 Date: Wed, 16 May 2012 16:03:46 -0300 From: Luiz Capitulino Message-ID: <20120516160346.1e1f2cb9@doriath.home> In-Reply-To: <4FB3F8DA.4030809@codemonkey.ws> References: <1337163047-6159-1-git-send-email-berrange@redhat.com> <20120516154245.0d9b0cec@doriath.home> <4FB3F8DA.4030809@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] Add event notification for guest balloon changes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Amit Shah , qemu-devel@nongnu.org, Markus Armbruster On Wed, 16 May 2012 13:58:34 -0500 Anthony Liguori wrote: > On 05/16/2012 01:42 PM, Luiz Capitulino wrote: > > On Wed, 16 May 2012 11:10:47 +0100 > > "Daniel P. Berrange" wrote: > > > >> From: "Daniel P. Berrange" > >> > >> After setting a balloon target value, applications have to > >> continually poll 'query-balloon' to determine whether the > >> guest has reacted to this request. The virtio-balloon backend > >> knows exactly when the guest has reacted though, and thus it > >> is possible to emit a JSON event to tell the mgmt application > >> whenever the guest balloon changes. > >> > >> This introduces a new 'qemu_balloon_change()' API which is > >> to be called by balloon driver backends, whenever they have > >> a change in balloon value. This takes the 'actual' balloon > >> value, as would be found in the BalloonInfo struct. > >> > >> The qemu_balloon_change API emits a JSON monitor event which > >> looks like: > >> > >> {"timestamp": {"seconds": 1337162462, "microseconds": 814521}, > >> "event": "BALLOON_CHANGE", "data": {"actual": 944766976}} > > > > It's missing an entry in QMP/qmp-events.txt and I have a comment below, > > but in general looks good. > > > > Amit, would be good to get your ack. > > I think it would be safer to limit this event to (1) only firing once target has > been reached (2) firing if target is deviated from without a corresponding > change in target. > > Otherwise, a guest could just flood libvirt with events. This would queue > memory in QEMU indefinitely as the events got queued up to potentially serving > as a DoS against other guests. Yeah, I've said something along these lines below: > >> @@ -146,8 +146,13 @@ static void virtio_balloon_set_config(VirtIODevice *vdev, > >> { > >> VirtIOBalloon *dev = to_virtio_balloon(vdev); > >> struct virtio_balloon_config config; > >> + uint32_t oldactual = dev->actual; > >> memcpy(&config, config_data, 8); > >> dev->actual = le32_to_cpu(config.actual); > >> + if (dev->actual != oldactual) { > >> + qemu_balloon_change(ram_size - > >> + (dev->actual<< VIRTIO_BALLOON_PFN_SHIFT)); > >> + } > > > > This can cause several events to be emitted until the memory is adjusted > > to the value asked by the user. I'm undecided if this is a feature, but > > if I were a client issuing the balloon command I'd expect to get the event > > only when the memory is fully adjusted to the value I asked. > > > > Not sure if this possible to implement though, or if we really want it.