[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20120907124432.GB17397@redhat.com>
Date: Fri, 7 Sep 2012 15:44:32 +0300
From: "Michael S. Tsirkin" <mst@...hat.com>
To: Paolo Bonzini <pbonzini@...hat.com>
Cc: Rusty Russell <rusty@...tcorp.com.au>, fes@...gle.com,
aarcange@...hat.com, riel@...hat.com, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org, mikew@...gle.com, yinghan@...gle.com,
virtualization@...ts.linux-foundation.org, yvugenfi@...hat.com,
vrozenfe@...hat.com
Subject: Re: [PATCH] virtio-balloon spec: provide a version of the "silent
deflate" feature that works
On Fri, Sep 07, 2012 at 02:22:47PM +0200, Paolo Bonzini wrote:
> Il 07/09/2012 14:17, Michael S. Tsirkin ha scritto:
> >>> Next, consider the interface proposed here. You defacto declare
> >>> all existing drivers buggy.
> >>
> >> No, only Windows (and it is buggy, it calls tell_host last).
> >
> > It is not buggy. It does not ack MUST_TELL_HOST. So it is free to tell
> > host at any point, it behaves exactly
> > to spec. You can not retroactively declare drivers buggy like that.
>
> Well, according to your reading of the spec.
>
> In the spec I'm reading "Host must be told before pages from the balloon
> are used". Doesn't say anything about the guest.
No? How is host told then? By divine force?
> Now, indeed a very free interpretation is "Guest will tell host before
> pages from the balloon are used". It turns out that it's exactly what
> guests have been doing, hence that's exactly what I'm proposing now:
> rename the feature to VIRTIO_BALLOON_F_WILL_TELL_HOST.
Rename is fine.
> >> True, but the choice is:
> >>
> >> 1) add a once-only hack to QEMU that fixes migration of
> >> VIRTIO_BALLOON_F_MUST_TELL_HOST;
> >>
> >> 2) always advertise VIRTIO_BALLOON_F_MUST_TELL_HOST. If you do this,
> >> guests cannot use anymore silent deflate, which is a regression.
> >>
> >> 3) use two bits. One tells the device that the driver supports chatty
> >> deflate; one tells the driver that the device supports silent deflate.
> >
> > The right thing to do is either
> > 4. realize we can not address all user errors, so no real issue
> > 5. address this class of user errors by migrating host features
> >
> >> So in the end you do use two feature bits for two different things.
> >> Plus, both feature bits are "positive" and I'm happy.
> >
> > I am not happy.
> > We lose compatibility with all existing drivers
>
> How so?
>
> > so it will take years until the feature is actually
> > useful.
>
> No, we don't! Windows guests will just not be able to use munlock/mlock
> until the driver is fixed. Which will probably happen before someone
> writes the munlock/mlock code.
If the only change is rename then ofcourse things keep working.
I don't care about the name it is up to Rusty.
> > Is this just a matter of naming? Same functionality:
> > driver that acks this bit will tell host first,
> > driver that does not will not?
> >
> > If yes that is fine.
>
> Yes, that part we agree on I think. We disagree that (after the rename)
> QEMU should start always proposing VIRTIO_BALLOON_F_WILL_TELL_HOST.
Not always. It must be off if compatibility with old qemu is disabled.
> _Plus_ adding the new feature bit, which is needed to actually tell the
This part I do not get. What is silent deflate, why is it useful
and what it has to do with what we are discussing here?
> driver that the host supports the silent deflate.
> Spec patch on the way.
>
> Paolo
Hang on.
Can we please talk about motivation? These patches which come
without motivation are very hard to review.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists