| Message ID | 1331036527-7651-3-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Mar 06, 2012 at 01:22:06PM +0100, Paolo Bonzini wrote: > It is not a problem if the destination does not have > VIRTIO_BALLOON_F_MUST_TELL_HOST. In that case, the guest > will simply do useless virtqueue traffic, but the destination > does not have a problem. > (In fact, it _is_ a problem if the destination has > VIRTIO_BALLOON_F_MUST_TELL_HOST but the source does not. > The feature bit should have been backwards! Luckily, > our implementation is free from this problem). > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Can't we just add a flag to control this feature? > --- > hw/virtio-balloon.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c > index 0ade8b0..d10df2e 100644 > --- a/hw/virtio-balloon.c > +++ b/hw/virtio-balloon.c > @@ -216,7 +216,7 @@ static int virtio_balloon_load(QEMUFile *f, void *opaque, int version_id) > if (version_id != 1) > return -EINVAL; > > - ret = virtio_load(&s->vdev, f, 0); > + ret = virtio_load(&s->vdev, f, VIRTIO_BALLOON_F_MUST_TELL_HOST); > if (ret) { > return ret; > } > -- > 1.7.7.6 >
Il 06/03/2012 15:55, Michael S. Tsirkin ha scritto: > > It is not a problem if the destination does not have > > VIRTIO_BALLOON_F_MUST_TELL_HOST. In that case, the guest > > will simply do useless virtqueue traffic, but the destination > > does not have a problem. > > (In fact, it _is_ a problem if the destination has > > VIRTIO_BALLOON_F_MUST_TELL_HOST but the source does not. > > The feature bit should have been backwards! Luckily, > > our implementation is free from this problem). > > > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > Can't we just add a flag to control this feature? I think the sane thing here would be to remove it from the spec. Paolo
On Tue, Mar 06, 2012 at 03:59:53PM +0100, Paolo Bonzini wrote: > Il 06/03/2012 15:55, Michael S. Tsirkin ha scritto: > > > It is not a problem if the destination does not have > > > VIRTIO_BALLOON_F_MUST_TELL_HOST. In that case, the guest > > > will simply do useless virtqueue traffic, but the destination > > > does not have a problem. > > > (In fact, it _is_ a problem if the destination has > > > VIRTIO_BALLOON_F_MUST_TELL_HOST but the source does not. > > > The feature bit should have been backwards! Luckily, > > > our implementation is free from this problem). > > > > > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > > > Can't we just add a flag to control this feature? > > I think the sane thing here would be to remove it from the spec. > > Paolo I guess we could but what does it buy us?
Il 06/03/2012 16:08, Michael S. Tsirkin ha scritto: > > > Can't we just add a flag to control this feature? > > > > I think the sane thing here would be to remove it from the spec. > > I guess we could but what does it buy us? Not having a broken spec. :) The pedantically correct thing to do would be to replace it with VIRTIO_BALLOON_F_DONT_TELL_HOST (so that it breaks in the 1->0 direction rather than 0->1). But indeed it doesn't buy enough, so it's not worth the effort of updating guests+QEMU+spec. Paolo
diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c index 0ade8b0..d10df2e 100644 --- a/hw/virtio-balloon.c +++ b/hw/virtio-balloon.c @@ -216,7 +216,7 @@ static int virtio_balloon_load(QEMUFile *f, void *opaque, int version_id) if (version_id != 1) return -EINVAL; - ret = virtio_load(&s->vdev, f, 0); + ret = virtio_load(&s->vdev, f, VIRTIO_BALLOON_F_MUST_TELL_HOST); if (ret) { return ret; }
It is not a problem if the destination does not have VIRTIO_BALLOON_F_MUST_TELL_HOST. In that case, the guest will simply do useless virtqueue traffic, but the destination does not have a problem. (In fact, it _is_ a problem if the destination has VIRTIO_BALLOON_F_MUST_TELL_HOST but the source does not. The feature bit should have been backwards! Luckily, our implementation is free from this problem). Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- hw/virtio-balloon.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)