From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:57119) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QkSbT-0005CW-BE for qemu-devel@nongnu.org; Fri, 22 Jul 2011 23:10:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QkSbS-00032f-8W for qemu-devel@nongnu.org; Fri, 22 Jul 2011 23:10:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11988) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QkSbS-00032L-14 for qemu-devel@nongnu.org; Fri, 22 Jul 2011 23:10:10 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p6N3A8tr021012 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 22 Jul 2011 23:10:08 -0400 Date: Sat, 23 Jul 2011 08:40:03 +0530 From: Amit Shah Message-ID: <20110723031003.GA14493@amit-x200.redhat.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 5/7] balloon: Separate out stat and balloon handling List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu list On (Fri) 22 Jul 2011 [16:45:55], Markus Armbruster wrote: > Amit Shah writes: > > > Passing on '0' as ballooning target to indicate retrieval of stats is > > bad API. It also makes 'balloon 0' in the monitor cause a segfault. > > Have two different functions handle the different functionality instead. > > > > Reported-by: Mike Cao > > Signed-off-by: Amit Shah > > Can you explain the fault? It's not obvious to me... There's a bt at: https://bugzilla.redhat.com/show_bug.cgi?id=694378 The callback is populated when called via 'info balloon', where some detail is printed on the monitor after the guest responds with the current balloon info. On the other hand, 'balloon X' just updates the balloon size; with no information to be printed. When 'balloon 0' is issued, virtio_balloon_to_target() thinks it is the 'info balloon' command, gets info from the guest, and then tries to call the monitor callback to print the info it got... and segfaults. > > --- a/hw/virtio-balloon.c > > +++ b/hw/virtio-balloon.c > > @@ -227,8 +227,7 @@ static void virtio_balloon_stat(void *opaque, MonitorCompletion cb, > > complete_stats_request(dev); > > } > > > > -static void virtio_balloon_to_target(void *opaque, ram_addr_t target, > > - MonitorCompletion cb, void *cb_data) > > +static void virtio_balloon_to_target(void *opaque, ram_addr_t target) > > { > > VirtIOBalloon *dev = opaque; > > > > @@ -238,8 +237,6 @@ static void virtio_balloon_to_target(void *opaque, ram_addr_t target, > > if (target) { > > dev->num_pages = (ram_size - target) >> VIRTIO_BALLOON_PFN_SHIFT; > > virtio_notify_config(&dev->vdev); > > - } else { > > - virtio_balloon_stat(opaque, cb, cb_data); > > } > > } > > > > Special case: do nothing when target == 0. Is that necessary/ Why make a round trip to the guest when we're doing nothing? Amit