On 21/06/2017 13:04, Stefan Hajnoczi wrote:> On Tue, Jun 20, 2017 at 05:58:41PM +0300, alazar@xxxxxxxxxxxxxxx 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