[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [PATCH v7 4/5] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT
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
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]