Re: [PATCH] virtio-balloon spec: rework VIRTIO_BALLOON_F_MUST_TELL_HOST feature, support silent deflation

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, May 28, 2013 at 10:38:35AM +0200, Paolo Bonzini wrote:
> Il 27/05/2013 19:02, Michael S. Tsirkin ha scritto:
> >>> So if we don't want to require all guests to tell host
> >>> first, all we need to do is admit it's not a bug.
> >>
> >> I think we want the possibility for the host to require that.
> > 
> > But why? TELL_HOST makes some optimizations possible, but if
> > guest won't cooperate, balloon is useless anyway.
> 
> If the guest won't tell host and still propose the feature,

Ack feature but don't tell host? That would be a clear guest bug.
AFAIK that's not what windows drivers do.
Am I wrong?

> then we can
> crash it.  So we need to know what the guest is going to do, in order to
> enable/disable the optimization.
> 
> > If guest cooperates we don't have to require anything,
> > just go with what guest tells us it will do.
> 
> Yes.
> 
> >>> Please see
> >>> 	[PATCH] virtio-spec: balloon: MUST_TELL_HOST is optional
> >>> that does exactly this.
> >>
> >> That patch mandates a change in guest behavior that is not compatible
> >> with the existing Windows driver.  Mine doesn't.
> >>
> >> Paolo
> > 
> > Hmm I don't see it.
> > In fact the goal was to document the Windows driver behaviour
> > as correct.
> > Can you explain the incompatibility please?
> 
> Whenever "If the X feature is (not) negotiated" is used in the spec, it
> means "in general you should be ready to implement both behaviors", or
> perhaps the guest should fail to initialize if the feature is not available.

"you" meaning host. Yes.
But here guest can tell host first *always if it wants to - it will
just be a bit slower when reusing pages from balloon.
If it acked the feature, it *must* tell host first.

> 
> Here it is the other way round.  The existing guest is not checking the
> outcome of the negotiation, so the host must check whether negotiation
> happened and possibly fail the initialization of the device.  It is
> sufficiently different from any other case that I don't think a one-word
> change is enough.
> 
> The way I read it yesterday I didn't see any change from the current
> specification, so the problem of having a "negative feature" remains.

This is standard behaviour:

- guest can ignore any feature that it does not ack
- host must implement both behaviours for guests that
  do and for guests that do not ack features

This is exactly what I'm proposing for TELL_HOST.

> Now rereading it, it may be correct, but it is not clear enough.
> 
> Perhaps my patch is even too verbose, but it doesn't leave anything open
> for interpretation.
> 
> Paolo

I'm fine with adding more clarifications but I don't yet see why
do we need a new bit.

-- 
MST