From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54537) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWM0D-0003nr-F6 for qemu-devel@nongnu.org; Tue, 23 Sep 2014 05:03:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XWM07-0002en-5w for qemu-devel@nongnu.org; Tue, 23 Sep 2014 05:03:17 -0400 Date: Tue, 23 Sep 2014 11:01:37 +0200 From: Igor Mammedov Message-ID: <20140923110137.419e8121@nial.usersys.redhat.com> In-Reply-To: <20140922111728.GB14882@redhat.com> References: <1410863956-11056-1-git-send-email-zhang.zhanghailiang@huawei.com> <1410863956-11056-2-git-send-email-zhang.zhanghailiang@huawei.com> <20140919143746.68f082b9@nial.usersys.redhat.com> <20140922111728.GB14882@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] pc-dimm: No numa option shouldn't break hotplug memory feature List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: zhanghailiang , hutao@cn.fujitsu.com, qemu-stable@nongnu.org, luonengjun@huawei.com, qemu-devel@nongnu.org, peter.huangpeng@huawei.com, pbonzini@redhat.com, gaowanlong@cn.fujitsu.com On Mon, 22 Sep 2014 14:17:28 +0300 "Michael S. Tsirkin" wrote: > On Fri, Sep 19, 2014 at 02:37:46PM +0200, Igor Mammedov wrote: > > On Tue, 16 Sep 2014 18:39:15 +0800 > > zhanghailiang wrote: > > > > > If we do not configure numa option, memory hotplug should work as well. > > > It should not depend on numa option. > > > > > > Steps to reproduce: > > > (1) Start VM: qemu-kvm -m 1024,slots=4,maxmem=8G > > > (2) Hotplug memory > > > It will fail and reports: > > > "'DIMM property node has value 0' which exceeds the number of numa nodes: 0" > > > > > > Signed-off-by: zhanghailiang > > > --- > > > hw/mem/pc-dimm.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c > > > index 5bfc5b7..a800ea7 100644 > > > --- a/hw/mem/pc-dimm.c > > > +++ b/hw/mem/pc-dimm.c > > > @@ -252,7 +252,7 @@ static void pc_dimm_realize(DeviceState *dev, Error **errp) > > > error_setg(errp, "'" PC_DIMM_MEMDEV_PROP "' property is not set"); > > > return; > > > } > > > - if (dimm->node >= nb_numa_nodes) { > > > + if ((nb_numa_nodes > 0) && (dimm->node >= nb_numa_nodes)) { > > > error_setg(errp, "'DIMM property " PC_DIMM_NODE_PROP " has value %" > > > PRIu32 "' which exceeds the number of numa nodes: %d", > > > dimm->node, nb_numa_nodes); > > > > Reviewed-By: Igor Mammedov > > > I read: > > Hmm, I have just tested this, and Yes, it didn't work for Windows guest. > > Thanks for your kind reminder.;) > > So should I expect v2 which works with windows? Hotplug wouldn't work with Windows without -numa (it's Windows limitation) and more importantly pc-dimm shouldn't be limited only to NUMA configs which this patch fixes. This patch is fine and should go to stable as well. On top of this we could add automatic NUMA node creation when memory hotplug is enabled if this Windows workaround is acceptable.