8:32 a.m.
On Tue, Jul 08, 2008 at 05:39:27PM +0100, Daniel P. Berrange wrote:
This replaces the code which converts from the XML into SEXPR
with code which converts from the virDomainDefPtr object to a
SEXPR. We then simply use virDomainDefParseString() to generate
the initial virDomainDefPtr. This makes the SEXPR generating
code much easier to follow.
okay
As part of this cleaned the code all moved out of xml.c and
into xend_internal.c so its in the same place as its inverse
SEXPR -> XML code. I'm half-inclined to actually move all
the SEXPR related code into a xen_conf.{c,h} file independant
of the main driver routine. This would reflect the split we
for QEMU, LXC & OpenVZ drivers.
might make sense, but not urgent
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 ?
@@ -2188,7 +2187,7 @@
if (sys_interface_version >= 4) {
- if (xenDaemonNodeGetTopology(conn, caps) != 0) {
+ if (xenDaemonNodeGetTopology(NULL, caps) != 0) {
Hum, why ?
-char *
-xenHypervisorMakeCapabilitiesXML(virConnectPtr conn,
- const char *hostmachine,
- FILE *cpuinfo, FILE *capabilities)
+virCapsPtr
+xenHypervisorMakeCapabilitiesInternal(const char *hostmachine,
+ FILE *cpuinfo, FILE *capabilities)
I don't understand really
+/**
+ * xenHypervisorMakeCapabilities:
+ *
+ * Return the capabilities of this hypervisor.
+ */
+virCapsPtr
+xenHypervisorMakeCapabilities(void)
+{
+ virCapsPtr caps;
+ FILE *cpuinfo, *capabilities;
+ struct utsname utsname;
+
+ /* Really, this never fails - look at the man-page. */
+ uname (&utsname);
+
+ cpuinfo = fopen ("/proc/cpuinfo", "r");
+ if (cpuinfo == NULL) {
+ if (errno != ENOENT) {
+ virXenPerror (NULL, "/proc/cpuinfo");
+ return NULL;
+ }
+ }
+
+ capabilities = fopen ("/sys/hypervisor/properties/capabilities",
"r");
+ if (capabilities == NULL) {
+ if (errno != ENOENT) {
+ fclose(cpuinfo);
+ virXenPerror (NULL, "/sys/hypervisor/properties/capabilities");
+ return NULL;
+ }
+ }
+
+ caps = xenHypervisorMakeCapabilitiesInternal(utsname.machine, cpuinfo,
capabilities);
+
+ if (cpuinfo)
+ fclose(cpuinfo);
+ if (capabilities)
+ fclose(capabilities);
+
+ return caps;
+}
I really think the connection should be passed down here.
[...]
@@ -3987,6 +3995,7 @@
static int
xenDaemonDetachDevice(virDomainPtr domain, const char *xml)
{
+#if 0
char class[8], ref[80];
if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) {
@@ -3998,6 +4007,8 @@
return (-1);
return(xend_op(domain->conn, domain->name, "op",
"device_destroy",
"type", class, "dev", ref, "force", "0",
"rm_cfg", "1", NULL));
+#endif
+ return (domain == (void*)xml) ? -1 : -1;
}
err, I really don't understand that is that a temporary state which
will be fixed by a later patch ?
[...]
+ 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.
+static int
+xenDaemonFormatSxprSound(virConnectPtr conn,
+ virDomainSoundDefPtr sound,
+ virBufferPtr buf)
+{
+ const char *str;
+ virDomainSoundDefPtr prev = NULL;
+ if (!sound)
+ return 0;
+
+ virBufferAddLit(buf, "(soundhw '");
+ while (sound) {
+ if (!(str = virDomainSoundModelTypeToString(sound->model))) {
+ virXendError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("unexpected sound model %d"), sound->model);
+ return -1;
+ }
+ virBufferVSprintf(buf, "%s%s", prev ? "," : "",
str);
+ prev = sound;
+ sound = sound->next;
+ }
based on this I suppose the problem exposed in the previous patch about
soundhw values, the 2 first values are lost when building the list.
+#if 0
+/**
+ * virDomainXMLDevID:
+ * @domain: pointer to domain object
+ * @xmldesc: string with the XML description
+ * @class: Xen device class "vbd" or "vif" (OUT)
+ * @ref: Xen device reference (OUT)
+ *
+ * Set class according to XML root, and:
+ * - if disk, copy in ref the target name from description
+ * - if network, get MAC address from description, scan XenStore and
+ * copy in ref the corresponding vif number.
+ *
+ * Returns 0 in case of success, -1 in case of failure.
+ */
hum, missing functionality ?
[...]
diff -r ae83be3e7918 tests/testutilsxen.c
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/testutilsxen.c Thu Jul 03 13:02:42 2008 +0100
@@ -0,0 +1,53 @@
+#include <config.h>
+
+#include <sys/utsname.h>
+#include <stdlib.h>
+
+#include "testutilsxen.h"
+
+virCapsPtr testXenCapsInit(void) {
+ struct utsname utsname;
+ virCapsPtr caps;
+ virCapsGuestPtr guest;
+ static const char *const x86_machines[] = {
+ "xenfv"
+ };
+ static const char *const xen_machines[] = {
+ "xenpv"
+ };
+
+ uname (&utsname);
+ if ((caps = virCapabilitiesNew(utsname.machine,
+ 0, 0)) == NULL)
+ return NULL;
I think it's better to pass a NULL here for the connection but still
keep the normal connection error reporting capabilities in the library
code.
+ if ((guest = virCapabilitiesAddGuest(caps, "hvm",
"i686", 32,
+ "/usr/lib/xen/bin/qemu-dm", NULL,
+ 1, x86_machines)) == NULL)
--- a/tests/xml2sexprdata/xml2sexpr-curmem.sexpr Thu Jul 03 12:50:05 2008 +0100
+++ b/tests/xml2sexprdata/xml2sexpr-curmem.sexpr Thu Jul 03 13:02:42 2008 +0100
@@ -1,1 +1,1 @@
-(vm (name 'rhel5')(memory 175)(maxmem 385)(vcpus 1)(uuid
'4f77abd2301958e83bab6fbf2118f880')(bootloader
'/usr/bin/pygrub')(on_poweroff 'destroy')(on_reboot
'restart')(on_crash 'restart')(device (tap (dev 'xvda:disk')(uname
'tap:aio:/xen/rhel5.img')(mode 'w')))(device (vif (mac
'00:16:3e:1d:06:15')(bridge 'xenbr0')(script 'vif-bridge'))))
\ No newline at end of file
+(vm (name 'rhel5')(memory 175)(maxmem 385)(vcpus 1)(uuid
'4f77abd2-3019-58e8-3bab-6fbf2118f880')(bootloader
'/usr/bin/pygrub')(on_poweroff 'destroy')(on_reboot
'restart')(on_crash 'restart')(device (tap (dev 'xvda:disk')(uname
'tap:aio:/xen/rhel5.img')(mode 'w')))(device (vif (mac
'00:16:3e:1d:06:15')(bridge 'xenbr0')(script 'vif-bridge'))))
Hum, we changed the UUID output format ! Isn't there a danger here ? I'm
not sure all versions of Xend will be smart ...
[...]
diff -r ae83be3e7918 tests/xml2sexprdata/xml2sexpr-fv-localtime.xml
--- a/tests/xml2sexprdata/xml2sexpr-fv-localtime.xml Thu Jul 03 12:50:05 2008 +0100
+++ b/tests/xml2sexprdata/xml2sexpr-fv-localtime.xml Thu Jul 03 13:02:42 2008 +0100
@@ -29,7 +29,7 @@
</disk>
<disk type='file'>
<source file='/root/foo.img'/>
- <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 ?
diff -r ae83be3e7918
tests/xml2sexprdata/xml2sexpr-no-source-cdrom.xml
--- a/tests/xml2sexprdata/xml2sexpr-no-source-cdrom.xml Thu Jul 03 12:50:05 2008 +0100
+++ b/tests/xml2sexprdata/xml2sexpr-no-source-cdrom.xml Thu Jul 03 13:02:42 2008 +0100
@@ -25,7 +25,7 @@
<disk type='block' device='disk'>
<driver name='phy'/>
<source dev='/dev/sda8'/>
- <target dev='hda:disk'/>
+ <target dev='hda'/>
</disk>
<disk device='cdrom'>
<target dev='hdc'/>
agreed the input looks a bit stange there :-)
+++ b/tests/xml2sexprdata/xml2sexpr-pv-vfb-new.sexpr Thu Jul 03
13:02:42 2008 +0100
@@ -1,1 +1,1 @@
-(vm (name 'pvtest')(memory 420)(maxmem 420)(vcpus 2)(uuid
'596a5d2171f48fb2e068e2386a5c413e')(on_poweroff 'destroy')(on_reboot
'destroy')(on_crash 'destroy')(image (linux (kernel
'/var/lib/xen/vmlinuz.2Dn2YT')(ramdisk
'/var/lib/xen/initrd.img.0u-Vhq')(args '
method=http://download.fedora.devel.redhat.com/pub/fedora/linux/core/test...
')))(device (vbd (dev 'xvda')(uname 'file:/root/some.img')(mode
'w')))(device (vkbd))(device (vfb (type vnc)(vncdisplay 6)(vnclisten
127.0.0.1)(vncpasswd 123456)(keymap ja))))
\ No newline at end of file
+(vm (name 'pvtest')(memory 420)(maxmem 420)(vcpus 2)(uuid
'596a5d21-71f4-8fb2-e068-e2386a5c413e')(on_poweroff 'destroy')(on_reboot
'destroy')(on_crash 'destroy')(image (linux (kernel
'/var/lib/xen/vmlinuz.2Dn2YT')(ramdisk
'/var/lib/xen/initrd.img.0u-Vhq')(args '
method=http://download.fedora.devel.redhat.com/pub/fedora/linux/core/test...
')(device_model '/usr/lib/xen/bin/qemu-dm')))(device (vbd (dev
'xvda')(uname 'file:/root/some.img')(mode 'w')))(device
(vkbd))(device (vfb (type vnc)(vncunused 0)(vncdisplay 6)(vnclisten
'127.0.0.1')(vncpasswd '123456')(keymap 'ja'))))
Hum we lost more informations here:
(device_model '/usr/lib/xen/bin/qemu-dm')
diff -r ae83be3e7918 tests/xml2sexprdata/xml2sexpr-pv-vfb-orig.sexpr
--- a/tests/xml2sexprdata/xml2sexpr-pv-vfb-orig.sexpr Thu Jul 03 12:50:05 2008 +0100
+++ b/tests/xml2sexprdata/xml2sexpr-pv-vfb-orig.sexpr Thu Jul 03 13:02:42 2008 +0100
@@ -1,1 +1,1 @@
-(vm (name 'pvtest')(memory 420)(maxmem 420)(vcpus 2)(uuid
'596a5d2171f48fb2e068e2386a5c413e')(on_poweroff 'destroy')(on_reboot
'destroy')(on_crash 'destroy')(image (linux (kernel
'/var/lib/xen/vmlinuz.2Dn2YT')(ramdisk
'/var/lib/xen/initrd.img.0u-Vhq')(args '
method=http://download.fedora.devel.redhat.com/pub/fedora/linux/core/test...
')(vnc 1)(vncdisplay 6)(vnclisten 127.0.0.1)(vncpasswd 123456)(keymap ja)))(device
(vbd (dev 'xvda')(uname 'file:/root/some.img')(mode 'w'))))
\ No newline at end of file
+(vm (name 'pvtest')(memory 420)(maxmem 420)(vcpus 2)(uuid
'596a5d21-71f4-8fb2-e068-e2386a5c413e')(on_poweroff 'destroy')(on_reboot
'destroy')(on_crash 'destroy')(image (linux (kernel
'/var/lib/xen/vmlinuz.2Dn2YT')(ramdisk
'/var/lib/xen/initrd.img.0u-Vhq')(args '
method=http://download.fedora.devel.redhat.com/pub/fedora/linux/core/test...
')(vnc 1)(vncunused 0)(vncdisplay 6)(vnclisten '127.0.0.1')(vncpasswd
'123456')(keymap 'ja')))(device (vbd (dev 'xvda')(uname
'file:/root/some.img')(mode 'w'))))
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"/>
Basically same as the previous one, i guess it's globally okay, there is a
few issues, one stylistic and the others on the outputs but which can be
adressed as separate patches on top.
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/