[Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V8)

2010-03-09 Thread Luiz Capitulino
On Tue, 09 Mar 2010 08:48:43 -0600
Adam Litke  wrote:

> On Tue, 2010-03-09 at 11:22 -0300, Luiz Capitulino wrote:
> > On Tue, 09 Mar 2010 14:51:31 +0100
> > Juan Quintela  wrote:
> > 
> > > Any recompilation/etc would break migration.  I have tried to understand
> > > what happened with monitor async commands, and my head exploded in
> > > indirections.
> > 
> >  The Monitor needs lots of cleanups to make things more obvious.
> > 
> > > Is there any written explanation of what are we trying to do here?
> > 
> >  Only the commit log 940cc30.
> > 
> >  Basically, an asynchronous handler has a completion function which is
> > called when the handler completes.
> > 
> >  If we're in the user Monitor, it's suspended until the completion
> > function is called. In QMP, the handler returns immediately and we
> > _should_ be emitting an event when we have the answer.
> > 
> >  The current code doesn't do that, which seems to be a new issue.
> 
> With current git, I cannot get QMP to recognize any commands.  Unless
> this is a known issue, I will look into it further to see what has
> caused it.

 You have to issue the 'qmp_capabilites' command before issuing commands,
take a look at the QMP/README and QMP/qmp-spec.txt files.




[Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V8)

2010-03-09 Thread Juan Quintela
Adam Litke  wrote:
> The changes in V8 of this patch are related to the monitor infrastructure.  No
> changes to the virtio interface core have been made since V4.  This is 
> intended
> to apply on top of my API for asynchronous monitor commands patch.

I know that I am late reviewing this.  Once told that, it has some
issues with migration.

>  typedef struct VirtIOBalloon
>  {



> +MonitorCompletion *stats_callback;

Notice this stats_callback.

typedef void (MonitorCompletion)(void *opaque, QObject *ret_data);

> +void *stats_opaque_callback_data;
>  } VirtIOBalloon;



It don't update the version field, should be two (that is easy to fix).

>  qemu_put_be32(f, s->num_pages);
>  qemu_put_be32(f, s->actual);
> +qemu_put_buffer(f, (uint8_t *)&s->stats_vq_elem,
>  sizeof(VirtQueueElement));

We send a struct directly, migration inter-architectures is broken (not
that virtio drivers in general are good here).

> +qemu_put_buffer(f, (uint8_t *)&s->stats_vq_offset, sizeof(size_t));
> +qemu_put_buffer(f, (uint8_t *)&s->stats_callback,
> sizeof(MonitorCompletion));

We send a pointer to one function.

> +qemu_put_buffer(f, (uint8_t *)&s->stats_opaque_callback_data,
> sizeof(void));

And a pointer to one opaque.

Any recompilation/etc would break migration.  I have tried to understand
what happened with monitor async commands, and my head exploded in
indirections.

Is there any written explanation of what are we trying to do here?

Later, Juan.




[Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V8)

2010-03-09 Thread Luiz Capitulino
On Tue, 09 Mar 2010 14:51:31 +0100
Juan Quintela  wrote:

> Any recompilation/etc would break migration.  I have tried to understand
> what happened with monitor async commands, and my head exploded in
> indirections.

 The Monitor needs lots of cleanups to make things more obvious.

> Is there any written explanation of what are we trying to do here?

 Only the commit log 940cc30.

 Basically, an asynchronous handler has a completion function which is
called when the handler completes.

 If we're in the user Monitor, it's suspended until the completion
function is called. In QMP, the handler returns immediately and we
_should_ be emitting an event when we have the answer.

 The current code doesn't do that, which seems to be a new issue.




[Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V8)

2010-03-09 Thread Adam Litke
On Tue, 2010-03-09 at 11:22 -0300, Luiz Capitulino wrote:
> On Tue, 09 Mar 2010 14:51:31 +0100
> Juan Quintela  wrote:
> 
> > Any recompilation/etc would break migration.  I have tried to understand
> > what happened with monitor async commands, and my head exploded in
> > indirections.
> 
>  The Monitor needs lots of cleanups to make things more obvious.
> 
> > Is there any written explanation of what are we trying to do here?
> 
>  Only the commit log 940cc30.
> 
>  Basically, an asynchronous handler has a completion function which is
> called when the handler completes.
> 
>  If we're in the user Monitor, it's suspended until the completion
> function is called. In QMP, the handler returns immediately and we
> _should_ be emitting an event when we have the answer.
> 
>  The current code doesn't do that, which seems to be a new issue.

With current git, I cannot get QMP to recognize any commands.  Unless
this is a known issue, I will look into it further to see what has
caused it.

-- 
Thanks,
Adam