# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1199395653 28800 # Node ID 65a07575d2cfc4dfd3c1d57e0a0828c88dacf540 # Parent 3a838dfd165b5f721687a2bb0b2e03bb0c8192b7 Add preliminary XML-parsing support for Xen FV domains This adds a type switch for XenPV, XenFV, KVM to the struct domain. It also adds parsing of 'graphics' and 'emulator' devices. Finally, amidst the compulsory changes to xmlgen.c are modifications to generate a proper <os> block for XenFV domains. More work will need to be done here, but this is an example of how the domain->type switch will be used to select different generation routines. Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r 3a838dfd165b -r 65a07575d2cf libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c Thu Jan 03 08:22:48 2008 -0800 +++ b/libxkutil/device_parsing.c Thu Jan 03 13:27:33 2008 -0800 @@ -36,6 +36,8 @@ #define DISK_XPATH (xmlChar *)"/domain/devices/disk" #define NET_XPATH (xmlChar *)"/domain/devices/interface" +#define EMU_XPATH (xmlChar *)"/domain/devices/emulator" +#define GRAPHICS_XPATH (xmlChar *)"/domain/devices/graphics" #define DEFAULT_BRIDGE "xenbr0" @@ -195,6 +197,25 @@ static int parse_net_device(xmlNode *ino return 0; } +static int parse_emu_device(xmlNode *node, struct virt_device *vdev) +{ + struct emu_device *edev = &(vdev->dev.emu); + + edev->path = get_node_content(node); + + return 1; +} + +static int parse_graphics_device(xmlNode *node, struct virt_device *vdev) +{ + struct graphics_device *gdev = &(vdev->dev.graphics); + + gdev->type = get_attr_value(node, "type"); + gdev->port = get_attr_value(node, "port"); + + return 1; +} + static int do_parse(xmlNodeSet *nsv, int type, struct virt_device **l) { int i = 0; @@ -209,6 +230,10 @@ static int do_parse(xmlNodeSet *nsv, int do_real_parse = &parse_net_device; else if (type == VIRT_DEV_DISK) do_real_parse = &parse_disk_device; + else if (type == VIRT_DEV_EMU) + do_real_parse = parse_emu_device; + else if (type == VIRT_DEV_GRAPHICS) + do_real_parse = parse_graphics_device; else goto err; @@ -260,6 +285,10 @@ static int parse_devices(char *xml, stru xpathstr = NET_XPATH; else if (type == VIRT_DEV_DISK) xpathstr = DISK_XPATH; + else if (type == VIRT_DEV_EMU) + xpathstr = EMU_XPATH; + else if (type == VIRT_DEV_GRAPHICS) + xpathstr = GRAPHICS_XPATH; else goto err1; @@ -324,6 +353,48 @@ struct virt_device *virt_device_dup(stru return dev; } +static int get_emu_device(virDomainPtr dom, struct virt_device **dev) +{ + char *xml; + int ret; + struct virt_device *list = NULL; + + xml = virDomainGetXMLDesc(dom, 0); + if (!xml) + return 0; + + ret = parse_devices(xml, &list, VIRT_DEV_EMU); + if (ret == 1) + *dev = &list[0]; + else + *dev = NULL; + + free(xml); + + return ret; +} + +static int get_graphics_device(virDomainPtr dom, struct virt_device **dev) +{ + char *xml; + int ret; + struct virt_device *list = NULL; + + xml = virDomainGetXMLDesc(dom, 0); + if (!xml) + return 0; + + ret = parse_devices(xml, &list, VIRT_DEV_GRAPHICS); + if (ret == 1) + *dev = &list[0]; + else + *dev = NULL; + + free(xml); + + return ret; +} + int get_disk_devices(virDomainPtr dom, struct virt_device **list) { char *xml; @@ -474,14 +545,32 @@ static int parse_os(struct domain *domin for (child = os->children; child != NULL; child = child->next) { if (XSTREQ(child->name, "type")) - STRPROP(dominfo, os_info.type, child); + STRPROP(dominfo, os_info.pv.type, child); else if (XSTREQ(child->name, "kernel")) - STRPROP(dominfo, os_info.kernel, child); + STRPROP(dominfo, os_info.pv.kernel, child); else if (XSTREQ(child->name, "initrd")) - STRPROP(dominfo, os_info.initrd, child); + STRPROP(dominfo, os_info.pv.initrd, child); else if (XSTREQ(child->name, "cmdline")) - STRPROP(dominfo, os_info.cmdline, child); - } + STRPROP(dominfo, os_info.pv.cmdline, child); + else if (XSTREQ(child->name, "loader")) + STRPROP(dominfo, os_info.fv.loader, child); + else if (XSTREQ(child->name, "boot")) + dominfo->os_info.fv.boot = get_attr_value(child, + "dev"); + } + + printf("Type string: %s\n", dominfo->typestr); + + if ((STREQC(dominfo->os_info.fv.type, "hvm")) && + (STREQC(dominfo->typestr, "xen"))) + dominfo->type = DOMAIN_XENFV; + else if ((STREQC(dominfo->os_info.fv.type, "hvm")) && + (STREQC(dominfo->typestr, "kvm"))) + dominfo->type = DOMAIN_KVM; + else if (STREQC(dominfo->os_info.pv.type, "linux")) + dominfo->type = DOMAIN_XENPV; + else + dominfo->type = -1; return 1; } @@ -508,6 +597,8 @@ static int parse_domain(xmlNodeSet *nsv, xmlNode *child; memset(dominfo, 0, sizeof(*dominfo)); + + dominfo->typestr = get_attr_value(nodes[0], "type"); for (child = nodes[0]->children; child != NULL; child = child->next) { if (XSTREQ(child->name, "name")) @@ -586,6 +677,9 @@ int get_dominfo(virDomainPtr dom, struct goto out; } + ret = get_emu_device(dom, &(*dominfo)->dev_emu); + ret = get_graphics_device(dom, &(*dominfo)->dev_graphics); + (*dominfo)->dev_mem_ct = get_mem_devices(dom, &(*dominfo)->dev_mem); (*dominfo)->dev_net_ct = get_net_devices(dom, &(*dominfo)->dev_net); (*dominfo)->dev_disk_ct = get_disk_devices(dom, &(*dominfo)->dev_disk); @@ -608,10 +702,18 @@ void cleanup_dominfo(struct domain **dom free(dom->uuid); free(dom->bootloader); free(dom->bootloader_args); - free(dom->os_info.type); - free(dom->os_info.kernel); - free(dom->os_info.initrd); - free(dom->os_info.cmdline); + + free(dom->os_info.pv.type); + if (dom->type == DOMAIN_XENPV) { + free(dom->os_info.pv.kernel); + free(dom->os_info.pv.initrd); + free(dom->os_info.pv.cmdline); + } else if (dom->type == DOMAIN_XENFV) { + free(dom->os_info.fv.loader); + free(dom->os_info.fv.boot); + } else { + CU_DEBUG("Unknown domain type %i", dom->type); + } cleanup_virt_devices(&dom->dev_mem, dom->dev_mem_ct); cleanup_virt_devices(&dom->dev_net, dom->dev_net_ct); diff -r 3a838dfd165b -r 65a07575d2cf libxkutil/device_parsing.h --- a/libxkutil/device_parsing.h Thu Jan 03 08:22:48 2008 -0800 +++ b/libxkutil/device_parsing.h Thu Jan 03 13:27:33 2008 -0800 @@ -48,38 +48,67 @@ struct mem_device { uint64_t maxsize; }; +struct emu_device { + char *path; +}; + +struct graphics_device { + char *type; + char *port; +}; + struct virt_device { enum {VIRT_DEV_UNKNOWN, VIRT_DEV_NET = CIM_RASD_TYPE_NET, VIRT_DEV_DISK = CIM_RASD_TYPE_DISK, VIRT_DEV_MEM = CIM_RASD_TYPE_MEM, VIRT_DEV_VCPU = CIM_RASD_TYPE_PROC, + VIRT_DEV_EMU, + VIRT_DEV_GRAPHICS, } type; union { struct disk_device disk; struct net_device net; struct mem_device mem; struct _virVcpuInfo vcpu; + struct emu_device emu; + struct graphics_device graphics; } dev; char *id; }; -struct os_info { +struct pv_os_info { char *type; char *kernel; char *initrd; char *cmdline; }; +struct fv_os_info { + char *type; /* Should always be 'hvm' */ + char *loader; + char *boot; +}; + struct domain { + enum { DOMAIN_XENPV, DOMAIN_XENFV, DOMAIN_KVM } type; char *name; + char *typestr; /*xen, kvm, etc */ char *uuid; char *bootloader; char *bootloader_args; - struct os_info os_info; + + union { + struct pv_os_info pv; + struct fv_os_info fv; + } os_info; + int on_poweroff; int on_reboot; int on_crash; + + struct virt_device *dev_graphics; + struct virt_device *dev_emu; struct virt_device *dev_mem; int dev_mem_ct; diff -r 3a838dfd165b -r 65a07575d2cf libxkutil/xmlgen.c --- a/libxkutil/xmlgen.c Thu Jan 03 08:22:48 2008 -0800 +++ b/libxkutil/xmlgen.c Thu Jan 03 13:27:33 2008 -0800 @@ -271,9 +271,9 @@ static char *system_xml(struct domain *d return xml; } -static char *os_xml(struct domain *domain) -{ - struct os_info *os = &domain->os_info; +static char *_xenpv_os_xml(struct domain *domain) +{ + struct pv_os_info *os = &domain->os_info.pv; int ret; char *xml; char *type = NULL; @@ -312,6 +312,60 @@ static char *os_xml(struct domain *domai free(cmdline); return xml; +} + +static char *_xenfv_os_xml(struct domain *domain) +{ + struct fv_os_info *os = &domain->os_info.fv; + int ret; + char *xml; + char *type; + char *loader; + char *boot; + struct kv bootattr = {"dev", NULL}; + + if (os->type == NULL) + os->type = strdup("hvm"); + + if (os->loader == NULL) + os->loader = strdup("/usr/lib/xen/boot/hvmloader"); + + if (os->boot == NULL) + os->boot = strdup("hd"); + + type = tagify("type", os->type, NULL, 0); + loader = tagify("loader", os->loader, NULL, 0); + + bootattr.val = os->boot; + boot = tagify("boot", NULL, &bootattr, 1); + + ret = asprintf(&xml, + "<os>\n" + " %s\n" + " %s\n" + " %s\n" + "</os>\n", + type, + loader, + boot); + if (ret == -1) + xml = NULL; + + free(type); + free(loader); + free(boot); + + return xml; +} + +static char *os_xml(struct domain *domain) +{ + if (domain->type == DOMAIN_XENPV) + return _xenpv_os_xml(domain); + else if (domain->type == DOMAIN_XENFV) + return _xenfv_os_xml(domain); + else + return strdup("<!-- unsupported domain type -->\n"); } char *system_to_xml(struct domain *dominfo)
Dan Smith wrote:
+ + printf("Type string: %s\n", dominfo->typestr);
This should probably be a CU_DEBUG() statement.
+ + free(dom->os_info.pv.type); + if (dom->type == DOMAIN_XENPV) { + free(dom->os_info.pv.kernel); + free(dom->os_info.pv.initrd); + free(dom->os_info.pv.cmdline); + } else if (dom->type == DOMAIN_XENFV) { + free(dom->os_info.fv.loader); + free(dom->os_info.fv.boot); + } else { + CU_DEBUG("Unknown domain type %i", dom->type); + }
Does os_info.fv.type need to be freed as well? -- Kaitlin Rupert IBM Linux Technology Center karupert@us.ibm.com
KR> This should probably be a CU_DEBUG() statement. Actually, it should be removed. That was left over from some debug. Thanks :)
+ + free(dom->os_info.pv.type); + if (dom->type == DOMAIN_XENPV) { + free(dom->os_info.pv.kernel); + free(dom->os_info.pv.initrd); + free(dom->os_info.pv.cmdline); + } else if (dom->type == DOMAIN_XENFV) { + free(dom->os_info.fv.loader); + free(dom->os_info.fv.boot); + } else { + CU_DEBUG("Unknown domain type %i", dom->type); + }
KR> Does os_info.fv.type need to be freed as well? No, because os_info.pv.type and os_info.fv_type are the same value. However, this is ugly and left over from earlier bits in the implementation. I'll change it up to use the type switch to free the correct field in the body of the if. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1199395653 28800 # Node ID 65a07575d2cfc4dfd3c1d57e0a0828c88dacf540 # Parent 3a838dfd165b5f721687a2bb0b2e03bb0c8192b7 Add preliminary XML-parsing support for Xen FV domains
This adds a type switch for XenPV, XenFV, KVM to the struct domain. It also adds parsing of 'graphics' and 'emulator' devices. Finally, amidst the compulsory changes to xmlgen.c are modifications to generate a proper <os> block for XenFV domains. More work will need to be done here, but this is an example of how the domain->type switch will be used to select different generation routines.
Signed-off-by: Dan Smith <danms@us.ibm.com>
+ +static int get_graphics_device(virDomainPtr dom, struct virt_device **dev) +{ + char *xml; + int ret; + struct virt_device *list = NULL; + + xml = virDomainGetXMLDesc(dom, 0); + if (!xml) + return 0;
I suppose this speaks well for the quality of the patch when this is all I can find to complain about, but isn't that a spot where we typically use "if (xml == NULL)" for the sake of clarity? Using "(!xml)" for the conditional tends to make me think of xml as an int. -- -Jay
JG> isn't that a spot where we typically use "if (xml == NULL)" for JG> the sake of clarity? Using "(!xml)" for the conditional tends to JG> make me think of xml as an int. Yes, and it pained me to do it this way. However, I was keeping with the rest of Zhengang's style in that file. I think it would be good to leave it as-is for this patch and then work up a follow-on that corrects quite a few style issues in that particular file. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
JG> isn't that a spot where we typically use "if (xml == NULL)" for JG> the sake of clarity? Using "(!xml)" for the conditional tends to JG> make me think of xml as an int.
Yes, and it pained me to do it this way. However, I was keeping with the rest of Zhengang's style in that file.
I think it would be good to leave it as-is for this patch and then work up a follow-on that corrects quite a few style issues in that particular file.
Okay, that works for me. -- -Jay
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1199395746 28800 # Node ID 003644d3e86528251c0c84b52af6a92a81253682 # Parent 65a07575d2cfc4dfd3c1d57e0a0828c88dacf540 Add XML parsing tester for use when working on Xen PV and KVM support This simply takes a domain argument, gets the XML from libvirt, parses it in the same way the providers will, and prints the domain information in a structured way. This can be used to test the previous patch, by validating the FV-specific data items, such as emulator and the HVM loader path. Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r 65a07575d2cf -r 003644d3e865 libxkutil/Makefile.am --- a/libxkutil/Makefile.am Thu Jan 03 13:27:33 2008 -0800 +++ b/libxkutil/Makefile.am Thu Jan 03 13:29:06 2008 -0800 @@ -12,3 +12,9 @@ AM_LDFLAGS = -lvirt -luuid libxkutil_la_SOURCES = cs_util_instance.c misc_util.c device_parsing.c \ xmlgen.c + +noinst_PROGRAMS = xml_parse_test + +xml_parse_test_SOURCES = xml_parse_test.c +#xml_parse_test_HEADERS = device_parsing.h +xml_parse_test_LDADD = -lvirt -lxkutil diff -r 65a07575d2cf -r 003644d3e865 libxkutil/xml_parse_test.c --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/libxkutil/xml_parse_test.c Thu Jan 03 13:29:06 2008 -0800 @@ -0,0 +1,172 @@ +#include <stdio.h> + +#include <libvirt/libvirt.h> + +#include "device_parsing.h" + +static void print_value(FILE *d, const char *name, const char *val) +{ + fprintf(d, "%-15s: %s\n", name, val); +} + +static void print_u64(FILE *d, const char *name, uint64_t val) +{ + fprintf(d, "%-15s: %lu\n", name, val); +} + +static void print_os(struct domain *dom, + FILE *d) +{ + + if (dom->type == DOMAIN_XENPV) { + print_value(d, "Domain Type", "Xen PV"); + print_value(d, "Type", dom->os_info.pv.type); + print_value(d, "Kernel", dom->os_info.pv.kernel); + print_value(d, "Ramdisk", dom->os_info.pv.initrd); + print_value(d, "Args", dom->os_info.pv.cmdline); + } else if (dom->type == DOMAIN_XENFV) { + print_value(d, "Domain Type", "Xen FV"); + print_value(d, "Type", dom->os_info.fv.type); + print_value(d, "Loader", dom->os_info.fv.loader); + print_value(d, "Boot", dom->os_info.fv.boot); + } else { + fprintf(d, "[ Unknown domain type %i ]\n", dom->type); + } +} + +static void print_dominfo(struct domain *dominfo, + FILE *d) +{ + print_value(d, "Name", dominfo->name); + print_value(d, "UUID", dominfo->uuid); + print_value(d, "Bootloader", dominfo->bootloader); + print_value(d, " args", dominfo->bootloader_args); + + fprintf(d, "Actions: : P:%i R:%i C:%i\n", + dominfo->on_poweroff, + dominfo->on_reboot, + dominfo->on_crash); + + print_os(dominfo, d); +} + +static void print_dev_mem(struct virt_device *dev, + FILE *d) +{ + print_u64(d, "Memory", dev->dev.mem.size); + print_u64(d, "Maximum", dev->dev.mem.maxsize); +} +static void print_dev_net(struct virt_device *dev, + FILE *d) +{ + print_value(d, "Type", dev->dev.net.type); + print_value(d, "MAC", dev->dev.net.mac); +} + +static void print_dev_disk(struct virt_device *dev, + FILE *d) +{ + print_value(d, "Type", dev->dev.disk.type); + print_value(d, "Device", dev->dev.disk.device); + print_value(d, "Driver", dev->dev.disk.driver); + print_value(d, "Source", dev->dev.disk.source); + print_value(d, "Virt Device", dev->dev.disk.virtual_dev); +} + +static void print_dev_vcpu(struct virt_device *dev, + FILE *d) +{ + print_value(d, "Virtual CPU", "Present"); +} + +static void print_dev_emu(struct virt_device *dev, + FILE *d) +{ + print_value(d, "Emulator", dev->dev.emu.path); +} + +static void print_dev_graphics(struct virt_device *dev, + FILE *d) +{ + print_value(d, "Graphics Type", dev->dev.graphics.type); + print_value(d, "Graphics Port", dev->dev.graphics.port); +} + +static void print_devices(struct domain *dominfo, + FILE *d) +{ + int i; + + fprintf(d, "\n-- Memory (%i) --\n", dominfo->dev_mem_ct); + for (i = 0; i < dominfo->dev_mem_ct; i++) + print_dev_mem(&dominfo->dev_mem[i], d); + + fprintf(d, "\n-- Network (%i) --\n", dominfo->dev_net_ct); + for (i = 0; i < dominfo->dev_net_ct; i++) + print_dev_net(&dominfo->dev_net[i], d); + + fprintf(d, "\n-- Disk (%i) --\n", dominfo->dev_disk_ct); + for (i = 0; i < dominfo->dev_disk_ct; i++) + print_dev_disk(&dominfo->dev_disk[i], d); + + fprintf(d, "\n-- VCPU (%i) -- \n", dominfo->dev_vcpu_ct); + for (i = 0; i < dominfo->dev_vcpu_ct; i++) + print_dev_vcpu(&dominfo->dev_vcpu[i], d); + + if (dominfo->type != DOMAIN_XENPV) { + fprintf(d, "\n-- Emulator --\n"); + print_dev_emu(dominfo->dev_emu, d); + } + + if (dominfo->dev_graphics) { + fprintf(d, "\n-- Graphics --\n"); + print_dev_graphics(dominfo->dev_graphics, d); + } +} + +int main(int argc, char **argv) +{ + virConnectPtr conn; + virDomainPtr dom; + struct domain *dominfo; + + if (argc != 2) { + printf("Usage: %s domain\n", argv[0]); + return 1; + } + + conn = virConnectOpen("xen:///"); + if (conn == NULL) { + printf("Unable to connect to libvirt\n"); + return 2; + } + + dom = virDomainLookupByName(conn, argv[1]); + if (dom == NULL) { + printf("Unable to lookup domain `%s'\n", argv[1]); + return 3; + } + + if (get_dominfo(dom, &dominfo) == 0) { + printf("Failed to parse domain info\n"); + return 4; + } + + printf("Parsed domain info\n"); + + print_dominfo(dominfo, stdout); + print_devices(dominfo, stdout); + + return 0; +} + + +/* + * Local Variables: + * mode: C + * c-set-style: "K&R" + * tab-width: 8 + * c-basic-offset: 8 + * indent-tabs-mode: nil + * End: + */
Dan Smith wrote:
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1199395746 28800 # Node ID 003644d3e86528251c0c84b52af6a92a81253682 # Parent 65a07575d2cfc4dfd3c1d57e0a0828c88dacf540 Add XML parsing tester for use when working on Xen PV and KVM support
This is a neat test. It worked with my paravirt guest. I don't have a fullvirt guest to test with, but the code looks sane enough. -- Kaitlin Rupert IBM Linux Technology Center karupert@us.ibm.com
participants (3)
-
Dan Smith -
Jay Gagnon -
Kaitlin Rupert