[PATCH 0 of 3] .#2 Memory devices from XML and raw XML

Changes to the third patch only. Sorry the first round didn't have a commit log :)

# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1199911995 28800 # Node ID a229f95979d52c955d46b93c589fd1cf4744ee27 # Parent ca8dc23eacc44970a2d79978bc9de65aaa600b92 Make device_parsing get XML description for a domain once for all phases Also, make memory devices parse the XML instead of ops on a domain. This gives us the ability to create a dominfo structure from XML-only. It would have been nice to put this into two separate patches, but I would have had to fake some memory stuff just to make the reorg work and then remove it in the next patch, so hopefully what I did is palatable to all :) Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r ca8dc23eacc4 -r a229f95979d5 libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c Wed Jan 09 13:08:17 2008 +0100 +++ b/libxkutil/device_parsing.c Wed Jan 09 12:53:15 2008 -0800 @@ -24,6 +24,7 @@ #include <stdio.h> #include <string.h> #include <stdlib.h> +#include <inttypes.h> #include <sys/stat.h> #include <libxml/tree.h> #include <libxml/parser.h> @@ -39,11 +40,13 @@ #define VCPU_XPATH (xmlChar *)"/domain/vcpu" #define NET_XPATH (xmlChar *)"/domain/devices/interface" #define EMU_XPATH (xmlChar *)"/domain/devices/emulator" +#define MEM_XPATH (xmlChar *)"/domain/memory | /domain/currentMemory" #define GRAPHICS_XPATH (xmlChar *)"/domain/devices/graphics" #define DEFAULT_BRIDGE "xenbr0" #define XSTREQ(x, y) (STREQ((char *)x, y)) +#define MAX(a,b) (((a)>(b))?(a):(b)) static void cleanup_disk_device(struct disk_device *dev) { @@ -306,6 +309,38 @@ static int parse_emu_device(xmlNode *nod return 0; } +static int parse_mem_device(xmlNode *node, struct virt_device **vdevs) +{ + struct virt_device *vdev = NULL; + struct mem_device *mdev = NULL; + char *content = NULL; + + vdev = calloc(1, sizeof(*vdev)); + if (vdev == NULL) + goto err; + + mdev = &(vdev->dev.mem); + + content = get_node_content(node); + + if (XSTREQ(node->name, "memory")) + sscanf(content, "%" PRIu64, &mdev->size); + else if (XSTREQ(node->name, "currentMemory")) + sscanf(content, "%" PRIu64, &mdev->maxsize); + + free(content); + + *vdevs = vdev; + + return 1; + + err: + free(content); + free(vdev); + + return 0; +} + static int parse_graphics_device(xmlNode *node, struct virt_device **vdevs) { struct virt_device *vdev = NULL; @@ -366,6 +401,8 @@ static int do_parse(xmlNodeSet *nsv, int do_real_parse = parse_vcpu_device; else if (type == VIRT_DEV_EMU) do_real_parse = parse_emu_device; + else if (type == VIRT_DEV_MEM) + do_real_parse = parse_mem_device; else if (type == VIRT_DEV_GRAPHICS) do_real_parse = parse_graphics_device; else @@ -414,7 +451,7 @@ static void swallow_err_msg(void *ctx, c /* do nothing, just swallow the message. */ } -static int parse_devices(char *xml, struct virt_device **_list, int type) +static int parse_devices(const char *xml, struct virt_device **_list, int type) { int len = 0; int count = 0; @@ -432,6 +469,8 @@ static int parse_devices(char *xml, stru xpathstr = VCPU_XPATH; else if (type == VIRT_DEV_EMU) xpathstr = EMU_XPATH; + else if (type == VIRT_DEV_MEM) + xpathstr = MEM_XPATH; else if (type == VIRT_DEV_GRAPHICS) xpathstr = GRAPHICS_XPATH; else @@ -500,128 +539,60 @@ struct virt_device *virt_device_dup(stru return dev; } -static int get_emu_device(virDomainPtr dom, struct virt_device **dev) +static int _get_mem_device(const char *xml, struct virt_device **list) +{ + struct virt_device *mdevs = NULL; + struct virt_device *mdev = NULL; + int ret; + + ret = parse_devices(xml, &mdevs, VIRT_DEV_MEM); + if (ret <= 0) + return ret; + + mdev = malloc(sizeof(*mdev)); + if (mdev == NULL) + return 0; + + memset(mdev, 0, sizeof(*mdev)); + + /* We could get one or two memory devices back, depending on + * if there is a currentMemory tag or not. Coalesce these + * into a single device to return + */ + + if (ret == 2) { + mdev->dev.mem.size = MAX(mdevs[0].dev.mem.size, + mdevs[1].dev.mem.size); + mdev->dev.mem.maxsize = MAX(mdevs[0].dev.mem.maxsize, + mdevs[1].dev.mem.maxsize); + } else { + mdev->dev.mem.size = MAX(mdevs[0].dev.mem.size, + mdevs[0].dev.mem.maxsize); + mdev->dev.mem.maxsize = mdev->dev.mem.size; + } + + mdev->type = VIRT_DEV_MEM; + mdev->id = strdup("mem"); + *list = mdev; + + cleanup_virt_devices(&mdevs, ret); + + return 1; +} + +int get_devices(virDomainPtr dom, struct virt_device **list, int type) { char *xml; int ret; - struct virt_device *list = NULL; xml = virDomainGetXMLDesc(dom, 0); if (xml == NULL) return 0; - ret = parse_devices(xml, &list, VIRT_DEV_EMU); - if (ret == 1) - *dev = &list[0]; + if (type == VIRT_DEV_MEM) + ret = _get_mem_device(xml, list); 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 == NULL) - 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; - int ret; - - xml = virDomainGetXMLDesc(dom, 0); - if (xml == NULL) - return 0; - - ret = parse_devices(xml, list, VIRT_DEV_DISK); - - free(xml); - - return ret; -} - -int get_net_devices(virDomainPtr dom, struct virt_device **list) -{ - char *xml; - int ret; - - xml = virDomainGetXMLDesc(dom, 0); - if (xml == NULL) - return 0; - - ret = parse_devices(xml, list, VIRT_DEV_NET); - - free(xml); - - return ret; -} - -int get_mem_devices(virDomainPtr dom, struct virt_device **list) -{ - int rc, ret; - uint64_t mem_size, mem_maxsize; - virDomainInfo dom_info; - struct virt_device *ret_list = NULL; - - rc = virDomainGetInfo(dom, &dom_info); - if (rc == -1) { - ret = -1; - goto out; - } - - mem_size = (uint64_t)dom_info.memory; - mem_maxsize = (uint64_t)dom_info.maxMem; - if (mem_size > mem_maxsize) { - ret = -1; - goto out; - } - - ret_list = malloc(sizeof(struct virt_device)); - if (ret_list == NULL) { - ret = -1; - free (ret_list); - goto out; - } - - ret_list->type = VIRT_DEV_MEM; - ret_list->dev.mem.size = mem_size; - ret_list->dev.mem.maxsize = mem_maxsize; - ret_list->id = strdup("mem"); - - ret = 1; - *list = ret_list; - out: - return ret; -} - -int get_vcpu_devices(virDomainPtr dom, struct virt_device **list) -{ - char *xml; - int ret; - - xml = virDomainGetXMLDesc(dom, 0); - if (xml == NULL) - return 0; - - ret = parse_devices(xml, list, VIRT_DEV_VCPU); + ret = parse_devices(xml, list, type); free(xml); @@ -794,14 +765,19 @@ int get_dominfo(virDomainPtr dom, struct goto out; } - get_emu_device(dom, &(*dominfo)->dev_emu); - 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); - (*dominfo)->dev_vcpu_ct = get_vcpu_devices(dom, &(*dominfo)->dev_vcpu); - + parse_devices(xml, &(*dominfo)->dev_emu, VIRT_DEV_EMU); + parse_devices(xml, &(*dominfo)->dev_graphics, VIRT_DEV_GRAPHICS); + + (*dominfo)->dev_mem_ct = _get_mem_device(xml, &(*dominfo)->dev_mem); + (*dominfo)->dev_net_ct = parse_devices(xml, + &(*dominfo)->dev_net, + VIRT_DEV_NET); + (*dominfo)->dev_disk_ct = parse_devices(xml, + &(*dominfo)->dev_disk, + VIRT_DEV_DISK); + (*dominfo)->dev_vcpu_ct = parse_devices(xml, + &(*dominfo)->dev_vcpu, + VIRT_DEV_VCPU); out: free(xml); diff -r ca8dc23eacc4 -r a229f95979d5 libxkutil/device_parsing.h --- a/libxkutil/device_parsing.h Wed Jan 09 13:08:17 2008 +0100 +++ b/libxkutil/device_parsing.h Wed Jan 09 12:53:15 2008 -0800 @@ -137,10 +137,7 @@ int get_dominfo(virDomainPtr dom, struct void cleanup_dominfo(struct domain **dominfo); -int get_disk_devices(virDomainPtr dom, struct virt_device **list); -int get_net_devices(virDomainPtr dom, struct virt_device **list); -int get_vcpu_devices(virDomainPtr dom, struct virt_device **list); -int get_mem_devices(virDomainPtr dom, struct virt_device **list); +int get_devices(virDomainPtr dom, struct virt_device **list, int type); void cleanup_virt_device(struct virt_device *dev); void cleanup_virt_devices(struct virt_device **devs, int count); diff -r ca8dc23eacc4 -r a229f95979d5 src/Virt_Device.c --- a/src/Virt_Device.c Wed Jan 09 13:08:17 2008 +0100 +++ b/src/Virt_Device.c Wed Jan 09 12:53:15 2008 -0800 @@ -283,22 +283,6 @@ int device_type_from_classname(const cha return VIRT_DEV_UNKNOWN; } -static int get_devices(virDomainPtr dom, - struct virt_device **devs, - int type) -{ - if (type == VIRT_DEV_NET) - return get_net_devices(dom, devs); - else if (type == VIRT_DEV_DISK) - return get_disk_devices(dom, devs); - else if (type == VIRT_DEV_MEM) - return get_mem_devices(dom, devs); - else if (type == VIRT_DEV_VCPU) - return get_vcpu_devices(dom, devs); - else - return -1; -} - int dom_devices(const CMPIBroker *broker, virDomainPtr dom, const char *ns, diff -r ca8dc23eacc4 -r a229f95979d5 src/Virt_DevicePool.c --- a/src/Virt_DevicePool.c Wed Jan 09 13:08:17 2008 +0100 +++ b/src/Virt_DevicePool.c Wed Jan 09 12:53:15 2008 -0800 @@ -174,7 +174,7 @@ static char *diskpool_member_of(const CM if (dom == NULL) goto out; - count = get_disk_devices(dom, &devs); + count = get_devices(dom, &devs, VIRT_DEV_DISK); for (i = 0; i < count; i++) { if (STREQ((devs[i].dev.disk.virtual_dev), dev)) { @@ -290,7 +290,7 @@ static char *netpool_member_of(const CMP if (dom == NULL) goto out; - count = get_net_devices(dom, &devs); + count = get_devices(dom, &devs, VIRT_DEV_NET); for (i = 0; i < count; i++) { if (STREQ((devs[i].id), dev)) { diff -r ca8dc23eacc4 -r a229f95979d5 src/Virt_RASD.c --- a/src/Virt_RASD.c Wed Jan 09 13:08:17 2008 +0100 +++ b/src/Virt_RASD.c Wed Jan 09 12:53:15 2008 -0800 @@ -66,16 +66,7 @@ static int list_devs(virConnectPtr conn, if (dom == NULL) return 0; - if (type == CIM_RASD_TYPE_DISK) - return get_disk_devices(dom, list); - else if (type == CIM_RASD_TYPE_NET) - return get_net_devices(dom, list); - else if (type == CIM_RASD_TYPE_PROC) - return get_vcpu_devices(dom, list); - else if (type == CIM_RASD_TYPE_MEM) - return get_mem_devices(dom, list); - else - return 0; + return get_devices(dom, list, type); } static struct virt_device *find_dev(virConnectPtr conn,

# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1199911995 28800 # Node ID a32cbc024d15b65dce1841025fafec5477614953 # Parent a229f95979d52c955d46b93c589fd1cf4744ee27 Add get_dominfo_from_xml() function to device_parsing Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r a229f95979d5 -r a32cbc024d15 libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c Wed Jan 09 12:53:15 2008 -0800 +++ b/libxkutil/device_parsing.c Wed Jan 09 12:53:15 2008 -0800 @@ -742,28 +742,17 @@ static int _get_dominfo(const char *xml, return ret; } -int get_dominfo(virDomainPtr dom, struct domain **dominfo) -{ - char *xml; +int get_dominfo_from_xml(const char *xml, struct domain **dominfo) +{ int ret; *dominfo = malloc(sizeof(**dominfo)); if (*dominfo == NULL) return 0; - xml = virDomainGetXMLDesc(dom, 0); - if (xml == NULL) { - free(*dominfo); - *dominfo = NULL; - return 0; - } - ret = _get_dominfo(xml, *dominfo); - if (ret == 0) { - free(*dominfo); - *dominfo = NULL; - goto out; - } + if (ret == 0) + goto err; parse_devices(xml, &(*dominfo)->dev_emu, VIRT_DEV_EMU); parse_devices(xml, &(*dominfo)->dev_graphics, VIRT_DEV_GRAPHICS); @@ -778,7 +767,27 @@ int get_dominfo(virDomainPtr dom, struct (*dominfo)->dev_vcpu_ct = parse_devices(xml, &(*dominfo)->dev_vcpu, VIRT_DEV_VCPU); -out: + + return ret; + + err: + free(*dominfo); + *dominfo = NULL; + + return 0; +} + +int get_dominfo(virDomainPtr dom, struct domain **dominfo) +{ + char *xml; + int ret; + + xml = virDomainGetXMLDesc(dom, 0); + if (xml == NULL) + return 0; + + ret = get_dominfo_from_xml(xml, dominfo); + free(xml); return ret; diff -r a229f95979d5 -r a32cbc024d15 libxkutil/device_parsing.h --- a/libxkutil/device_parsing.h Wed Jan 09 12:53:15 2008 -0800 +++ b/libxkutil/device_parsing.h Wed Jan 09 12:53:15 2008 -0800 @@ -134,6 +134,7 @@ int disk_type_from_file(const char *path int disk_type_from_file(const char *path); int get_dominfo(virDomainPtr dom, struct domain **dominfo); +int get_dominfo_from_xml(const char *xml, struct domain **dominfo); void cleanup_dominfo(struct domain **dominfo);

# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1199915038 28800 # Node ID 20c65a24edc83c4445527736476a2ad49f2c4066 # Parent a32cbc024d15b65dce1841025fafec5477614953 Make xml_parse_test capable of parsing an XML file to test the get_dominfo_from_xml() functionality. Changes: - Commit log - Removed an extraneous blank line addition Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r a32cbc024d15 -r 20c65a24edc8 libxkutil/xml_parse_test.c --- a/libxkutil/xml_parse_test.c Wed Jan 09 12:53:15 2008 -0800 +++ b/libxkutil/xml_parse_test.c Wed Jan 09 13:43:58 2008 -0800 @@ -1,5 +1,7 @@ #include <stdio.h> #include <inttypes.h> + +#include <getopt.h> #include <libvirt/libvirt.h> @@ -149,40 +151,164 @@ static void print_domxml(struct domain * printf("%s\n", xml); } -int main(int argc, char **argv) -{ - virConnectPtr conn; - virDomainPtr dom; - struct domain *dominfo; - - if (argc < 2) { - printf("Usage: %s domain [URI] [xml]\n", argv[0]); - return 1; - } - - if (argc > 2) - conn = virConnectOpen(argv[2]); - else - conn = virConnectOpen("xen:///"); +static char *read_from_file(FILE *file) +{ + char *xml = NULL; + char buf[256]; + int size = 0; + + while (fgets(buf, sizeof(buf) - 1, file) != NULL) { + xml = realloc(xml, size + strlen(buf) + 1); + if (xml == NULL) { + printf("Out of memory\n"); + return NULL; + } + + strcat(xml, buf); + size += strlen(buf); + } + + return xml; +} + +static int dominfo_from_dom(const char *uri, + const char *domain, + struct domain **d) +{ + virConnectPtr conn = NULL; + virDomainPtr dom = NULL; + int ret = 0; + + conn = virConnectOpen(uri); if (conn == NULL) { printf("Unable to connect to libvirt\n"); + goto out; + } + + dom = virDomainLookupByName(conn, domain); + if (dom == NULL) { + printf("Unable to find domain `%s'\n", domain); + goto out; + } + + ret = get_dominfo(dom, d); + + out: + virDomainFree(dom); + virConnectClose(conn); + + return ret; +} + +static int dominfo_from_file(const char *fname, struct domain **d) +{ + char *xml; + FILE *file; + int ret; + + if (fname[0] == '-') + file = stdin; + else + file = fopen(fname, "r"); + + if (file == NULL) { + printf("Unable to open `%s'\n", fname); + return 0; + } + + xml = read_from_file(file); + if (xml == NULL) { + printf("Unable to read from `%s'\n", fname); + return 0; + } + + ret = get_dominfo_from_xml(xml, d); + + free(xml); + fclose(file); + + printf("XML:\n%s", xml); + + return ret; +} + +static void usage(void) +{ + printf("xml_parse_test -f [FILE | -] [--xml]\n" + "xml_parse_test -d domain [--uri URI] [--xml]\n" + "\n" + "-f,--file FILE Parse domain XML from file (or stdin if -)\n" + "-d,--domain DOM Display dominfo for a domain from libvirt\n" + "-u,--uri URI Connect to libvirt with URI\n" + "-x,--xml Dump generated XML instead of summary\n" + "-h,--help Display this help message\n"); +} + +int main(int argc, char **argv) +{ + int c; + char *domain = NULL; + char *uri = "xen"; + char *file = NULL; + bool xml = false; + struct domain *dominfo = NULL; + int ret; + + static struct option lopts[] = { + {"domain", 1, 0, 'd'}, + {"uri", 1, 0, 'u'}, + {"xml", 0, 0, 'x'}, + {"file", 1, 0, 'f'}, + {"help", 0, 0, 'h'}, + {0, 0, 0, 0}}; + + while (1) { + int optidx = 0; + + c = getopt_long(argc, argv, "d:u:f:xh", lopts, &optidx); + if (c == -1) + break; + + switch (c) { + case 'd': + domain = optarg; + break; + + case 'u': + uri = optarg; + break; + + case 'f': + file = optarg; + break; + + case 'x': + xml = true; + break; + + case '?': + case 'h': + usage(); + return c == '?'; + + }; + } + + if (file != NULL) + ret = dominfo_from_file(file, &dominfo); + else if (domain != NULL) + ret = dominfo_from_dom(uri, domain, &dominfo); + else { + printf("Need a data source (--domain or --file)\n"); + return 1; + } + + if (ret == 0) { + printf("Unable to get dominfo\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"); - - if ((argc > 3) && (argv[3][0] == 'x')) + if (xml) print_domxml(dominfo, stdout); else { print_dominfo(dominfo, stdout);

Dan Smith wrote:
Changes to the third patch only. Sorry the first round didn't have a commit log :)
The consolidation of get_* functions into the get_devices() is nice. Makes the patch a bit tough to read, but the resulting device_parsing.c file is much easier to read. +1 -- Kaitlin Rupert IBM Linux Technology Center karupert@us.ibm.com

Kaitlin Rupert wrote:
Dan Smith wrote:
Changes to the third patch only. Sorry the first round didn't have a commit log :)
The consolidation of get_* functions into the get_devices() is nice. Makes the patch a bit tough to read, but the resulting device_parsing.c file is much easier to read. +1 I do not have that many experiences with the device-parsing stuff, but I agree, that the code is good to read. Testing is a bit difficult, as I get the following message when doing an ein on VSSD: misc_util.c(70): Connecting to libvirt with uri `qemu:///system' device_parsing.c(818): Unknown domain type -1
Seems like KVM is not supported at the moment. But the instance for VSSD is returned by ein and gi. So a +1 with the excuse for not testing it correct. -- Regards Heidi Eckhart Software Engineer IBM Linux Technology Center - Open Hypervisor

HE> Seems like KVM is not supported at the moment. But the instance for HE> VSSD is returned by ein and gi. So a +1 with the excuse for not HE> testing it correct. Hmm, it seems to work okay for me: % wbemcli ein http://root:password@localhost/root/virt:CIM_VirtualSystemSettingData localhost:5988/root/virt:KVM_VirtualSystemSettingData.InstanceID="KVM:kvm" -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com

HE> misc_util.c(70): Connecting to libvirt with uri `qemu:///system' HE> device_parsing.c(818): Unknown domain type -1 Okay, I should start looking at the code before I speak :) It looks to me like you have a KVM domain with XML that doesn't fit with my type detection logic, hence the -1 domain type. Right now, if the <os> block has a type of 'hvm', and the <domain> tag has a type of 'kvm', then we consider it to be a KVM domain. However, I'm assuming you're missing the type in your <os> block, which really shouldn't be necessary (it's needed to determine between Xen PV and FV), so maybe removing that check would fix this problem. I'll submit a patch to do that; please report if it fixes your problem. Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
participants (3)
-
Dan Smith
-
Heidi Eckhart
-
Kaitlin Rupert