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

[Michael Roth <mdroth@linux.vnet.ibm.com>]

On 06/16/2011 01:29 PM, Luiz Capitulino wrote:
> On Tue, 14 Jun 2011 15:06:21 -0500
> Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
>
>>
>> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
>> ---
>>   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<mdroth@linux.vnet.ibm.com>
>> + *
>> + * 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<aglitke@linux.vnet.ibm.com>
>> + *  Michael Roth<mdroth@linux.vnet.ibm.com>
>> + *
>> + * 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);
>