From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37492) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e2feO-0007dK-Dm for qemu-devel@nongnu.org; Thu, 12 Oct 2017 11:43:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1e2feJ-0004SX-Cj for qemu-devel@nongnu.org; Thu, 12 Oct 2017 11:43:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56330) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1e2feJ-0004SC-2p for qemu-devel@nongnu.org; Thu, 12 Oct 2017 11:43:51 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CD47FC0587DE for ; Thu, 12 Oct 2017 15:43:49 +0000 (UTC) Date: Thu, 12 Oct 2017 12:43:46 -0300 From: Eduardo Habkost Message-ID: <20171012154346.GZ3246@localhost.localdomain> References: <1507801198-98182-1-git-send-email-imammedo@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1507801198-98182-1-git-send-email-imammedo@redhat.com> Subject: Re: [Qemu-devel] [PATCH] numa: fixup parsed NumaNodeOptions earlier List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: qemu-devel@nongnu.org, Markus Armbruster On Thu, Oct 12, 2017 at 11:39:58AM +0200, Igor Mammedov wrote: > numa 'mem' option with suffix or without one is possible > only on CLI/HMP. Instead of fixing up special suffix less > CLI case deep in parse_numa_node() do it earlier right > after option is parsed into NumaNodeOptions with OptVisistor > so that the rest of the code would use valid values in > NumaNodeOptions and won't have to reparse QemuOpts. > > It will help to isolate CLI/HMP parts in parse_numa() and > split out parsed NumaNodeOptions processing into separate > function that could be reused by QMP handler where we have > only NumaNodeOptions and don't need any fixups. > > While at it reuse qemu_strtosz_MiB() instead of manually > checking for suffixes. > > Signed-off-by: Igor Mammedov > --- > numa.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/numa.c b/numa.c > index 17ea514..fb4c35a 100644 > --- a/numa.c > +++ b/numa.c > @@ -38,6 +38,7 @@ > #include "hw/mem/pc-dimm.h" > #include "qemu/option.h" > #include "qemu/config-file.h" > +#include "qemu/cutils.h" > > QemuOptsList qemu_numa_opts = { > .name = "numa", > @@ -142,7 +143,7 @@ uint32_t numa_get_node(ram_addr_t addr, Error **errp) > } > > static void parse_numa_node(MachineState *ms, NumaNodeOptions *node, > - QemuOpts *opts, Error **errp) > + Error **errp) > { > uint16_t nodenr; > uint16List *cpus = NULL; > @@ -199,13 +200,7 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node, > } > > if (node->has_mem) { > - uint64_t mem_size = node->mem; > - const char *mem_str = qemu_opt_get(opts, "mem"); > - /* Fix up legacy suffix-less format */ > - if (g_ascii_isdigit(mem_str[strlen(mem_str) - 1])) { > - mem_size <<= 20; > - } > - numa_info[nodenr].node_mem = mem_size; > + numa_info[nodenr].node_mem = node->mem; > } > if (node->has_memdev) { > Object *o; > @@ -290,9 +285,15 @@ int parse_numa(void *opaque, QemuOpts *opts, Error **errp) > goto end; > } > > + /* Fix up legacy suffix-less format */ > + if ((object->type == NUMA_OPTIONS_TYPE_NODE) && object->u.node.has_mem) { > + const char *mem_str = qemu_opt_get(opts, "mem"); > + qemu_strtosz_MiB(mem_str, NULL, &object->u.node.mem); > + } > + I would like to have a way to represent this behavior in the QAPI schema somehow. Markus, do you see a solution for that? But this patch is an improvement, anyway, so: Reviewed-by: Eduardo Habkost > switch (object->type) { > case NUMA_OPTIONS_TYPE_NODE: > - parse_numa_node(ms, &object->u.node, opts, &err); > + parse_numa_node(ms, &object->u.node, &err); > if (err) { > goto end; > } > -- > 2.7.4 > -- Eduardo