Re: [Qemu-devel] [PATCH v2 1/1] virtio-balloon: include statistics of disk/file caches

["Tomáš Golembiovský" <tgolembi@redhat.com>]

On Mon, 19 Feb 2018 21:11:27 -0800
Jonathan Helman <jonathan.helman@oracle.com> wrote:

> > On Feb 19, 2018, at 5:11 AM, Tomáš Golembiovský <tgolembi@redhat.com> wrote:
> > 
> > On Wed, 14 Feb 2018 00:07:53 +0200
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >   
> >> On Tue, Feb 13, 2018 at 12:29:39PM -0800, Jonathan Helman wrote:  
> >>> 
> >>> 
> >>> On 02/05/2018 04:08 AM, Tomáš Golembiovský wrote:    
> >>>> ping
> >>>> 
> >>>> On Tue,  5 Dec 2017 13:14:46 +0100
> >>>> Tomáš Golembiovský <tgolembi@redhat.com> wrote:
> >>>>   
> >>> 
> >>> It would be good to include the corresponding upstream kernel change in the
> >>> commit message. This would be similar to a previous change:
> >>> https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg05555.html    
> >> 
> >> good idea, but this has been merged by now.  
> > 
> > Maybe next time, sorry.
> > 
> >   
> >>>>> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> >>>>> ---
> >>>>>  hw/virtio/virtio-balloon.c                      | 1 +
> >>>>>  include/standard-headers/linux/virtio_balloon.h | 3 ++-
> >>>>>  2 files changed, 3 insertions(+), 1 deletion(-)
> >>>>> 
> >>>>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> >>>>> index 37cde38982..8141326a51 100644
> >>>>> --- a/hw/virtio/virtio-balloon.c
> >>>>> +++ b/hw/virtio/virtio-balloon.c
> >>>>> @@ -50,6 +50,7 @@ static const char *balloon_stat_names[] = {
> >>>>>     [VIRTIO_BALLOON_S_MEMFREE] = "stat-free-memory",
> >>>>>     [VIRTIO_BALLOON_S_MEMTOT] = "stat-total-memory",
> >>>>>     [VIRTIO_BALLOON_S_AVAIL] = "stat-available-memory",
> >>>>> +   [VIRTIO_BALLOON_S_CACHES] = "stat-disk-caches",
> >>>>>     [VIRTIO_BALLOON_S_NR] = NULL
> >>>>>  };
> >>>>> diff --git a/include/standard-headers/linux/virtio_balloon.h b/include/standard-headers/linux/virtio_balloon.h
> >>>>> index 9d06ccd066..7b0a41b8fc 100644
> >>>>> --- a/include/standard-headers/linux/virtio_balloon.h
> >>>>> +++ b/include/standard-headers/linux/virtio_balloon.h
> >>>>> @@ -52,7 +52,8 @@ struct virtio_balloon_config {
> >>>>>  #define VIRTIO_BALLOON_S_MEMFREE  4   /* Total amount of free memory */
> >>>>>  #define VIRTIO_BALLOON_S_MEMTOT   5   /* Total amount of memory */
> >>>>>  #define VIRTIO_BALLOON_S_AVAIL    6   /* Available memory as in /proc */
> >>>>> -#define VIRTIO_BALLOON_S_NR       7
> >>>>> +#define VIRTIO_BALLOON_S_CACHES   7   /* Disk caches */    
> >>> 
> >>> I've been wondering, VIRTIO_BALLOON_S_AVAIL is not in the virtio spec (see
> >>> Section 5.5.6.3). It seems like this header file needs to be in sync with
> >>> the virtio spec in order to make this change.
> >>> 
> >>> I have a similar change to add a new statistic and was wondering this.    
> >> 
> >> Absolutely. Tomáš?  
> > 
> > Sent. I also included description for the VIRTIO_BALLOON_S_AVAIL that
> > was missing.
> > 
> >   
> >>>>> +#define VIRTIO_BALLOON_S_NR       8
> >>>>>  /*
> >>>>>   * Memory statistics structure.
> >>>>> -- 
> >>>>> 2.15.1
> >>>>>   
> >>>> 
> >>>>   
> >>> 
> >>> You need to add your new stat to the list of stats in
> >>> docs/virtio-balloon-stats.txt.
> >>> 
> >>> Jon    
> >> 
> >> Can't hurt, I agree.  
> > 
> > Sent.
> >   
> 
> Awesome, thank you.
> 
> Do you also plan to make the commensurate changes in libvirt, or did I just miss the patch?

Yes, I do. I just didn't get to it yet.

    Tomas

> 
> Jon
> 
> >    Tomas
> > 
> > -- 
> > Tomáš Golembiovský <tgolembi@redhat.com>  
> 


-- 
Tomáš Golembiovský <tgolembi@redhat.com>