On Thu, Jul 24, 2008 at 04:49:50PM +0100, Daniel P. Berrange wrote: > On Thu, Jul 24, 2008 at 09:32:41AM -0400, Daniel Veillard wrote: > > On Tue, Jul 08, 2008 at 05:39:27PM +0100, Daniel P. Berrange wrote: > > > > > static virCapsPtr > > > -xenHypervisorBuildCapabilities(virConnectPtr conn, > > > - const char *hostmachine, > > > +xenHypervisorBuildCapabilities(const char *hostmachine, > > > int host_pae, > > > char *hvm_type, > > > struct guest_arch *guest_archs, > > > > Sounds fishy obviously some error can occur and not propagating > > the connection how to you carry the error properly to the remote > > connection using the Xen node ? For example virCaps allocation may > > fail no ? > > Previously the code for building the virCaps object would be > called every time the virConnectGetCapabilities() API was > invokved, so it'd have a virConnectPtr object. > > After this re-factoring the virCaps object is created at the > time the connection is opened, so no virConnectPtr object yet > exists. The reason for this is that the virCaps object is > used in the XML parsers/formatters now so we need it around > all the time. > > The errors are still reported - they'll be reported as a > failure to cthe virConnectOpen() call, accessible via the > global virGetLastError() API. Ah, okay, I remember now this been discussed, but i had forgotten ! > Urgh, I've posted the wrong version of this patch. The one on my machine > implements this correctly. haha, :-) > > [...] > > > + switch (def->type) { > > > + case VIR_DOMAIN_NET_TYPE_BRIDGE: > > > + virBufferVSprintf(buf, "(bridge '%s')", def->data.bridge.brname); > > > + virBufferAddLit(buf, "(script 'vif-bridge')"); > > > > hum, see the problem reported in the previous patch. i think we should > > carry the bridge script in the definition structure or loose some functionality > > in the transition. > > The issue is that when converting from a SEXPR -> XML format, the Xend > driver does > > > if (STREQ(script, "vif-bridge")) > type = BRIDGE > else > type = ETHERNET > > So, if you pass a custom script for vif-bridge, that check won't match > and thus the XML you get back out is > > <interface type='ethernet'> > > So, it seems pointless adding <script> element for <interface type='bridge'> > since it'll never change. okay, that removes my concern, there is no loss of functionlity possible. > Yep, also fixed on my laptop. Let's push the final version to CVS, really ! > > Hum, we changed the UUID output format ! Isn't there a danger here ? I'm > > not sure all versions of Xend will be smart ... > > The xm_internal.c driver already used the format including '-', but > I could change this back if desired. nahh as long as it doesn't break, I don't care > > > - <target dev='ioemu:hda'/> > > > + <target dev='hda'/> > > > </disk> > > > <graphics type='vnc' port='5917' keymap='ja'/> > > > </devices> > > > > hum, why changing the inputs ? we can't parse it ? > > Actaully we can parse & ignore this prefix. I can get rid of > this tweak to the XML - its left-over from an intermediate > version of my patch. okay > > We add (vncunused 0) I must admit i don't see what it could mean in that > > context the input is > > <graphics type='vnc' port='5906' listen="127.0.0.1" passwd="123456" keymap="ja"/> > > It is a safety net - we have an explicit port, given via (vncdisplay 6), > but adding the (vncunused 0) is just a safety net incase XenD changes > its default handling - one version of xen-unstable did this before, > but we caught it before release. Okay +1 Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list