Re: [Qemu-devel] Re: [PATCH 28/40] xenner: libxc emu: evtchn

[Paolo Bonzini <pbonzini@redhat.com>]

On 11/02/2010 05:33 AM, Stefano Stabellini wrote:
> On Mon, 1 Nov 2010, Anthony Liguori wrote:
>> On 11/01/2010 02:47 PM, Alexander Graf wrote:
>>> Where else would it belong? Qemu is an emulator. Device emulation belongs to qemu code. The xen PV machine is nothing but a special case of the pc machine with custom firmware and odd devices :).
>>>
>>> As I stated in my cover letter, the goal of all this should be to have the qemu pieces be 100% independent of any xen headers or libraries,
>>
>> I'm not sure I agree with the goal.  I think where ever possible we
>> should reuse code with the Xen project when it makes sense.  Reusing
>> blkback/netback is impossible because we want userspace implementations
>> and the current implementations are in the kernel.  blktap also doesn't
>> tie into the QEMU block layer and making it tie into the QEMU block
>> layer would probably result in more code than it saved.
>>
>> OTOH, xenstored and xenconsoled have very little direct dependence on
>> Xen.  I'm not saying that we shouldn't make things Just Work in QEMU, so
>> if that means spawning xenconsoled/xenstored automagically from QEMU
>> with special options, that's perfectly fine.
>>
>> But to replicate the functionality of this code solely because of NIH
>> seems like a waste of effort.
>
> I am a strong proponent of code sharing and reuse so I agree with
> Anthony on this: we should reuse Xen libraries and daemons as much as
> possible. If you need some patches to port xenstored and/or xenconsoled
> to PowerPC we would gladly accept them.

The question is, how much do the Xen userspace and Xenner have in common?

If you remove code that Xen runs in the hypervisor or in the dom0 
kernel, or code that (like xenconsoled) is IMHO best moved to the Xenner 
kernel, what remains is the domain builder and of course xenstore 
handling.  The domain builder is in libxc, which makes it hard to share, 
and this leaves xenstore.

Now, half of it (the ring buffer protocol) already has a million 
duplicate implementation in userspace, in the kernel, in Windows PV 
drivers (at least three independent versions), and is pretty much set in 
stone.

So, what remains is actually parsing the xenstore messages and handling 
the tree data structure.  Which is actually a _very_ small part of 
xenstored: xenstored has to work across multiple domains and clients, be 
careful about inter-domain security, and so on.  Xenner has the _big_ 
advantage of having total independence between domUs (it's like if each 
domU had its own little dom0, its own little xenstore and so on).  While 
it doesn't mean there are no security concerns with guest-facing code, 
it simplifies the code to the point where effectively it makes no sense 
to share anything but the APIs.

I took a look at recent changes to libxs and xenstored in 
xen-unstable.hg. Here are some subjects going back to c/s 17400 (about 
30 months):

- xenstore: libxenstore: fix threading bug which cause xend startup hang
- xenstore: correctly handle errors from read_message
- xenstore: Make sure that libxs reports an error if xenstored drops
- xenstore: Fix cleanup_pop() definition for some (buggy) pthread.h headers.
- xs: avoid pthread_join deadlock in xs_daemon_close
- xs: make sure mutexes are cleaned up and memory freed if the read 
thread is cancelled
- xenstore,libxl: cleanup of xenstore connections across fork()
- xenstored: fix use-after free bug
- xenstore: Fix a memory leak in 'xs_is_domain_introduced'.
- xenstored: Fix xenstored abort when connection dropped.
- xenstore: fix canonicalize for metanodes

Almost all of them are about threading or error conditions, and even 
those that aren't wouldn't apply to a simple implementation like 
Xenner's.  This shows that the risk of missing bugfixes in guest-facing 
code is much smaller than one would think (including what I thought).

(BTW, I noticed that Xenner does not limit guest segments like Xen does. 
  Does it mean the guest can overwrite the Xenner kernel and effectively 
run ring0?)

Paolo