[Libvirt-cim] [PATCH V3] Add dumpCore tag support to memory
John Ferlan
jferlan at redhat.com
Fri Aug 23 19:33:34 UTC 2013
On 08/19/2013 05:11 AM, Xu Wang wrote:
> dumpCore tag in the <memory> is not supported by libvirt-cim and
> it will be dropped during updating any element in the xml definition
> of a domain. This patch keep the tag all the time.
>
I had put off reviewing/testing because I wasn't sure what the outcome
of the initial responses on the list were going to be...
Anyway, couple of thoughts
1. Is there test for this?
That is - is there a way to ensure the right things are done based on
the setting of this? Is there a specific cimtest that could be added?
How would one know this setting has taken ahold?
2. Did you run cimtest with this applied? In an environment without
dumpCore?
Mine failed rather spectacularly today with just this applied to top of
the tree. In fact "ComputerSystem" and "01_enum.py" returns:
ComputerSystem - 01_enum.py: FAIL
ERROR - Provider does not report system `f18', but virsh does
ERROR - Provider does not report system `if18lcl', but virsh does
ERROR - Provider does not report system `if18net', but virsh does
ERROR - Provider does not report system `if18nopool', but virsh does
ERROR - Provider does not report system `rh64', but virsh does
ERROR - Provider does not report system `rh70-alpha3', but virsh does
CIM_ERR_FAILED: Lost connection with cimprovagt "libvirt-cim".
I did a bit of debugging though and believe I know the cause (see below).
> Signed-off-by: Xu Wang <gesaint at linux.vnet.ibm.com>
> ---
> libxkutil/device_parsing.c | 28 +++++++++++++++++++++++++++-
> libxkutil/device_parsing.h | 3 +++
> libxkutil/xmlgen.c | 9 +++++++++
> 3 files changed, 39 insertions(+), 1 deletions(-)
>
> diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c
> index 7900e06..96db532 100644
> --- a/libxkutil/device_parsing.c
> +++ b/libxkutil/device_parsing.c
> @@ -605,8 +605,17 @@ static int parse_mem_device(xmlNode *node, struct virt_device **vdevs)
>
> if (XSTREQ(node->name, "currentMemory"))
> sscanf(content, "%" PRIu64, &mdev->size);
> - else if (XSTREQ(node->name, "memory"))
> + else if (XSTREQ(node->name, "memory")) {
> sscanf(content, "%" PRIu64, &mdev->maxsize);
> + content = get_attr_value(node, "dumpCore");
if (content == NULL), then this fails rather spectacularly
I squashed the following and pushed (after running cimtest successfully
and checking for coverity issues)
diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c
index fc4338b..542e4e9 100644
--- a/libxkutil/device_parsing.c
+++ b/libxkutil/device_parsing.c
@@ -609,9 +609,9 @@ static int parse_mem_device(xmlNode *node, struct
virt_devic
else if (XSTREQ(node->name, "memory")) {
sscanf(content, "%" PRIu64, &mdev->maxsize);
content = get_attr_value(node, "dumpCore");
- if (XSTREQ(content, "on")) {
+ if (content && XSTREQ(content, "on")) {
mdev->dumpCore = MEM_DUMP_CORE_ON;
- } else if (XSTREQ(content, "off")) {
+ } else if (content && XSTREQ(content, "off")) {
mdev->dumpCore = MEM_DUMP_CORE_OFF;
} else {
mdev->dumpCore = MEM_DUMP_CORE_NOT_SET;
John
> + if (XSTREQ(content, "on")) {
> + mdev->dumpCore = MEM_DUMP_CORE_ON;
> + } else if (XSTREQ(content, "off")) {
> + mdev->dumpCore = MEM_DUMP_CORE_OFF;
> + } else {
> + mdev->dumpCore = MEM_DUMP_CORE_NOT_SET;
> + }
> + }
>
> free(content);
>
> @@ -968,6 +977,7 @@ static int _get_mem_device(const char *xml, struct virt_device **list)
> struct virt_device *mdevs = NULL;
> struct virt_device *mdev = NULL;
> int ret;
> + bool mem_dump_core_set = false;
>
> ret = parse_devices(xml, &mdevs, CIM_RES_TYPE_MEM);
> if (ret <= 0)
> @@ -987,10 +997,26 @@ static int _get_mem_device(const char *xml, struct virt_device **list)
> mdevs[1].dev.mem.size);
> mdev->dev.mem.maxsize = MAX(mdevs[0].dev.mem.maxsize,
> mdevs[1].dev.mem.maxsize);
> + /* libvirt dumpCore tag always belong to memory xml node, but
> + * here we may have two mdev for memory node and currentMemory
> + * node. So pick up one value.
> + */
> + if (mdevs[0].dev.mem.dumpCore != MEM_DUMP_CORE_NOT_SET) {
> + mdev->dev.mem.dumpCore = mdevs[0].dev.mem.dumpCore;
> + mem_dump_core_set = true;
> + } else if (mdevs[1].dev.mem.dumpCore !=
> + MEM_DUMP_CORE_NOT_SET) {
> + if (mem_dump_core_set) {
> + CU_DEBUG("WARN: libvirt set memory core dump in"
> + "two nodes!");
> + }
> + mdev->dev.mem.dumpCore = mdevs[1].dev.mem.dumpCore;
> + }
> } 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->dev.mem.dumpCore = mdevs[0].dev.mem.dumpCore;
> }
>
> mdev->type = CIM_RES_TYPE_MEM;
> diff --git a/libxkutil/device_parsing.h b/libxkutil/device_parsing.h
> index 2b6d3d1..979b792 100644
> --- a/libxkutil/device_parsing.h
> +++ b/libxkutil/device_parsing.h
> @@ -75,6 +75,9 @@ struct net_device {
> struct mem_device {
> uint64_t size;
> uint64_t maxsize;
> + enum { MEM_DUMP_CORE_NOT_SET,
> + MEM_DUMP_CORE_ON,
> + MEM_DUMP_CORE_OFF } dumpCore;
> };
>
> struct vcpu_device {
> diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c
> index 4287d42..30e9a5e 100644
> --- a/libxkutil/xmlgen.c
> +++ b/libxkutil/xmlgen.c
> @@ -498,6 +498,15 @@ static const char *mem_xml(xmlNodePtr root, struct domain *dominfo)
> BAD_CAST string);
>
> free(string);
> +
> + if (tmp == NULL)
> + return XML_ERROR;
> + if (mem->dumpCore == MEM_DUMP_CORE_ON) {
> + xmlNewProp(tmp, BAD_CAST "dumpCore", BAD_CAST "on");
> + } else if (mem->dumpCore == MEM_DUMP_CORE_OFF) {
> + xmlNewProp(tmp, BAD_CAST "dumpCore", BAD_CAST "off");
> + }
> +
> out:
> if (tmp == NULL)
> return XML_ERROR;
>
More information about the Libvirt-cim
mailing list