qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 1/3] softmmu/physmem: fallback to opening guest RAM file a


From: David Hildenbrand
Subject: Re: [PATCH v1 1/3] softmmu/physmem: fallback to opening guest RAM file as readonly in a MAP_PRIVATE mapping
Date: Thu, 17 Aug 2023 17:15:52 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0

On 17.08.23 17:13, Stefan Hajnoczi wrote:
On Thu, 17 Aug 2023 at 05:08, David Hildenbrand <david@redhat.com> wrote:

@Stefan, see below on a R/O NVDIMM question.

We're discussing how to get MAPR_PRIVATE R/W mapping of a
memory-backend-file running when using R/O files.


This seems a good idea. I am good with the solution you proposed
here as well.

I was just going to get started working on that, when I realized
something important:


"@readonly: if true, the backing file is opened read-only; if false,
   it is opened read-write.  (default: false)"

"@share: if false, the memory is private to QEMU; if true, it is
   shared (default: false)"

So readonly is *all* about the file access mode already ... the mmap()
parameters are just a side-effect of that. Adding a new
"file-access-mode" or similar would be wrong.


Here are the combinations we have right now:

-object memory-backend-file,share=on,readonly=on

   -> Existing behavior: Open readonly, mmap readonly shared
   -> Makes sense, mmap'ing readwrite would fail

-object memory-backend-file,share=on,readonly=off

   -> Existing behavior: Open readwrite, mmap readwrite shared
   -> Mostly makes sense: why open a shared file R/W and not mmap it
      R/W?

-object memory-backend-file,share=off,readonly=off
   -> Existing behavior: Open readwrite, mmap readwrite private
   -> Mostly makes sense: why open a file R/W and not map it R/W (even if
      private)?

-object memory-backend-file,share=off,readonly=on
   -> Existing behavior: Open readonly, mmap readonly private
   -> That's the problematic one


So for your use case (VM templating using a readonly file), you
would actually want to use:

-object memory-backend-file,share=off,readonly=on

BUT, have the mmap be writable (instead of currently readonly).

Assuming we would change the current behavior, what if someone would
specify:

-object memory-backend-file,readonly=on

(because the default is share=off ...) and using it for a R/O NVDIMM,
where we expect any write access to fail.


But let's look at the commit that added the "readonly" parameter:

commit 86635aa4e9d627d5142b81c57a33dd1f36627d07
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date:   Mon Jan 4 17:13:19 2021 +0000

      hostmem-file: add readonly=on|off option

      Let -object memory-backend-file work on read-only files when the
      readonly=on option is given. This can be used to share the contents of a
      file between multiple guests while preventing them from consuming
      Copy-on-Write memory if guests dirty the pages, for example.

That was part of

https://lore.kernel.org/all/20210104171320.575838-3-stefanha@redhat.com/T/#m712f995e6dcfdde433958bae5095b145dd0ee640

  From what I understand, for NVDIMMs we always use
"-object memory-backend-file,share=on", even when we want a
readonly NVDIMM.


So we have two options:

1) Change the current behavior of -object 
memory-backend-file,share=off,readonly=on:

-> Open the file r/o but mmap it writable

Commit 86635aa4e9d627d5142b81c57a33dd1f36627d07 mentions that we don't
want guests to be able to dirty pages on the host. The change you're
proposing would not protect against guests that dirty the memory.

The guest could write memory but not modify the file. Only with "share=off,readonly=on" of course, not with "share=on,readonly=on".


I don't know how important that requirement was (that commit was a
request from Kata Containers).

Let me take a look if Kata passes "share=on,readonly=on" or "share=off,readonly=off".

Thanks!

--
Cheers,

David / dhildenb




reply via email to

[Prev in Thread] Current Thread [Next in Thread]