On 06.07.23 15:20, Juan Quintela wrote:
David Hildenbrand <da...@redhat.com> wrote:
On 06.07.23 10:10, Juan Quintela wrote:
David Hildenbrand <da...@redhat.com> wrote:
ram_block_discard_range() cannot possibly do the right thing in
MAP_PRIVATE file mappings in the general case.
To achieve the documented semantics, we also have to punch a hole into
the file, possibly messing with other MAP_PRIVATE/MAP_SHARED mappings
of such a file.
For example, using VM templating -- see commit b17fbbe55cba ("migration:
allow private destination ram with x-ignore-shared") -- in combination with
any mechanism that relies on discarding of RAM is problematic. This
includes:
* Postcopy live migration
* virtio-balloon inflation/deflation or free-page-reporting
* virtio-mem
So at least warn that there is something possibly dangerous is going on
when using ram_block_discard_range() in these cases.
Acked-by: Peter Xu <pet...@redhat.com>
Tested-by: Mario Casquero <mcasq...@redhat.com>
Signed-off-by: David Hildenbrand <da...@redhat.com>
Reviewed-by: Juan Quintela <quint...@redhat.com>
(at least we give a warning)
But I wonder if we can do better and test that:
* Postcopy live migration
We can check if we are on postcopy, or put a marker so we know
that
postcopy can have problems when started.
* virtio-balloon inflation/deflation or free-page-reporting
We can check if we have ever used virtio-balloon.
* virtio-mem
We can check if we have used virtio-men
I am just wondering if that is even possible?
Now we warn when any of these features actually tries discarding RAM
(calling ram_block_discard_range()).
As these features trigger discarding of RAM, once we reach this point
we know that they are getting used. (in comparison to default libvirt
attaching a virtio-balloon device without anybody ever using it)
The alternative would be checking the RAM for compatibility when
configuring each features. I decided to warn at a central place for
now, which covers any users.
I think this is the right thing to do.
Patient: It hurts when I do this.
Doctor: Don't do that.
O:-)
:)
Now more seriously, at this point we are very late to do anything
sensible. I think that the normal thing when we are configuring
incompatible things just flag it.
We are following that approach with migration for some time now, and
everybody is happier.
For the time being I'll move forward with this patch.
I agree that warning early is nicer (but warning for example for
virtio-balloon early doesn't make too much sense: libvirt adds it
blindly to each VM just to query guest statistics and never inflate the
balloon).
In any case we'll want to warn here as well, because we know that new
callers will easily ignore that limitation / checks.
--
Cheers,
David / dhildenb
