Re: [PATCH] virtio-balloon spec: provide a version of the "silentdeflate" feature that works

[Michael S. Tsirkin]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/