On 03/31/2012 01:19 AM, Osier Yang wrote: > On 03/30/2012 11:56 PM, Eric Blake wrote: >> Commit 1b1402b introduced a regression. Since older libvirt versions >> would silently round memory up (until the previous patch), but populated >> current memory based on querying the guest, it was possible to have >> current> maximum by the amount of the rounding. Accept this fuzz >> factor, and silently clamp current down to maximum in that case, >> rather than failing to reparse XML for an existing VM. >> >> * src/conf/domain_conf.c (virDomainDefParseXML): Don't reject >> existing XML. >> Based on a report by Zhou Peng. >> --- >> src/conf/domain_conf.c | 19 +++++++++++++++---- >> src/qemu/qemu_monitor_json.c | 2 +- >> 2 files changed, 16 insertions(+), 5 deletions(-) >> >> - goto error; >> + /* Older libvirt could get into this situation due to >> + * rounding; if the discrepancy is less than 1MiB, we silently >> + * round down, otherwise we flag the issue. */ >> + if (VIR_DIV_UP(def->mem.cur_balloon, 1024)> >> + VIR_DIV_UP(def->mem.max_balloon, 1024)) { >> + virDomainReportError(VIR_ERR_XML_ERROR, >> + _("current memory '%lluk' exceeds " >> + "maximum '%lluk'"), >> + def->mem.cur_balloon, >> def->mem.max_balloon); >> + goto error; >> + } else { >> + VIR_DEBUG("Truncating current %lluk to maximum %lluk", >> + def->mem.cur_balloon, def->mem.max_balloon); >> + def->mem.cur_balloon = def->mem.max_balloon; >> + } > > But this needs the documentation IMHO, it actually changes > the behaviours. I don't see that this needs to be documented in user documentation, only in the commit message. In 0.9.10, there was no checking done at all. If libvirt output cur > max (possible only up to the fuzz of a megabyte boundary), then this still made no difference: the guest was running with max at the megabyte boundary, so the current fit within the actual max of the guest. If you hand-crafted xml with cur > max and outside the fuzz, you couldn't run anyways, because qemu would complain that cur was out of range. With these two patches, libvirt will now adjust max to the actual megabyte boundary, at which point cur is no longer > max. And when importing an older xml where cur > max, this silently clamps cur down to max, but then libvirt rounds it back up again when passing it to qemu, so you still end up with the same max at the megabyte boundary and cur rounded up to match max. Meanwhile, if the user hand-writes xml with too large of cur, we error out up front rather than letting qemu complain, but it doesn't change the fact that it gets rejected at some point in the chain. That is, I don't see how this fuzz is user visible, so I don't think it needs user visible documentation, only more details in the commit message. v2 with improved commit message coming up. -- Eric Blake eblake@xxxxxxxxxx +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list