Re: [RFC PATCH 00/19] Guest introspection

From: Paolo Bonzini <pbonzini@redhat.com>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: alazar@bitdefender.com, "Jan Kiszka" <jan.kiszka@siemens.com>,
	"Mihai Dontu" <mdontu@bitdefender.com>,
	"Radim Krčmář" <rkrcmar@redhat.com>,
	kvm@vger.kernel.org
Subject: Re: [RFC PATCH 00/19] Guest introspection
Date: Wed, 21 Jun 2017 09:25:47 -0400 (EDT)	[thread overview]
Message-ID: <645181790.10962776.1498051547977.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <20170621110407.GE16183@stefanha-x1.localdomain>

On 21/06/2017 13:04, Stefan Hajnoczi wrote:> On Tue, Jun 20, 2017 at 05:58:41PM +0300, alazar@bitdefender.com wrote:
>> Moving the vsock to userland will change this:
>>
>>                                      -----------------------------
>>                  /----- /dev/kvm -->| new_tool (guest on/off/list)|<-- vsock -->\
>>                  |                   -----------------------------              |
>>                  |                                                              |
>>  ----------------v-                  -----------------------------              |
>> |                  |<-- /dev/kvm -->| qemu        VM1             |<-- vsock -->|
>> |                  |                |-------                      |             |
>> |                  |                | Linux |                     |             |
>> | KVM              |                 -----------------------------              |
>> |                  |<-- /dev/kvm -->| qemu        VM2             |<-- vsock -->|
>> |                  |                |---------                    |             |
>> |                  |                | Windows |                   |             |
>> |                  |                 -----------------------------              |
>> |                  |<-- /dev/kvm -->| qemu        VM3      /----->|<-- vsock -->/
>> |           -------|                |---------------------v----   |
>> |          | kvmi  |                | guest introspection tool |  |
>>  ------------------                  -----------------------------
>>
>> There will be a need for a new tool (and/or libvirt modified) to get
>> the guest events (on/off/list) and change the VM1, VM2 invocations (to
>> make them connect with the introspection tool

This kind of event should be provided directly by QEMU to the guest
introspection tool---see below.

>> This might also be a
>> problem with products having the host locked down (eg. RHEV).
> I think that is desirable in fact.  kvmi should be an explicit feature
> that is controlled by the management tools.  This way the policy can be
> decided by the administrator.  Libvirt changes will be necessary.
> 
> Some KVM users do not want kvmi.  Think of the new memory encryption
> hardware support that is coming out - the point is to prevent the
> hypervisor from looking inside the VMs!  What you are doing is the
> opposite of that.

I think Stefan has made quite a point here.  The policy manager for
kvmi should definitely be on the host, not on the introspector machine.
There can be multiple introspectors, some on the host and some on an
appliance, though I suppose a limit of one introspector per VM is
acceptable.

And this should be the starting point of the design.

Compared to Stefan's proposed command line:

  qemu --chardev socket,id=chardev0,type=vsock,port=1234,server,nowait \
       --guest-introspection chardev=chardev0,allowed-cids=10

I would do it in the opposite direction.  The introspector is the one that
presents a server socket; QEMU connects to the introspection VM, possibly
does some handshaking, and passes the file descriptor to KVM.  With another
small change, replacing --guest-introspection with the generic --object, that
gives the following:

  qemu --chardev socket,id=chardev0,type=vsock,cid=10,port=1234,nowait \
       --object introspection chardev=chardev0,allow=all,id=kvmi \
       --accel kvm,introspection=kvmi

The policy is specified via kvmi-{allow,deny} parameters and passed to KVM
via ioctls together with the socket file descriptor.

This lets you reuse common POSIX concepts and simplify the kernel code.
KVMI_EVENT_GUEST_ON is just POLLIN on the server socket (plus handshaking
on the client socket); KVMI_EVENT_GUEST_OFF is POLLHUP on the client socket.
There's no need for KVM to know a UUID, as the introspection application
can just have your usual poll() event loop or thread, and look up the VM
from the file descriptor.

QEMU supports socket reconnection, so you don't need KVMI_GET_GUESTS either.
If KVM cannot write to the socket, it should exit to userspace with a new
KVM_EXIT_KVMI vmexit (which can have multiple subcodes, one of them being
KVM_EXIT_KVMI_SOCKET_ERROR).

Of course the link need not even be VSOCK-based.  It can be a Unix socket
as Stefan has already mentioned, which is always nice when debugging or
writing unit tests.  I assume you'll want later some VMFUNC-based access
to the guest's memory; local introspection tools could use an alternative
way via file descriptor passing, similar to what is used already by vhost-user.
And dually, a hypothetical vhost-user server living in a VM could use VMFUNC
to access guest memory without being able to do all the kind of ugly traps
that your current usecase does.  This is another reason why policy has to
be in userspace.

Also, as a matter of fact: this series does not include either documentation
or unit tests.  That's seriously bad.

Patch 1 should explain the socket protocol in English and only affect
Documentation/ and possibly arch/x86/include/uapi.  There's no way that
I can review 2000 lines of code without even knowing what it is supposed
to be like.  In fact, for the next RFC, perhaps you should only submit
patch 1. :)

Paolo