From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1NozqT-0008ED-Uw
	for qemu-devel@nongnu.org; Tue, 09 Mar 2010 08:51:37 -0500
Received: from [199.232.76.173] (port=58509 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1NozqS-0008DZ-Vj
	for qemu-devel@nongnu.org; Tue, 09 Mar 2010 08:51:37 -0500
Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim
	4.60) (envelope-from <quintela@redhat.com>) id 1NozqR-0003NQ-DO
	for qemu-devel@nongnu.org; Tue, 09 Mar 2010 08:51:36 -0500
Received: from mx1.redhat.com ([209.132.183.28]:35909)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <quintela@redhat.com>) id 1NozqR-0003NJ-1Z
	for qemu-devel@nongnu.org; Tue, 09 Mar 2010 08:51:35 -0500
From: Juan Quintela <quintela@redhat.com>
In-Reply-To: <1264537055.2890.40.camel@aglitke> (Adam Litke's message of "Tue,
	26 Jan 2010 14:17:35 -0600")
References: <1264537055.2890.40.camel@aglitke>
Date: Tue, 09 Mar 2010 14:51:31 +0100
Message-ID: <m3zl2hvc0s.fsf@trasno.mitica>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Subject: [Qemu-devel] Re: virtio: Add memory statistics reporting to the
	balloon driver (V8)
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Adam Litke <agl@us.ibm.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>, Luiz Capitulino <lcapitulino@redhat.com>, qemu-devel@nongnu.org, Avi Kivity <avi@redhat.com>

Adam Litke <agl@us.ibm.com> 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.