[libvirt] [PATCHv2 2/2] conf: allow fuzz in XML with cur balloon > max

[Eric Blake]

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 at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120331/8a925234/attachment-0001.sig>