On Mon, 22 Sep 2014 11:36:35 +0800 zhanghailiang <zhang.zhanghaili...@huawei.com> wrote:
> Hi Igor, > > Thanks for your reviewing... > > > On Mon, 15 Sep 2014 20:29:38 +0800 > > zhanghailiang <zhang.zhanghaili...@huawei.com> 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. Yep, mem hotplug is a coarse-grained method of control and requires some planning when plugging/unplugging memory. I'm not oposed to ballooning, just make sure that guests that have a ballooning driver will still work as expected when memory plugged/unplugged. > > Thanks, > zhanghailiang > > >> > >> Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com> > >> --- > >> 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 > > > > > > . > > > > >