[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] vl.c: Fix error messages when parsing maxme
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] vl.c: Fix error messages when parsing maxmem parameters |
Date: |
Tue, 27 Jan 2015 11:15:41 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
On 01/26/2015 08:31 AM, Peter Krempa wrote:
> Produce more human readable error messages and fix few spelling
> mistakes.
>
> Also remove a redundant check for the max memory size.
>
> Signed-off-by: Peter Krempa <address@hidden>
> ---
> vl.c | 22 +++++++---------------
> 1 file changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 983259b..cdc920c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2694,29 +2694,21 @@ static void set_memory_options(uint64_t *ram_slots,
> ram_addr_t *maxram_size)
> uint64_t slots;
>
> sz = qemu_opt_get_size(opts, "maxmem", 0);
> - if (sz < ram_size) {
> - error_report("invalid -m option value: maxmem "
> - "(0x%" PRIx64 ") <= initial memory (0x"
> - RAM_ADDR_FMT ")", sz, ram_size);
> + if (sz <= ram_size) {
Why are we changing from '<' to '<='? I think the error was in the
message, not in the code, and that setting max == size should be
allowed. [1]
> + error_report("invalid value of -m option maxmem: "
> + "maximum memory size (0x%" PRIx64 ") must be
> greater "
> + "than initial memory size (0x" RAM_ADDR_FMT ")",
Why two spaces? If I'm correct that we want '<' in the condition, then
the wording 'must be at least the initial memory size' would be better.
> + sz, ram_size);
> exit(EXIT_FAILURE);
> }
>
> slots = qemu_opt_get_number(opts, "slots", 0);
> if ((sz > ram_size) && !slots) {
> - error_report("invalid -m option value: maxmem "
> - "(0x%" PRIx64 ") more than initial memory (0x"
> - RAM_ADDR_FMT ") but no hotplug slots where "
> - "specified", sz, ram_size);
> + error_report("invalid value of -m option: maxmem was specified, "
> + "but no hotplug slots were specified");
> exit(EXIT_FAILURE);
> }
>
> - if ((sz <= ram_size) && slots) {
> - error_report("invalid -m option value: %"
> - PRIu64 " hotplug slots where specified but "
> - "maxmem (0x%" PRIx64 ") <= initial memory (0x"
> - RAM_ADDR_FMT ")", slots, sz, ram_size);
> - exit(EXIT_FAILURE);
> - }
Okay, I see. This is dead if condition [1] is changed to '<=', but
still reachable (sz == ram_size) if condition [1] is left at '<'. Maybe
better logic would be:
if (sz < ram_size) {
max cannot be less than memory
}
if (sz > ram_size) {
if (!slots) {
max cannot be larger than size without hotplug slots
}
} else if (slots) {
max must be larger than size to support hotplug slots
}
to allow max==ram_size when slots is not present.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature