From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:56650)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1UPblc-0005vu-OX
	for qemu-devel@nongnu.org; Tue, 09 Apr 2013 12:51:37 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1UPblX-0003pN-PK
	for qemu-devel@nongnu.org; Tue, 09 Apr 2013 12:51:32 -0400
Received: from mx1.redhat.com ([209.132.183.28]:40294)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1UPblX-0003p9-GH
	for qemu-devel@nongnu.org; Tue, 09 Apr 2013 12:51:27 -0400
Message-ID: <516446E1.9070002@redhat.com>
Date: Tue, 09 Apr 2013 18:50:41 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1365476681-31593-1-git-send-email-mrhines@linux.vnet.ibm.com>
	<1365476681-31593-10-git-send-email-mrhines@linux.vnet.ibm.com>
In-Reply-To: <1365476681-31593-10-git-send-email-mrhines@linux.vnet.ibm.com>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [RFC PATCH RDMA support v5: 09/12] transmit pc.ram
	using RDMA
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: mrhines@linux.vnet.ibm.com
Cc: aliguori@us.ibm.com, mst@redhat.com, qemu-devel@nongnu.org, owasserm@redhat.com, abali@us.ibm.com, mrhines@us.ibm.com, gokul@us.ibm.com

Il 09/04/2013 05:04, mrhines@linux.vnet.ibm.com ha scritto:
> 
> +            if ((bytes_sent = save_rdma_page(f, block->offset, 
> +                            offset, cont, TARGET_PAGE_SIZE, zero)) >= 0) {
> +                acct_info.norm_pages++;
> +                qemu_file_update_position(f, bytes_sent);
> +            } else if (zero) {

I think this should become a new QEMUFileOps member, save_ram_page.  If
NULL it can return ENOTSUP.

> +/* 
> + * Inform server to begin handling dynamic page registrations
> + */
> +static void ram_registration_start(QEMUFile *f)
> +{
> +    if(qemu_file_ops_are(f, &rdma_write_ops)) {
> +        qemu_put_be64(f, RAM_SAVE_FLAG_RDMA);
> +    }
> +}
> +
> +/*
> + * Inform server that dynamic registrations are done for now.
> + * First, flush writes, if any.
> + */
> +static int ram_registration_stop(QEMUFile *f)
> +{
> +    int ret = 0;
> +
> +    if (qemu_file_ops_are(f, &rdma_write_ops)) {
> +        ret = qemu_rdma_drain_cq(f);
> +        if(ret >= 0)
> +            ret = qemu_rdma_finish_registrations(f);
> +    }
> +
> +    return ret;

I think this should become two QEMUFileOps instead: before_ram_iterate
and after_ram_iterate, or something like that.  Again, if NULL they
should just do nothing.

Errors from the callback should be persisted in the QEMUFile with
qemu_file_set_error.  Then you do not need any checks in the caller.

Paolo