[libvirt] [PATCH v2 4/4] conf: Use correct type for balloon stats period
Erik Skultety
eskultet at redhat.com
Mon Mar 16 13:10:35 UTC 2015
On 03/13/2015 05:17 PM, Martin Kletzander wrote:
> We're parsing memballoon status period as unsigned int, but when we're
> trying to set it, both we and qemu use signed int. That means large
> values will get wrapped around to negative one resulting in error.
> Basically the same problem as commit e3a7b874 was dealing with when
> updating live domain.
>
> QEMU changed the accepted value to int64 in commit 1f9296b5, but even
> values as INT_MAX don't make sense since the value passed means seconds.
> Hence adding capability flag for this change isn't worth it.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140958
>
> Signed-off-by: Luyao Huang <lhuang at redhat.com>
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
> docs/formatdomain.html.in | 2 ++
> src/conf/domain_conf.c | 9 +++++++--
> src/conf/domain_conf.h | 2 +-
> src/qemu/qemu_process.c | 2 +-
> 4 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 40e2b29..7a11cc7 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -5630,6 +5630,8 @@ qemu-kvm -net nic,model=? /dev/null
> only be made to the active guest.
> If the QEMU driver is not at the right
> revision, the attempt to set the period will fail.
> + Large values might be ignored, but this only affects
> + non-sensical numbers (i.e. many years).
> <span class='since'>Since 1.1.1, requires QEMU 1.5</span>
> </p>
> </dd>
Just a nitpick, I'd probably avoid word construction non-sensical in our
docs (it's not even correct --> nonsensical) and simplify this to "Large
values (i.e. many years) might be ignored."
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index e010040..b3d9999 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -10432,6 +10432,7 @@ virDomainMemballoonDefParseXML(xmlNodePtr node,
> char *model;
> virDomainMemballoonDefPtr def;
> xmlNodePtr save = ctxt->node;
> + unsigned int period = 0;
>
> if (VIR_ALLOC(def) < 0)
> return NULL;
> @@ -10450,12 +10451,16 @@ virDomainMemballoonDefParseXML(xmlNodePtr node,
> }
>
> ctxt->node = node;
> - if (virXPathUInt("string(./stats/@period)", ctxt, &def->period) < -1) {
> + if (virXPathUInt("string(./stats/@period)", ctxt, &period) < -1) {
> virReportError(VIR_ERR_XML_ERROR, "%s",
> _("invalid statistics collection period"));
> goto error;
> }
>
> + def->period = period;
> + if (def->period < 0)
> + def->period = 0;
> +
> if (def->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE)
> VIR_DEBUG("Ignoring device address for none model Memballoon");
> else if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
> @@ -18823,7 +18828,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
> virBufferAdjustIndent(&childrenBuf, indent + 2);
>
> if (def->period)
> - virBufferAsprintf(&childrenBuf, "<stats period='%u'/>\n", def->period);
> + virBufferAsprintf(&childrenBuf, "<stats period='%i'/>\n", def->period);
>
> if (virDomainDeviceInfoNeedsFormat(&def->info, flags) &&
> virDomainDeviceInfoFormat(&childrenBuf, &def->info, flags) < 0) {
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index ea463cb..ee0f5fd 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1556,7 +1556,7 @@ enum {
> struct _virDomainMemballoonDef {
> int model;
> virDomainDeviceInfo info;
> - unsigned int period; /* seconds between collections */
> + int period; /* seconds between collections */
> };
>
> struct _virDomainNVRAMDef {
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index d1f089d..0f357d5 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4390,7 +4390,7 @@ int qemuProcessStart(virConnectPtr conn,
> virCommandPtr cmd = NULL;
> struct qemuProcessHookData hookData;
> unsigned long cur_balloon;
> - unsigned int period = 0;
> + int period = 0;
> size_t i;
> bool rawio_set = false;
> char *nodeset = NULL;
> --
> 2.3.2
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
ACK with the nitpick fixed.
Erik
More information about the libvir-list
mailing list