[PATCH 0 of 3] Fix offline VCPU representation
Currently, we cannot present information and instances for VCPUs when a domain is powered off, because virDomainGetInfo() does not work unless a domain is running. This set ultimately changes the vcpu bits in device_parsing to, that's right, actually *parse* the VCPU information from the XML like all the other devices, which gives us VCPU information for offline domains. To get there, I had to make the core of device_parsing a little more dyanmic so that the parsing modules could return mulitple devices for a given XML cycle, which vcpu needs.
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1199827433 28800 # Node ID 7272934dc44185bef3923c86fa19961d299518fd # Parent 10d141f683370b6f04637bb9ad059bbd92051ce0 Fix xml_parse_test if no emulator is defined Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r 10d141f68337 -r 7272934dc441 libxkutil/xml_parse_test.c --- a/libxkutil/xml_parse_test.c Tue Jan 08 10:51:44 2008 -0800 +++ b/libxkutil/xml_parse_test.c Tue Jan 08 13:23:53 2008 -0800 @@ -121,7 +121,7 @@ static void print_devices(struct domain for (i = 0; i < dominfo->dev_vcpu_ct; i++) print_dev_vcpu(&dominfo->dev_vcpu[i], d); - if (dominfo->type != DOMAIN_XENPV) { + if ((dominfo->type != DOMAIN_XENPV) && (dominfo->dev_emu)) { fprintf(d, "\n-- Emulator --\n"); print_dev_emu(dominfo->dev_emu, d); }
Dan Smith wrote:
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1199827433 28800 # Node ID 7272934dc44185bef3923c86fa19961d299518fd # Parent 10d141f683370b6f04637bb9ad059bbd92051ce0 Fix xml_parse_test if no emulator is defined
Signed-off-by: Dan Smith <danms@us.ibm.com>
diff -r 10d141f68337 -r 7272934dc441 libxkutil/xml_parse_test.c --- a/libxkutil/xml_parse_test.c Tue Jan 08 10:51:44 2008 -0800 +++ b/libxkutil/xml_parse_test.c Tue Jan 08 13:23:53 2008 -0800 @@ -121,7 +121,7 @@ static void print_devices(struct domain for (i = 0; i < dominfo->dev_vcpu_ct; i++) print_dev_vcpu(&dominfo->dev_vcpu[i], d);
- if (dominfo->type != DOMAIN_XENPV) { + if ((dominfo->type != DOMAIN_XENPV) && (dominfo->dev_emu)) { fprintf(d, "\n-- Emulator --\n"); print_dev_emu(dominfo->dev_emu, d); }
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
Well I gotta say, I'm a bit disappointed. All that lead up to the set, and the first patch doesn't even produce a vertical scrollbar in thunderbird. At the very least you could have done some pointer arithmetic. +1, I guess. -- -Jay
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1199827433 28800 # Node ID 3e5265023ddda5cfa24e7449c9f06bbd7dcaba5c # Parent 7272934dc44185bef3923c86fa19961d299518fd Allow device_parsing modules to return multiple devices per cycle Do this by growing the device list during the cycle instead of trying to predict it ahead of time based on the numbers of nodes in the XML tree. This could be a little nicer with a more integrated list structure or a linked-list of some kind, but this is the smallest change to accomplish the goal, which I think is a good idea for now. The need for this is apparent in the next patch for vcpu parsing. Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r 7272934dc441 -r 3e5265023ddd libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c Tue Jan 08 13:23:53 2008 -0800 +++ b/libxkutil/device_parsing.c Tue Jan 08 13:23:53 2008 -0800 @@ -130,12 +130,17 @@ static char *get_node_content(xmlNode *n return buf; } -static bool parse_disk_device(xmlNode *dnode, struct virt_device *vdev) -{ - struct disk_device *ddev = &(vdev->dev.disk); +static int parse_disk_device(xmlNode *dnode, struct virt_device **vdevs) +{ + struct virt_device *vdev = NULL; + struct disk_device *ddev = NULL; xmlNode * child = NULL; - memset(ddev, 0, sizeof(*ddev)); + vdev = calloc(1, sizeof(*vdev)); + if (vdev == NULL) + goto err; + + ddev = &(vdev->dev.disk); ddev->type = get_attr_value(dnode, "type"); if (ddev->type == NULL) @@ -174,20 +179,28 @@ static bool parse_disk_device(xmlNode *d vdev->type = VIRT_DEV_DISK; vdev->id = strdup(ddev->virtual_dev); - return true; + *vdevs = vdev; + + return 1; err: cleanup_disk_device(ddev); - - return false; -} - -static bool parse_net_device(xmlNode *inode, struct virt_device *vdev) -{ - struct net_device *ndev = &(vdev->dev.net); + free(vdev); + + return 0; +} + +static int parse_net_device(xmlNode *inode, struct virt_device **vdevs) +{ + struct virt_device *vdev = NULL; + struct net_device *ndev = NULL; xmlNode *child = NULL; - memset(ndev, 0, sizeof(*ndev)); + vdev = calloc(1, sizeof(*vdev)); + if (vdev == NULL) + goto err; + + ndev = &(vdev->dev.net); ndev->type = get_attr_value(inode, "type"); if (ndev->type == NULL) @@ -216,16 +229,26 @@ static bool parse_net_device(xmlNode *in vdev->type = VIRT_DEV_NET; vdev->id = strdup(ndev->mac); - return true; + *vdevs = vdev; + + return 1; err: cleanup_net_device(ndev); - - return false; -} - -static bool parse_emu_device(xmlNode *node, struct virt_device *vdev) -{ - struct emu_device *edev = &(vdev->dev.emu); + free(vdev); + + return 0; +} + +static int parse_emu_device(xmlNode *node, struct virt_device **vdevs) +{ + struct virt_device *vdev = NULL; + struct emu_device *edev = NULL; + + vdev = calloc(1, sizeof(*vdev)); + if (vdev == NULL) + goto err; + + edev = &(vdev->dev.emu); edev->path = get_node_content(node); if (edev->path != NULL) @@ -233,16 +256,26 @@ static bool parse_emu_device(xmlNode *no vdev->type = VIRT_DEV_EMU; - return true; + *vdevs = vdev; + + return 1; err: cleanup_emu_device(edev); - - return false; -} - -static bool parse_graphics_device(xmlNode *node, struct virt_device *vdev) -{ - struct graphics_device *gdev = &(vdev->dev.graphics); + free(vdev); + + return 0; +} + +static int parse_graphics_device(xmlNode *node, struct virt_device **vdevs) +{ + struct virt_device *vdev = NULL; + struct graphics_device *gdev = NULL; + + vdev = calloc(1, sizeof(*vdev)); + if (vdev == NULL) + goto err; + + gdev = &(vdev->dev.graphics); gdev->type = get_attr_value(node, "type"); gdev->port = get_attr_value(node, "port"); @@ -252,21 +285,37 @@ static bool parse_graphics_device(xmlNod vdev->type = VIRT_DEV_GRAPHICS; - return true; + *vdevs = vdev; + + return 1; err: cleanup_graphics_device(gdev); - - return false; + free(vdev); + + return 0; +} + +static bool resize_devlist(struct virt_device **list, int newsize) +{ + struct virt_device *_list; + + _list = realloc(*list, newsize * sizeof(struct virt_device)); + if (_list == NULL) + return false; + + *list = _list; + + return true; } static int do_parse(xmlNodeSet *nsv, int type, struct virt_device **l) { int devidx; - int lstidx; + int lstidx = 0; int count = 0; struct virt_device *list = NULL; xmlNode **dev_nodes = NULL; - bool (*do_real_parse)(xmlNode *, struct virt_device *) = NULL; + int (*do_real_parse)(xmlNode *, struct virt_device **) = NULL; /* point to correct parser function according to type */ if (type == VIRT_DEV_NET) @@ -289,27 +338,32 @@ static int do_parse(xmlNodeSet *nsv, int if (count <= 0) goto out; - list = (struct virt_device *)malloc(count * sizeof(struct virt_device)); - if (list == NULL) { - count = 0; - goto out; - } - /* walk thru the array, do real parsing on each node */ - lstidx = 0; for (devidx = 0; devidx < count; devidx++) { - if (do_real_parse(dev_nodes[devidx], &list[lstidx])) - lstidx++; - } - - if (lstidx < devidx) { - list = realloc(list, lstidx * sizeof(struct virt_device)); - count = lstidx; + struct virt_device *tmp_list = NULL; + int devices = 0; + + devices = do_real_parse(dev_nodes[devidx], &tmp_list); + if (devices <= 0) + continue; + + if (!resize_devlist(&list, lstidx + devices)) { + /* Skip these devices and try again for the + * next cycle, which will probably fail, but + * what else can you do? + */ + goto end; + } + + memcpy(&list[lstidx], tmp_list, devices * sizeof(*tmp_list)); + lstidx += devices; + end: + free(tmp_list); } out: *l = list; - return count; + return lstidx; } /* Dummy function to suppress error message from libxml2 */
Dan Smith wrote:
-static bool parse_disk_device(xmlNode *dnode, struct virt_device *vdev) -{ - struct disk_device *ddev = &(vdev->dev.disk); +static int parse_disk_device(xmlNode *dnode, struct virt_device **vdevs)
Why the move from int to bool and now back to int?
+ + devices = do_real_parse(dev_nodes[devidx], &tmp_list); + if (devices <= 0) + continue;
Why is the check devices <= 0? The parse functions only return either 0 or 1. -- Kaitlin Rupert IBM Linux Technology Center karupert@us.ibm.com
Kaitlin Rupert wrote:
Dan Smith wrote:
-static bool parse_disk_device(xmlNode *dnode, struct virt_device *vdev) -{ - struct disk_device *ddev = &(vdev->dev.disk); +static int parse_disk_device(xmlNode *dnode, struct virt_device **vdevs)
Why the move from int to bool and now back to int?
+ + devices = do_real_parse(dev_nodes[devidx], &tmp_list); + if (devices <= 0) + continue;
Why is the check devices <= 0? The parse functions only return either 0 or 1.
Ah, I see - the vcpu parse function returns the count. I should read all the patches in a set before responding. =) -- Kaitlin Rupert IBM Linux Technology Center karupert@us.ibm.com
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1199827433 28800 # Node ID ebc8d09b3be6c1f1fe5b5a7ff865037298cd9f48 # Parent 3e5265023ddda5cfa24e7449c9f06bbd7dcaba5c Change behavior to get VCPU information from XML like other devices This is necessary for us to be able to show VCPU devices when a domain is offline. Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r 3e5265023ddd -r ebc8d09b3be6 libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c Tue Jan 08 13:23:53 2008 -0800 +++ b/libxkutil/device_parsing.c Tue Jan 08 13:23:53 2008 -0800 @@ -36,6 +36,7 @@ #include "../src/svpc_types.h" #define DISK_XPATH (xmlChar *)"/domain/devices/disk" +#define VCPU_XPATH (xmlChar *)"/domain/vcpu" #define NET_XPATH (xmlChar *)"/domain/devices/interface" #define EMU_XPATH (xmlChar *)"/domain/devices/emulator" #define GRAPHICS_XPATH (xmlChar *)"/domain/devices/graphics" @@ -239,6 +240,45 @@ static int parse_net_device(xmlNode *ino return 0; } +static int parse_vcpu_device(xmlNode *node, struct virt_device **vdevs) +{ + struct virt_device *list = NULL; + char *count_str; + int count; + int i; + + count_str = get_node_content(node); + if (count_str == NULL) + count = 1; /* Default to 1 VCPU if non specified */ + else if (sscanf(count_str, "%i", &count) != 1) + count = 1; /* Default to 1 VCPU if garbage */ + + free(count_str); + + list = calloc(count, sizeof(*list)); + if (list == NULL) + goto err; + + for (i = 0; i < count; i++) { + struct virt_device *vdev = &list[i]; + struct vcpu_device *cdev = &vdev->dev.vcpu; + + cdev->number = i; + + vdev->type = VIRT_DEV_VCPU; + if (asprintf(&vdev->id, "%i", i) == -1) + vdev->id = NULL; + } + + *vdevs = list; + + return count; + err: + free(list); + + return 0; +} + static int parse_emu_device(xmlNode *node, struct virt_device **vdevs) { struct virt_device *vdev = NULL; @@ -322,6 +362,8 @@ 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_VCPU) + do_real_parse = parse_vcpu_device; else if (type == VIRT_DEV_EMU) do_real_parse = parse_emu_device; else if (type == VIRT_DEV_GRAPHICS) @@ -386,6 +428,8 @@ static int parse_devices(char *xml, stru xpathstr = NET_XPATH; else if (type == VIRT_DEV_DISK) xpathstr = DISK_XPATH; + else if (type == VIRT_DEV_VCPU) + xpathstr = VCPU_XPATH; else if (type == VIRT_DEV_EMU) xpathstr = EMU_XPATH; else if (type == VIRT_DEV_GRAPHICS) @@ -446,9 +490,6 @@ struct virt_device *virt_device_dup(stru dev->dev.mem.maxsize = _dev->dev.mem.maxsize; } else if (dev->type == VIRT_DEV_VCPU) { dev->dev.vcpu.number = _dev->dev.vcpu.number; - dev->dev.vcpu.state = _dev->dev.vcpu.state; - dev->dev.vcpu.cpuTime = _dev->dev.vcpu.cpuTime; - dev->dev.vcpu.cpu = _dev->dev.vcpu.cpu; } else if (dev->type == VIRT_DEV_EMU) { DUP_FIELD(dev, _dev, dev.emu.path); } else if (dev->type == VIRT_DEV_GRAPHICS) { @@ -573,42 +614,17 @@ int get_mem_devices(virDomainPtr dom, st int get_vcpu_devices(virDomainPtr dom, struct virt_device **list) { - int i, rc, ret, num_filled, num_vcpus; - virDomainInfo dom_info; - virVcpuInfoPtr vcpu_info = NULL; - struct virt_device *ret_list = NULL; - - rc = virDomainGetInfo(dom, &dom_info); - if (rc == -1) { - ret = -1; - goto out1; - } - - num_vcpus = dom_info.nrVirtCpu; - vcpu_info = calloc(num_vcpus, sizeof(virVcpuInfo)); - num_filled = virDomainGetVcpus(dom, vcpu_info, num_vcpus, NULL, 0); - if (num_vcpus != num_filled) { - ret = -1; - goto out2; - } - - ret_list = calloc(num_vcpus, sizeof(struct virt_device)); - for (i = 0; i < num_vcpus; i++) { - ret_list[i].type = VIRT_DEV_VCPU; - ret_list[i].dev.vcpu = vcpu_info[i]; - if (asprintf(&ret_list[i].id, "%d", - vcpu_info[i].number) == -1) { - ret = -1; - free(ret_list); - goto out2; - } - } - - ret = num_vcpus; - *list = ret_list; - out2: - free(vcpu_info); - out1: + char *xml; + int ret; + + xml = virDomainGetXMLDesc(dom, 0); + if (xml == NULL) + return 0; + + ret = parse_devices(xml, list, VIRT_DEV_VCPU); + + free(xml); + return ret; } diff -r 3e5265023ddd -r ebc8d09b3be6 libxkutil/device_parsing.h --- a/libxkutil/device_parsing.h Tue Jan 08 13:23:53 2008 -0800 +++ b/libxkutil/device_parsing.h Tue Jan 08 13:23:53 2008 -0800 @@ -49,6 +49,10 @@ struct mem_device { uint64_t maxsize; }; +struct vcpu_device { + uint32_t number; +}; + struct emu_device { char *path; }; @@ -72,7 +76,7 @@ struct virt_device { struct disk_device disk; struct net_device net; struct mem_device mem; - struct _virVcpuInfo vcpu; + struct vcpu_device vcpu; struct emu_device emu; struct graphics_device graphics; } dev; diff -r 3e5265023ddd -r ebc8d09b3be6 libxkutil/xml_parse_test.c --- a/libxkutil/xml_parse_test.c Tue Jan 08 13:23:53 2008 -0800 +++ b/libxkutil/xml_parse_test.c Tue Jan 08 13:23:53 2008 -0800 @@ -14,6 +14,11 @@ static void print_u64(FILE *d, const cha static void print_u64(FILE *d, const char *name, uint64_t val) { fprintf(d, "%-15s: %" PRIu64 "\n", name, val); +} + +static void print_u32(FILE *d, const char *name, uint32_t val) +{ + fprintf(d, "%-15s: %" PRIu32 "\n", name, val); } static void print_os(struct domain *dom, @@ -84,7 +89,7 @@ static void print_dev_vcpu(struct virt_d static void print_dev_vcpu(struct virt_device *dev, FILE *d) { - print_value(d, "Virtual CPU", "Present"); + print_u32(d, "Virtual CPU", dev->dev.vcpu.number); } static void print_dev_emu(struct virt_device *dev, diff -r 3e5265023ddd -r ebc8d09b3be6 libxkutil/xmlgen.c --- a/libxkutil/xmlgen.c Tue Jan 08 13:23:53 2008 -0800 +++ b/libxkutil/xmlgen.c Tue Jan 08 13:23:53 2008 -0800 @@ -163,7 +163,7 @@ static char *net_to_xml(struct net_devic return xml; } -static char *proc_to_xml(struct _virVcpuInfo *proc) +static char *proc_to_xml(struct vcpu_device *proc) { return strdup(""); } diff -r 3e5265023ddd -r ebc8d09b3be6 src/Virt_Device.c --- a/src/Virt_Device.c Tue Jan 08 13:23:53 2008 -0800 +++ b/src/Virt_Device.c Tue Jan 08 13:23:53 2008 -0800 @@ -187,7 +187,7 @@ static CMPIInstance *mem_instance(const } static CMPIInstance *vcpu_instance(const CMPIBroker *broker, - struct _virVcpuInfo *dev, + struct vcpu_device *dev, const virDomainPtr dom, const char *ns) {
Dan Smith wrote:
Currently, we cannot present information and instances for VCPUs when a domain is powered off, because virDomainGetInfo() does not work unless a domain is running.
This set ultimately changes the vcpu bits in device_parsing to, that's right, actually *parse* the VCPU information from the XML like all the other devices, which gives us VCPU information for offline domains.
To get there, I had to make the core of device_parsing a little more dyanmic so that the parsing modules could return mulitple devices for a given XML cycle, which vcpu needs.
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
This is an excellent patch set ! I even wasn't able to see an instance of KVM_Processor with a running domain (because libvirt wasn't supporting it), but now I can ! This patch set gets a BIG +1 ! -- Regards Heidi Eckhart Software Engineer IBM Linux Technology Center - Open Hypervisor
Dan Smith wrote:
Currently, we cannot present information and instances for VCPUs when a domain is powered off, because virDomainGetInfo() does not work unless a domain is running.
This set ultimately changes the vcpu bits in device_parsing to, that's right, actually *parse* the VCPU information from the XML like all the other devices, which gives us VCPU information for offline domains.
To get there, I had to make the core of device_parsing a little more dyanmic so that the parsing modules could return mulitple devices for a given XML cycle, which vcpu needs.
This tested out fine on my system. No complaints. =) +1 -- Kaitlin Rupert IBM Linux Technology Center karupert@us.ibm.com
participants (4)
-
Dan Smith -
Heidi Eckhart -
Jay Gagnon -
Kaitlin Rupert