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@xxxxxxxxxx> > Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx> > --- > 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@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list > ACK with the nitpick fixed. Erik -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list