On 06/01/2018 01:42 AM, Michael S. Tsirkin wrote:
On Thu, May 31, 2018 at 10:27:00AM +0800, Wei Wang wrote:
On 05/30/2018 08:47 PM, Michael S. Tsirkin wrote:
On Wed, May 30, 2018 at 05:12:09PM +0800, Wei Wang wrote:
On 05/29/2018 11:24 PM, Michael S. Tsirkin wrote:
On Tue, Apr 24, 2018 at 02:13:47PM +0800, Wei Wang wrote:
+/*
+ * Balloon will report pages which were free at the time of this call. As the
+ * reporting happens asynchronously, dirty bit logging must be enabled before
+ * this call is made.
+ */
+void balloon_free_page_start(void)
+{
+ balloon_free_page_start_fn(balloon_opaque);
+}
Please create notifier support, not a single global.
OK. The start is called at the end of bitmap_sync, and the stop is called at
the beginning of bitmap_sync. In this case, we will need to add two
migration states, MIGRATION_STATUS_BEFORE_BITMAP_SYNC and
MIGRATION_STATUS_AFTER_BITMAP_SYNC, right?
If that's the way you do it, you need to ask migration guys, not me.
Yeah, I know.. thanks for the virtio part.
+
+static bool virtio_balloon_free_page_support(void *opaque)
+{
+ VirtIOBalloon *s = opaque;
+ VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+ return virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT);
or if poison is negotiated.
Will make it
return virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT) &&
!virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON)
I mean the reverse:
virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT) ||
virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON)
If poison has been negotiated you must migrate the
guest supplied value even if you don't use it for hints.
Just a little confused with the logic. Writing it that way means that we are
taking this possibility "virtio_vdev_has_feature(vdev,
VIRTIO_BALLOON_F_FREE_PAGE_HINT)=fasle, virtio_vdev_has_feature(vdev,
VIRTIO_BALLOON_F_PAGE_POISON)=true" into account, and let the support
function return true when F_FREE_PAGE_HINT isn't supported.
All I am saying is that in this configuration, you must migrate
the poison value programmed by guest even if you do not
yet use it without VIRTIO_BALLOON_F_FREE_PAGE_HINT.
Right now you have
a section:
+ .needed = virtio_balloon_free_page_support,
which includes the poison value.
So if guest migrates after writing the poison value,
it's lost. Not nice.
If guest doesn't support F_FREE_PAGE_HINT, it doesn't support the free page
reporting (even the free page vq). I'm not sure why we tell the migration
thread that the free page reporting feature is supported via this support
function. If the support function simply returns false when F_FREE_PAGE_HINT
isn't negotiated, the legacy migration already migrates the poisoned pages
(not skipped, but may be compressed).
I think it would be better to simply use the original "return
virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)" here.
So maybe you should put the poison value in a separate section then.
Yes, that looks good to me, thanks.
Best,
Wei
