From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49685) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XVuRL-0001ZI-6m for qemu-devel@nongnu.org; Sun, 21 Sep 2014 23:37:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XVuRG-0007GN-7y for qemu-devel@nongnu.org; Sun, 21 Sep 2014 23:37:27 -0400 Received: from szxga01-in.huawei.com ([119.145.14.64]:19425) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XVuRF-0007Fh-EL for qemu-devel@nongnu.org; Sun, 21 Sep 2014 23:37:22 -0400 Message-ID: <541F9943.4040108@huawei.com> Date: Mon, 22 Sep 2014 11:36:35 +0800 From: zhanghailiang MIME-Version: 1.0 References: <1410784178-9012-1-git-send-email-zhang.zhanghailiang@huawei.com> <20140919150948.7ae4a8a1@nial.usersys.redhat.com> In-Reply-To: <20140919150948.7ae4a8a1@nial.usersys.redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] virtio-balloon: Fix ballooning not working correctly when hotplug memory List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: peter.maydell@linaro.org, mst@redhat.com, qemu-devel@nongnu.org, luonengjun@huawei.com, peter.huangpeng@huawei.com, lcapitulino@redhat.com Hi Igor, Thanks for your reviewing... > On Mon, 15 Sep 2014 20:29:38 +0800 > zhanghailiang wrote: > >> When do memory balloon, it references the ram_size as the real ram size of VM, >> But here ram_size is not include the hotplugged memory, and the result will >> be confused. >> >> Steps to reproduce: >> (1)Start VM: qemu -m size=1024,slots=4,maxmem=8G >> (2)In VM: #free -m : 1024M >> (3)qmp balloon 512M >> (4)In VM: #free -m : 512M >> (5)hotplug pc-dimm 1G >> (6)In VM: #free -m : 1512M >> (7)qmp balloon 256M >> (8)In VM: #free -m :1256M >> >> Here we add a new global variable 'vm_ram_size', it will stat > "qmp balloon" is not performance critical code and instead of a global > variable, size could be calculated each time by enumerating present memory devices. > Good idea, in this way, we don't have to treat hotplug/unplug memory specially. Will fix like that. Thanks. > >> the VM's real ram size which include configured ram and hotplugged ram. >> virtio-balloon will reference this parameter. > I know it's not supported yet but what will happen with balloonig > if dimm device is removed without telling about it to balloon first? > Then, calculating the ram size will also be wrong and the result of balloon will be confused too. 'size be calculated each time' you have suggested above will solve this problem;) > I'm not sure if balloon and native memory hotplug should be integrated. > Native memory hotplug was intended as a replacement for ballooning > without its drawbacks albeit guest OS memory unplug support is in > its infancy stage yet. > IMHO memory hotplug/unplug can not replace the balloon completely, Memory hotplug/unplug way may be a little stiff. Example: VM's memroy :1G hotplugged one pc-dimm mem :1G Now we want to limit the VM's ram size of guest OS to 1512M. I don't know if we can unplugging a pc-dimm which has a different size to its original size (hot add stage)? Here is 512M. If it supports, what about limit ram size to 1100M?;) There seems to be a minimal size limit for memory hotplug...(If i'm wrong, please let me know;)) Maybe in actual usage scenario we will use them together. So i think we'd better to fix it. Thanks, zhanghailiang >> >> Signed-off-by: zhanghailiang >> --- >> hw/i386/pc.c | 1 + >> hw/virtio/virtio-balloon.c | 10 +++++----- >> include/exec/cpu-common.h | 1 + >> vl.c | 3 +++ >> 4 files changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >> index b6c9b61..817810b 100644 >> --- a/hw/i386/pc.c >> +++ b/hw/i386/pc.c >> @@ -1606,6 +1606,7 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev, >> memory_region_add_subregion(&pcms->hotplug_memory, >> addr - pcms->hotplug_memory_base, mr); >> vmstate_register_ram(mr, dev); >> + vm_ram_size += memory_region_size(mr); >> >> hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev); >> hhc->plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err); >> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c >> index 2c30b3d..205e1fe 100644 >> --- a/hw/virtio/virtio-balloon.c >> +++ b/hw/virtio/virtio-balloon.c >> @@ -292,7 +292,7 @@ static void virtio_balloon_set_config(VirtIODevice *vdev, >> memcpy(&config, config_data, sizeof(struct virtio_balloon_config)); >> dev->actual = le32_to_cpu(config.actual); >> if (dev->actual != oldactual) { >> - qapi_event_send_balloon_change(ram_size - >> + qapi_event_send_balloon_change(vm_ram_size - >> ((ram_addr_t) dev->actual << VIRTIO_BALLOON_PFN_SHIFT), >> &error_abort); >> } >> @@ -307,7 +307,7 @@ static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f) >> static void virtio_balloon_stat(void *opaque, BalloonInfo *info) >> { >> VirtIOBalloon *dev = opaque; >> - info->actual = ram_size - ((uint64_t) dev->actual << >> + info->actual = vm_ram_size - ((uint64_t) dev->actual << >> VIRTIO_BALLOON_PFN_SHIFT); >> } >> >> @@ -316,11 +316,11 @@ static void virtio_balloon_to_target(void *opaque, ram_addr_t target) >> VirtIOBalloon *dev = VIRTIO_BALLOON(opaque); >> VirtIODevice *vdev = VIRTIO_DEVICE(dev); >> >> - if (target > ram_size) { >> - target = ram_size; >> + if (target > vm_ram_size) { >> + target = vm_ram_size; >> } >> if (target) { >> - dev->num_pages = (ram_size - target) >> VIRTIO_BALLOON_PFN_SHIFT; >> + dev->num_pages = (vm_ram_size - target) >> VIRTIO_BALLOON_PFN_SHIFT; >> virtio_notify_config(vdev); >> } >> } >> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h >> index e3ec4c8..f55db6a 100644 >> --- a/include/exec/cpu-common.h >> +++ b/include/exec/cpu-common.h >> @@ -46,6 +46,7 @@ typedef uintptr_t ram_addr_t; >> #endif >> >> extern ram_addr_t ram_size; >> +extern ram_addr_t vm_ram_size; >> >> /* memory API */ >> >> diff --git a/vl.c b/vl.c >> index 9c9acf5..5d20d0c 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -132,6 +132,7 @@ DisplayType display_type = DT_DEFAULT; >> static int display_remote; >> const char* keyboard_layout = NULL; >> ram_addr_t ram_size; >> +ram_addr_t vm_ram_size; /* ram_size + hotplugged ram size */ >> const char *mem_path = NULL; >> int mem_prealloc = 0; /* force preallocation of physical target memory */ >> int nb_nics; >> @@ -3015,6 +3016,7 @@ int main(int argc, char **argv, char **envp) >> machine_class = find_default_machine(); >> cpu_model = NULL; >> ram_size = default_ram_size; >> + vm_ram_size = ram_size; >> snapshot = 0; >> cyls = heads = secs = 0; >> translation = BIOS_ATA_TRANSLATION_AUTO; >> @@ -3388,6 +3390,7 @@ int main(int argc, char **argv, char **envp) >> "'%s' option\n", slots_str ? "maxmem" : "slots"); >> exit(EXIT_FAILURE); >> } >> + vm_ram_size = ram_size; >> break; >> } >> #ifdef CONFIG_TPM > > > . >