Re: [Libvir] Unified Xen patch (second version)
On Mon, Apr 02, 2007 at 03:15:37PM +0100, Richard W.M. Jones wrote:
>>+static int
>>+qemuNetworkClose (virConnectPtr conn ATTRIBUTE_UNUSED)
>>+{
>>+ /* NB: This will fail to close the connection properly
>>+ * qemuRegister is changed so that it only registers the
>>+ * network (not the connection driver).
>>+ */
>>+ return 0;
>>+}
>
>I'm not sure what you mean by this comment ? I'm fairly sure we need to
>have
>code in qemuNetworkClose() though to close the QEMU socket when the XenD
>driver is used QEMU driver for the networking APIs.
So the current code is complicated and somewhat devious. In the current
code, the qemu network close just reuses qemuClose. In the unified case
this fails because it tries to double-free a structure.
My first time around this I got around the double-free by keeping track
with a usage counter. However that had its own problems, so seeing that
currently we always register the qemu connection and qemu network
drivers together, I just created a separate qemuNetworkClose function
which does nothing. If on the other hand in future we will use qemu
network without qemu connections, then we'll need to change this (hence
added comments). Note that this applies to registration (ie.
vir*Register), not whether or not we manage to connect to libvirtd.
The Xen driver *already* activates the QEMU networking driver so we
have to deal with that situation now. virsh net-list for example fails
with this patch applied now.
The problem is that QEMU network driver impl with this patch is just casting
the privateData straight to qemudPrivatePtr regardless of whether the QEMU
driver is actually active.
I'm attaching a patch which applies on-top of yours to change the qemudNetworkOpen
method to allocate a block of private data specific for the neworking functions.
If the conn->driver->name == QEMU, then we re-use the existing QEMUD socket,
otherwise we open a new one.
>Apart from the 2 questions about suspend/resume/destroy APIs and the QEMU
>networking code, this patch looks fine for inclusion to me. Although it is
>a big patch, it is basically a straight forward re-factoring with no major
>operational changes aside from in the open/close routines.
>
>I'm going to actually try it out for real shortly....
Yeah, I'm also testing it now.
I discovered the aforemetioned issue with Xen driver not being able to use
any of the network driver methods.
There was another interaction with the proxy code - the proxy driver should
not be activated if running as root. In addition if running as non-root,
any Xen driver is allowed to fail, except for the proxy driver. The attached
patch tries to alter the xen_unified.c do_open logic to deal with that.
The proxy itself was SEGV'ing because it didn't allocate the private data
blob, so the patch fixes that too.
I can now open QEMU & XEn as root, and use networking functions, as well
as opening both as non-root.
Finally, the libvirt.c was not checking if conn->networkDriver was NULL,
so if no network driver was activated, the code would SEGV on any network
functions instead of returning operation not supported error.
Regards,
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
Attachments:
- xen-unified-network-2.patch (text/plain — 35.0 KB)