From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:37640)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1USjgZ-00021q-K4
	for qemu-devel@nongnu.org; Thu, 18 Apr 2013 03:55:17 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1USjgY-0001U5-1a
	for qemu-devel@nongnu.org; Thu, 18 Apr 2013 03:55:15 -0400
Received: from mail-ee0-f50.google.com ([74.125.83.50]:35086)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1USjgX-0001Tn-PK
	for qemu-devel@nongnu.org; Thu, 18 Apr 2013 03:55:13 -0400
Received: by mail-ee0-f50.google.com with SMTP id e53so1145753eek.37
	for <qemu-devel@nongnu.org>; Thu, 18 Apr 2013 00:55:13 -0700 (PDT)
Sender: Paolo Bonzini <paolo.bonzini@gmail.com>
Message-ID: <516FA6D5.8010307@redhat.com>
Date: Thu, 18 Apr 2013 09:55:01 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1366240040-10730-1-git-send-email-mrhines@linux.vnet.ibm.com>
	<1366240040-10730-9-git-send-email-mrhines@linux.vnet.ibm.com>
In-Reply-To: <1366240040-10730-9-git-send-email-mrhines@linux.vnet.ibm.com>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PULL v4 08/11] rdma: core logic
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, quintela@redhat.com, qemu-devel@nongnu.org, owasserm@redhat.com, abali@us.ibm.com, mrhines@us.ibm.com, gokul@us.ibm.com

Il 18/04/2013 01:07, mrhines@linux.vnet.ibm.com ha scritto:
> +/*
> + * Virtual address of the above structures used for transmitting
> + * the RAMBlock descriptions at connection-time.
> + * This structure is *not* transmitted.
> + */
> +typedef struct RDMALocalBlocks {
> +    int num_blocks;
> +    RDMALocalBlock *block;
> +} RDMALocalBlocks;
> +
> +/*
> + * Same as above
> + */
> +typedef struct RDMARemoteBlocks {
> +    RDMARemoteBlock *block;
> +    void *remote_area;
> +} RDMARemoteBlocks;

block and remote_area can be reduced to a single pointer.

> +    if (rdma == NULL) {
> +        goto err;
> +    }
> +
> +    ret = qemu_rdma_source_init(rdma, NULL,
> +        s->enabled_capabilities[MIGRATION_CAPABILITY_X_CHUNK_REGISTER_DESTINATION]);
> +
> +    if (ret) {
> +        goto err;
> +    }
> +
> +    DPRINTF("qemu_rdma_source_init success\n");
> +    ret = qemu_rdma_connect(rdma, NULL);

The usage of error_setg is correct, but here you must pass your errp
down to qemu_rdma_source_init and qemu_rdma_connect.  You can then
remove this:

+    error_setg(errp, "Error connecting using rdma! %d\n", ret);

because the error was already set in the callees.

Also, you should use the Error API also on the destination side, where
rdma_start_incoming_migration can pass errp to qemu_rdma_dest_prepare or
set it itself instead of printing to stderr.

The reason is that when using a management layer stderr will be logged
but not printed to the operator's console.  Instead, Errors are sent on
the monitor connection.  The management layer(s) then pick up the error
and propagate it through the various APIs until they appear in the
terminal/browser/whatever.

Nothing else from me.

Paolo