Re: [Qemu-devel] [PATCH v5 1/5] guest agent: command state class

[Michael Roth]

On 06/16/2011 01:29 PM, Luiz Capitulino wrote:
> On Tue, 14 Jun 2011 15:06:21 -0500
> Michael Roth<address@hidden>  wrote:
>
> > Signed-off-by: Michael Roth<address@hidden>
> > ---
> >   qga/guest-agent-command-state.c |   73 +++++++++++++++++++++++++++++++++++++++
> >   qga/guest-agent-core.h          |   25 +++++++++++++
>
> It's not possible to build this. I see that the last patch has the Makefile
> changes, but what we want is that a patch introducing a .c file also updates
> the Makefile, so that the .c file can be built.

I made it a point to test build each C file as introduced, but I'm not 
sure there's a way of doing incremental builds without modifying the 
Makefile manually anyway, since we'll be missing a main() function until 
qemu-ga.c. Or maybe I'm just missing something...any ideas?

> >   2 files changed, 98 insertions(+), 0 deletions(-)
> >   create mode 100644 qga/guest-agent-command-state.c
> >   create mode 100644 qga/guest-agent-core.h
> >
> > diff --git a/qga/guest-agent-command-state.c b/qga/guest-agent-command-state.c
> > new file mode 100644
> > index 0000000..969da23
> > --- /dev/null
> > +++ b/qga/guest-agent-command-state.c
> > @@ -0,0 +1,73 @@
> > +/*
> > + * QEMU Guest Agent command state interfaces
> > + *
> > + * Copyright IBM Corp. 2011
> > + *
> > + * Authors:
> > + *  Michael Roth<address@hidden>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +#include<glib.h>
> > +#include "qga/guest-agent-core.h"
> > +
> > +struct GACommandState {
> > +    GSList *groups;
> > +};
> > +
> > +typedef struct GACommandGroup {
> > +    void (*init)(void);
> > +    void (*cleanup)(void);
> > +} GACommandGroup;
> > +
> > +/* handle init/cleanup for stateful guest commands */
> > +
> > +void ga_command_state_add(GACommandState *cs,
> > +                          void (*init)(void),
> > +                          void (*cleanup)(void))
> > +{
> > +    GACommandGroup *cg = g_malloc0(sizeof(GACommandGroup));
>
> This is linked against qemu-ga only, right? Otherwise I think we should
> use qemu_mallocz(). And you probably want to check against NULL.
>
> > +    cg->init = init;
> > +    cg->cleanup = cleanup;
> > +    cs->groups = g_slist_append(cs->groups, cg);
>
> Not that I'm asking to you change anything here, but we're going to
> get a funny mix with QObjects :)

glib is a hard dependency here so i tried to stick with it where 
possible, since there's a lot of code where i'm kinda forced to use 
GObject stuff, like all the GIO functions for instance (GObject + GError 
+ g_free etc). I think fsfreeze still uses qlist/qemu_free/etc, best I 
can do is fix that up to relegate all the non-glib stuff to qemu-ga.c 
and the qapi/qmp stuff it pulls in.

> > +}
> > +
> > +static void ga_command_group_init(gpointer opaque, gpointer unused)
> > +{
> > +    GACommandGroup *cg = opaque;
> > +
> > +    g_assert(cg);
> > +    if (cg->init) {
> > +        cg->init();
> > +    }
> > +}
> > +
> > +void ga_command_state_init_all(GACommandState *cs)
> > +{
> > +    g_assert(cs);
> > +    g_slist_foreach(cs->groups, ga_command_group_init, NULL);
> > +}
> > +
> > +static void ga_command_group_cleanup(gpointer opaque, gpointer unused)
> > +{
> > +    GACommandGroup *cg = opaque;
> > +
> > +    g_assert(cg);
> > +    if (cg->cleanup) {
> > +        cg->cleanup();
> > +    }
> > +}
> > +
> > +void ga_command_state_cleanup_all(GACommandState *cs)
> > +{
> > +    g_assert(cs);
> > +    g_slist_foreach(cs->groups, ga_command_group_cleanup, NULL);
> > +}
> > +
> > +GACommandState *ga_command_state_new(void)
> > +{
> > +    GACommandState *cs = g_malloc0(sizeof(GACommandState));
> > +    cs->groups = NULL;
> > +    return cs;
> > +}
> > diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
> > new file mode 100644
> > index 0000000..688f120
> > --- /dev/null
> > +++ b/qga/guest-agent-core.h
> > @@ -0,0 +1,25 @@
> > +/*
> > + * QEMU Guest Agent core declarations
> > + *
> > + * Copyright IBM Corp. 2011
> > + *
> > + * Authors:
> > + *  Adam Litke<address@hidden>
> > + *  Michael Roth<address@hidden>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +#include "qapi/qmp-core.h"
> > +#include "qemu-common.h"
> > +
> > +#define QGA_VERSION "1.0"
> > +
> > +typedef struct GACommandState GACommandState;
> > +
> > +void ga_command_state_add(GACommandState *cs,
> > +                          void (*init)(void),
> > +                          void (*cleanup)(void));
> > +void ga_command_state_init_all(GACommandState *cs);
> > +void ga_command_state_cleanup_all(GACommandState *cs);
> > +GACommandState *ga_command_state_new(void);