Re: [Qemu-devel] [PATCH] vhost: secure vhost shared log files using argv paremeter

[Rafael David Tinoco]

Hello,

> On Oct 22, 2016, at 05:18, Marc-André Lureau <address@hidden> wrote:
> 
> Hi
> 
> On Sat, Oct 22, 2016 at 10:01 AM Rafael David Tinoco <address@hidden> wrote:
> Commit 31190ed7 added a migration blocker in vhost_dev_init() to
> check if memfd would succeed. It is better if this blocker first
> checks if vhost backend requires shared log. This will avoid a
> situation where a blocker is added inappropriately (e.g. shared
> log allocation fails when vhost backend doesn't support it).
> 
> Could you make this a seperate patch?

Just did, in another e-mail, cc'ing you.

> Argv examples:
> 
>     -netdev tap,id=net0,vhost=on
>     -netdev tap,id=net0,vhost=on,vhostlog=/tmp/guest.log
>     -netdev tap,id=net0,vhost=on,vhostlog=/tmp
> 
> Could it be only a filename? This would simplify testing.

It could. Should I keep the /tmp/<random> logic if no vhostlog arg is present ? 
Or you think it should fail if no arg is given ? I'm afraid of backward 
compatibility when back-porting this to older qemu versions on stable releases 
(like my case: I'll backport this to ~3 different versions). 

> For vhost backends supporting shared logs, if vhostlog is non-existent,
> or a directory, random files are going to be created in the specified
> directory (or, for non-existent, in tmpdir). If vhostlog is specified,
> the filepath is always used when allocating vhost log files.
> 
> 
> Regarding testing, you add utility code mmap-file, could you make this a 
> seperate commit, with unit tests?
> 

Sure, I'll work on it.

> thanks

Thank u!

-Rafael Tinoco