[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] virtio-balloon: Fix ballooning not working corr
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH] virtio-balloon: Fix ballooning not working correctly when hotplug memory |
Date: |
Mon, 22 Sep 2014 10:14:53 +0200 |
On Mon, 22 Sep 2014 11:36:35 +0800
zhanghailiang <address@hidden> wrote:
> Hi Igor,
>
> Thanks for your reviewing...
>
> > On Mon, 15 Sep 2014 20:29:38 +0800
> > zhanghailiang <address@hidden> 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 <address@hidden>
> >> ---
> >> 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
> >
> >
> > .
> >
>
>
>