On 02.07.2013 15:39, John Ferlan wrote:
> Add a period in seconds to allow/enable statistics gathering from the
> Balloon driver for 'virsh dommemstat <domain>'.
> ---
> docs/schemas/domaincommon.rng | 7 +++++++
> src/conf/domain_conf.c | 27 +++++++++++++++++++++++----
> src/conf/domain_conf.h | 1 +
> 3 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index cf82878..53e707c 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -2915,6 +2915,13 @@
> <optional>
> <ref name="address"/>
> </optional>
> + <optional>
> + <element name="stats">
> + <attribute name="period">
> + <ref name="positiveInteger"/>
> + </attribute>
> + </element>
> + </optional>
> </element>
> </define>
> <define name="parallel">
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 011de71..cd7dbee 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -8571,10 +8571,12 @@ error:
>
> static virDomainMemballoonDefPtr
> virDomainMemballoonDefParseXML(const xmlNodePtr node,
> + xmlXPathContextPtr ctxt,
> unsigned int flags)
> {
> char *model;
> virDomainMemballoonDefPtr def;
> + xmlNodePtr save = ctxt->node;
>
> if (VIR_ALLOC(def) < 0) {
> virReportOOMError();
> @@ -8587,18 +8589,24 @@ virDomainMemballoonDefParseXML(const xmlNodePtr node,
> _("balloon memory must contain model name"));
> goto error;
> }
> +
> if ((def->model = virDomainMemballoonModelTypeFromString(model)) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("unknown memory balloon model '%s'"), model);
> goto error;
> }
>
> + ctxt->node = node;
> + if (virXPathInt("string(./stats/@period)", ctxt, &def->period) < 0)
> + def->period = 0;
This is silently ignoring invalid integer value in @period. Moreover,
it's merging two different cases into one: invalid integer and @period
not present. After all, setting def->period to zero is done by
VIR_ALLOC. So I think we must distinguish if virXPathInt returned -1
(@period not present in XML) or -2(@period is not valid integer). And we
must not forget to check if user hasn't entered period < 0.
> +
> if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
> goto error;
>
> cleanup:
> VIR_FREE(model);
>
> + ctxt->node = save;
> return def;
>
> error:
> @@ -11911,7 +11919,7 @@ virDomainDefParseXML(xmlDocPtr xml,
> }
> if (n > 0) {
> virDomainMemballoonDefPtr memballoon =
> - virDomainMemballoonDefParseXML(nodes[0], flags);
> + virDomainMemballoonDefParseXML(nodes[0], ctxt, flags);
> if (!memballoon)
> goto error;
>
> @@ -15080,6 +15088,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
> unsigned int flags)
> {
> const char *model = virDomainMemballoonModelTypeToString(def->model);
> + bool noopts = true;
>
> if (!model) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -15093,11 +15102,21 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
> virBufferAddLit(buf, ">\n");
> if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
> return -1;
> - virBufferAddLit(buf, " </memballoon>\n");
> - } else {
> - virBufferAddLit(buf, "/>\n");
> + noopts = false;
> }
>
> + if (def->period) {
> + if (noopts)
> + virBufferAddLit(buf, ">\n");
> + virBufferAsprintf(buf, " <stats period='%d'/>\n", def->period);
> + noopts = false;
> + }
> +
> + if (noopts)
> + virBufferAddLit(buf, "/>\n");
> + else
> + virBufferAddLit(buf, " </memballoon>\n");
> +
> return 0;
> }
>
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 3817e37..405903f 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1527,6 +1527,7 @@ enum {
> struct _virDomainMemballoonDef {
> int model;
> virDomainDeviceInfo info;
> + int period; /* seconds between collections */
> };
>
> struct _virDomainNVRAMDef {
>
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list