virtualization.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: fes@google.com, aarcange@redhat.com, riel@redhat.com,
	yvugenfi@redhat.com, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, mikew@google.com,
	yinghan@google.com, virtualization@lists.linux-foundation.org
Subject: Re: [PATCH] virtio-balloon spec: provide a version of the "silent deflate" feature that works
Date: Fri, 07 Sep 2012 13:20:57 +0200	[thread overview]
Message-ID: <5049D899.60705@redhat.com> (raw)
In-Reply-To: <20120907105335.GB17211@redhat.com>

Il 07/09/2012 12:53, Michael S. Tsirkin ha scritto:
> Let us start with what is broken currently. Looking at
> it very closely, I think the answer is nothing.
> Even migration in qemu is not broken as you claimed initially.

Correct, migration would be broken as soon as QEMU starts using
MUST_TELL_HOST.  I'm trying to think ahead, since we have many ideas
floating around on the implementation of ballooning.

If you implement the mlock/munlock trick, you must start using
MUST_TELL_HOST in QEMU to advertise it to guests, and migration breaks.

> 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).  Linux and
BSD drivers do negotiate MUST_TELL_HOST, and are not buggy.

> This is a wrong thing to do.
> You also use two feature bits for a single simple thing,
> this is inelegant.

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.

So in the end you do use two feature bits for two different things.
Plus, both feature bits are "positive" and I'm happy.

> Last, let us consider how existing feature can be used
> in the hypervisor. If driver did not ack
> MUST_TELL_HOST, it is *not* buggy but it means we can not
> do munlock. This applies to current windows drivers.
> If driver *did* ack MUST_TELL_HOST, we can munlock
> and mlock back on leak.
> Seems useful, driver support is already there,
> so removing the MUST_TELL_HOST bit seems like a bad idea.

Indeed, repurposing MUST_TELL_HOST to WILL_TELL_HOST is better than
killing it.

Paolo

  reply	other threads:[~2012-09-07 11:20 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-06  7:46 [PATCH] virtio-balloon spec: provide a version of the "silent deflate" feature that works Paolo Bonzini
2012-09-06  8:47 ` Michael S. Tsirkin
2012-09-06  9:24   ` Paolo Bonzini
2012-09-06  9:44     ` Michael S. Tsirkin
2012-09-06  9:57       ` Paolo Bonzini
2012-09-06 10:53         ` Michael S. Tsirkin
2012-09-06 12:13           ` Paolo Bonzini
2012-09-06 12:51             ` Michael S. Tsirkin
2012-09-06 13:12               ` Paolo Bonzini
2012-09-06 23:45             ` Rusty Russell
2012-09-07  5:42               ` Michael S. Tsirkin
2012-09-07  6:39                 ` Rusty Russell
2012-09-07  9:27                   ` Paolo Bonzini
2012-09-07 10:53                     ` Michael S. Tsirkin
2012-09-07 11:20                       ` Paolo Bonzini [this message]
2012-09-07 12:17                         ` Michael S. Tsirkin
2012-09-07 12:22                           ` Paolo Bonzini
2012-09-07 12:44                             ` Michael S. Tsirkin
2012-09-07 14:04                               ` Paolo Bonzini
2012-09-07 14:25                                 ` Michael S. Tsirkin
2012-09-07 14:44                                   ` Paolo Bonzini
2012-09-08 22:22                                     ` Michael S. Tsirkin
2012-09-10  5:50                                       ` Paolo Bonzini
     [not found]                                       ` <504D7F95.9070700@redhat.com>
2012-09-10  6:03                                         ` Michael S. Tsirkin
2012-09-10  6:38                                           ` Paolo Bonzini
2012-09-10  6:47                                             ` Michael S. Tsirkin
2012-09-10  6:55                                               ` Paolo Bonzini
2012-09-07 10:43                   ` Michael S. Tsirkin
2012-09-08  5:06                     ` Rusty Russell
2012-09-08 10:32                       ` Paolo Bonzini
2012-09-08 22:37                       ` Michael S. Tsirkin
2012-09-10  1:43                         ` Rusty Russell
2012-09-10  5:51                           ` Paolo Bonzini
2012-09-10  5:51                           ` Michael S. Tsirkin
2012-09-12  6:24                             ` Rusty Russell
2012-09-06 23:41 ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5049D899.60705@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=aarcange@redhat.com \
    --cc=fes@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mikew@google.com \
    --cc=mst@redhat.com \
    --cc=riel@redhat.com \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=yinghan@google.com \
    --cc=yvugenfi@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).