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

[Zhou Peng]

Thanks for your patch serials.
I think they fix the true bug.

But I have a little doubt on the fuzz allowance, pls have a see
comment in line below.

On Fri, Mar 30, 2012 at 11:56 PM, Eric Blake <eblake at redhat.com> 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(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 4caef6f..7c95920 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7776,10 +7776,21 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>         goto error;
>
>     if (def->mem.cur_balloon > def->mem.max_balloon) {
> -        virDomainReportError(VIR_ERR_XML_ERROR,
> -                             _("current memory '%lluk' exceeds maximum '%lluk'"),
> -                             def->mem.cur_balloon, def->mem.max_balloon);
> -        goto error;
> +        /* Older libvirt could get into this situation due to
Do you mean to allow the scene:
* The checkpoint( thought invalid by old libvirt) produced by older libvirt
can be thought valid if restored by the patched newer libvirt?
But it will also allow a typo xml.
* Another scene may be live migration between old libvirt
and newer libvirt (pls correct me if err)

If so, I think these scene should be documented or commented here or
in the commit msg explicitly, otherwise this piece of code may confuse many
readers, because I don't think cur_balloon can exceed max_balloon
again after patched
(so if exceed must be typo xml).

> +         * 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;
> +        }
>     } else if (def->mem.cur_balloon == 0) {
>         def->mem.cur_balloon = def->mem.max_balloon;
>     }
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 7093e1d..6d66587 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -2022,7 +2022,7 @@ int qemuMonitorJSONSetBalloon(qemuMonitorPtr mon,
>  {
>     int ret;
>     virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("balloon",
> -                                                     "U:value", ((unsigned long long)newmem)*1024,
> +                                                     "U:value", newmem*1024ULL,
>                                                      NULL);
>     virJSONValuePtr reply = NULL;
>     if (!cmd)
> --
> 1.7.1
>



--
Zhou Peng