Comment 30 for bug 1626972

Revision history for this message
elmarco (marcandre-lureau) wrote : Re: [Qemu-devel] [Bug 1626972] Re: [PATCH] util: secure memfd_create fallback mechanism

Hi

On Fri, Oct 21, 2016 at 6:03 AM Rafael David Tinoco <
<email address hidden>> wrote:

> Judging by the outputs above, looks like vhost_dev_log_is_shared is
> returning false, making (2) - vhost_dev_start - to use a different log
> allocation (malloc) than the one that was tested for allowing migrations at
> (1) - vhost_dev_init.
>
>
correct

> Question: Why to check for "memfd" when its not sure - yet - if a shared
> descriptor and memory pointer is going to be needed for the migration to
> happen ? Do you want me to

It's done early enough to disable migration.

> change that ? If memfd fails, but, the guest in question is using regular
> "malloc" for vhost log, we are marking it unable to live migrate by
> mistake. I could check for vhost_requires_shm_log pointer during
> vhost_dev_init (coming from tap).
>
>
Right, it should be done only if vhost_dev_log_is_shared is true. Patch
welcome

> Also, if possible, I would like comments about a draft:
>
> https://pastebin.canonical.com/168579/
> (please disregard printfs and minor problems)
>
> OBS: I'm basically removing fallback mechanism from memfd, creating a
> generic qemu_mmap_XXX implementation, adding a vhostlog parameter in tap
> cmdline AND changing the decision on what to use: if vhostlog is present in
> cmdline, qemu_mmap_XXX on vhostlog is used. If it is a directory, a random
> file is created inside it. If it is a file, the file is used. If no
> vhostlog is given (default while libvirt isn't changed), it tries first to
> use memfd (all newer kernels), and, if not possible, it tries to fallback
> using the qemu_mmap mechanism on "tmp" directory creating random files.
>

Sounds reasonable, but I am not sure so many fallbacks are necessary. I
would just have an optional filename.

>
> PS: Remember that this is because selinux/apparmor labelling on tmp files
> (and because file descriptors can be passed away, like we discussed before).
>
> If that is okay I'll provide a patch asap. Let me know if you prefer
> something else.
>

Ok, I hope other comments on the idea, and I'll review your patch once on
the ML.

Thanks
--
Marc-André Lureau