[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/7] balloon: Separate out stat and balloon hand
From: |
Amit Shah |
Subject: |
Re: [Qemu-devel] [PATCH 5/7] balloon: Separate out stat and balloon handling |
Date: |
Sat, 23 Jul 2011 08:40:03 +0530 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On (Fri) 22 Jul 2011 [16:45:55], Markus Armbruster wrote:
> Amit Shah <address@hidden> 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 <address@hidden>
> > Signed-off-by: Amit Shah <address@hidden>
>
> 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
- [Qemu-devel] [PATCH 0/7] balloon: cleanups, fix segfault, Amit Shah, 2011/07/20
- [Qemu-devel] [PATCH 1/7] balloon: Make functions, local vars static, Amit Shah, 2011/07/20
- [Qemu-devel] [PATCH 2/7] balloon: Add braces around if statements, Amit Shah, 2011/07/20
- [Qemu-devel] [PATCH 3/7] balloon: Simplify code flow, Amit Shah, 2011/07/20
- [Qemu-devel] [PATCH 4/7] virtio-balloon: Separate status handling into separate function, Amit Shah, 2011/07/20
- [Qemu-devel] [PATCH 5/7] balloon: Separate out stat and balloon handling, Amit Shah, 2011/07/20
- [Qemu-devel] [PATCH 6/7] balloon: Fix header comment; add Copyright, Amit Shah, 2011/07/20
- [Qemu-devel] [PATCH 7/7] virtio-balloon: Fix header comment; add Copyright, Amit Shah, 2011/07/20
- Re: [Qemu-devel] [PATCH 0/7] balloon: cleanups, fix segfault, Markus Armbruster, 2011/07/25