Re: [Qemu-devel] [PATCH v2 1/1] nvdimm: disable balloon

[Vladimir Sementsov-Ogievskiy]

On 26.01.2016 13:38, Igor Mammedov wrote:
> On Mon, 25 Jan 2016 19:50:31 +0300
> "Denis V. Lunev" <address@hidden> wrote:
>
> > From: Vladimir Sementsov-Ogievskiy <address@hidden>
> >
> > NVDIMM for now is planned to use as a backing store for DAX filesystem
> > in the guest and thus this memory is excluded from guest memory management
> > and LRUs.
> >
> > In this case libvirt running QEMU along with configured balloon almost
> > immediately inflates balloon and effectively kill the guest as
> > qemu counts nvdimm as part of the ram.
> Isn't issue in ballooning impl. and not of pc-dimm/nvdimm,
> so make ballooning code to distinguish between kinds of memory
> rather than adding not related fields to PCDIMMDeviceClass
>
> why don't just move get_current_ram_size() into the sole user
> virtio-balloon.c and ignore NVDIMMs when counting ram in
> get_current_ram_size(),
> that would be much less intrusive patch.

Ok, will do.

> > Counting dimm devices as part of the ram for ballooning was started from
> > commit 463756d03:
> >   virtio-balloon: Fix balloon not working correctly when hotplug memory
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> > Signed-off-by: Denis V. Lunev <address@hidden>
> > CC: Stefan Hajnoczi <address@hidden>
> > CC: Xiao Guangrong <address@hidden>
> > CC: "Michael S. Tsirkin" <address@hidden>
> > CC: Igor Mammedov <address@hidden>
> > CC: Eric Blake <address@hidden>
> > CC: Markus Armbruster <address@hidden>
> > ---
> > v2:
> >      - some rewordings, thanks to Eric Blake
> >
> >   hw/mem/nvdimm.c          | 4 ++++
> >   hw/mem/pc-dimm.c         | 7 ++++++-
> >   include/hw/mem/pc-dimm.h | 1 +
> >   qapi-schema.json         | 5 ++++-
> >   4 files changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
> > index 4fd397f..4f4d29a 100644
> > --- a/hw/mem/nvdimm.c
> > +++ b/hw/mem/nvdimm.c
> > @@ -27,9 +27,13 @@
> >   static void nvdimm_class_init(ObjectClass *oc, void *data)
> >   {
> >       DeviceClass *dc = DEVICE_CLASS(oc);
> > +    PCDIMMDeviceClass *ddc = PC_DIMM_CLASS(oc);
> >   
> >       /* nvdimm hotplug has not been supported yet. */
> >       dc->hotpluggable = false;
> > +
> > +    /* ballooning is not supported */
> > +    ddc->in_ram = false;
> >   }
> >   
> >   static TypeInfo nvdimm_info = {
> > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> > index d5cdab2..e0f869d 100644
> > --- a/hw/mem/pc-dimm.c
> > +++ b/hw/mem/pc-dimm.c
> > @@ -164,6 +164,7 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque)
> >               MemoryDeviceInfo *info = g_new0(MemoryDeviceInfo, 1);
> >               PCDIMMDeviceInfo *di = g_new0(PCDIMMDeviceInfo, 1);
> >               DeviceClass *dc = DEVICE_GET_CLASS(obj);
> > +            PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(obj);
> >               PCDIMMDevice *dimm = PC_DIMM(obj);
> >   
> >               if (dev->id) {
> > @@ -172,6 +173,7 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque)
> >               }
> >               di->hotplugged = dev->hotplugged;
> >               di->hotpluggable = dc->hotpluggable;
> > +            di->in_ram = ddc->in_ram;
> >               di->addr = dimm->addr;
> >               di->slot = dimm->slot;
> >               di->node = dimm->node;
> > @@ -205,7 +207,9 @@ ram_addr_t get_current_ram_size(void)
> >           if (value) {
> >               switch (value->type) {
> >               case MEMORY_DEVICE_INFO_KIND_DIMM:
> > -                size += value->u.dimm->size;
> > +                if (value->u.dimm->in_ram) {
> > +                    size += value->u.dimm->size;
> > +                }
> >                   break;
> >               default:
> >                   break;
> > @@ -444,6 +448,7 @@ static void pc_dimm_class_init(ObjectClass *oc, void *data)
> >       dc->props = pc_dimm_properties;
> >       dc->desc = "DIMM memory module";
> >   
> > +    ddc->in_ram = true;
> >       ddc->get_memory_region = pc_dimm_get_memory_region;
> >   }
> >   
> > diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> > index d83bf30..3bcb505 100644
> > --- a/include/hw/mem/pc-dimm.h
> > +++ b/include/hw/mem/pc-dimm.h
> > @@ -65,6 +65,7 @@ typedef struct PCDIMMDevice {
> >   typedef struct PCDIMMDeviceClass {
> >       /* private */
> >       DeviceClass parent_class;
> > +    bool in_ram;
> >   
> >       /* public */
> >       MemoryRegion *(*get_memory_region)(PCDIMMDevice *dimm);
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 2e960db..3cafa2b 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -3922,6 +3922,8 @@
> >   #
> >   # @hotpluggable: true if device if could be added/removed while machine is 
> > running
> >   #
> > +# @in-ram: true if device is counted in current ram size (since 2.6)
> > +#
> >   # Since: 2.1
> >   ##
> >   { 'struct': 'PCDIMMDeviceInfo',
> > @@ -3932,7 +3934,8 @@
> >               'node': 'int',
> >               'memdev': 'str',
> >               'hotplugged': 'bool',
> > -            'hotpluggable': 'bool'
> > +            'hotpluggable': 'bool',
> > +            'in-ram': 'bool'
> >             }
> >   }
> >   


--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.