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

[Juan Quintela]

Adam Litke <address@hidden> 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.