[Spice-devel] [PATCH spice-server v2 1/3] Separate code to manage statistic file

[Frediano Ziglio]

> 
> On Wed, Nov 16, 2016 at 11:35:05AM +0000, Frediano Ziglio wrote:
> > Code is quite independent so move in separate file.
> > 
> > diff --git a/server/reds.c b/server/reds.c
> > index 1a58c9d..731a356 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -76,6 +76,8 @@
> >  #include "main-channel-client.h"
> >  #include "red-client.h"
> >  
> > +#define REDS_MAX_STAT_NODES 100
> > +
> 
> Not overly convinced this needs to be configurable from the stat user.
> But this can be adjusted later if we decide we don't want that.
> 

Just moved the constant.
Maybe the file could be also extended dynamically.

> >  static void reds_client_monitors_config(RedsState *reds,
> >  VDAgentMonitorsConfig *monitors_config);
> >  static gboolean reds_use_client_monitors_config(RedsState *reds);
> >  
> >      if (reds_init_net(reds) < 0) {
> 
> > diff --git a/server/stat-file.c b/server/stat-file.c
> > new file mode 100644
> > index 0000000..7a07edb
> > --- /dev/null
> > +++ b/server/stat-file.c
> > @@ -0,0 +1,176 @@
> > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > +/*
> > +   Copyright (C) 2009-2016 Red Hat, Inc.
> > +
> > +   This library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   This library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with this library; if not, see
> > <http://www.gnu.org/licenses/>.
> > +*/
> > +#ifdef HAVE_CONFIG_H
> > +#include <config.h>
> > +#endif
> > +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <unistd.h>
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <sys/mman.h>
> > +#include <common/log.h>
> > +#include <common/mem.h>
> > +
> > +#include "stat-file.h"
> > +
> > +#define STAT_SHM_SIZE(max_nodes) (sizeof(SpiceStat) + max_nodes *
> > sizeof(SpiceStatNode))
> 
> (max_nodes)
> 
> > +
> > +void stat_file_init(RedStatFile *stat_file, unsigned int max_nodes)
> > +{
> > +    int shm_name_len;
> > +    int fd;
> > +    size_t shm_size = STAT_SHM_SIZE(max_nodes);
> > +
> > +    stat_file->max_nodes = max_nodes;
> > +    shm_name_len = strlen(SPICE_STAT_SHM_NAME) + 20;
> > +    stat_file->shm_name = (char *)spice_malloc(shm_name_len);
> > +    snprintf(stat_file->shm_name, shm_name_len, SPICE_STAT_SHM_NAME,
> > getpid());
> 
> Here g_strdup_printf() could be used (agreed that this does not belong
> in this patch).
> 

I'll write a follow up.

> > diff --git a/server/stat-file.h b/server/stat-file.h
> > new file mode 100644
> > index 0000000..8ac8efc
> > --- /dev/null
> > +++ b/server/stat-file.h
> > @@ -0,0 +1,43 @@
> > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > +/*
> > +   Copyright (C) 2016 Red Hat, Inc.
> > +
> > +   This library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   This library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with this library; if not, see
> > <http://www.gnu.org/licenses/>.
> > +*/
> > +#ifndef STAT_FILE_H_
> > +#define STAT_FILE_H_
> > +
> > +#include <pthread.h>
> > +#include <spice/stats.h>
> > +
> > +typedef uint32_t StatNodeRef;
> > +#define INVALID_STAT_REF (~(StatNodeRef)0)
> > +
> > +typedef struct {
> > +    char *shm_name;
> > +    SpiceStat *stat;
> > +    pthread_mutex_t lock;
> > +    unsigned int max_nodes;
> > +} RedStatFile;
> 
> Any reason why this is not private? I think the struct content is not
> used outside of stat-file.c
> 
> Christophe
> 

It used by the test.
Also requires less change to reds-private.h.

Frediano