From: Orit Wasserman <owasserm@redhat.com>
To: "Michael R. Hines" <mrhines@linux.vnet.ibm.com>
Cc: aliguori@us.ibm.com, quintela@redhat.com, mst@redhat.com,
qemu-devel@nongnu.org, abali@us.ibm.com, mrhines@us.ibm.com,
gokul@us.ibm.com, pbonzini@redhat.com
Subject: Re: [Qemu-devel] [PULL v4 08/11] rdma: core logic
Date: Thu, 18 Apr 2013 18:51:44 +0300 [thread overview]
Message-ID: <51701690.5070306@redhat.com> (raw)
In-Reply-To: <516FFB11.90505@linux.vnet.ibm.com>
On 04/18/2013 04:54 PM, Michael R. Hines wrote:
> On 04/18/2013 04:44 AM, Orit Wasserman wrote:
>> Hi Michael,
>>
>> I don't see you addressed any of the comment I had in v3
>> (especially the error handling)
>>
>> please, fix those
>
> I did fix them, but not all of your comments were correct,
> because I was passing errp in many places were errp
> did not supposed to belong there.
If you decide not to fix some comment you just need
to reply to the comment and explain the reason,
this makes the reviewing process easier.
>
> So, first I *removed* errp from being proliferated all over
> the entire file, which was not necessary.
>
> Then, in the places where errp is required by migration.c,
> I added new uses of errp.
That is fine as long as external rdma API use errp and the
internal always return some error code.
Also I noticed that the errors are very general for example
in qemu_rdma_connect we set the same error always it will be more
helpful to have different errors for each case. We need
errors that can help the user to understand what went wrong.
Orit
>
> There are some places, as Paolo mentioned where errp was
> written twice, which I must fix, however.
>
>
next prev parent reply other threads:[~2013-04-18 15:51 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-17 23:07 [Qemu-devel] [PULL v4 00/11] rdma: migration support mrhines
2013-04-17 23:07 ` [Qemu-devel] [PULL v4 01/11] rdma: export yield_until_fd_readable() mrhines
2013-04-17 23:07 ` [Qemu-devel] [PULL v4 02/11] rdma: introduce qemu_ram_foreach_block() mrhines
2013-04-17 23:07 ` [Qemu-devel] [PULL v4 03/11] rdma: introduce qemu_file_mode_is_not_valid() mrhines
2013-04-17 23:07 ` [Qemu-devel] [PULL v4 04/11] rdma: export ram_handle_compressed() mrhines
2013-04-17 23:07 ` [Qemu-devel] [PULL v4 05/11] rdma: export qemu_fflush() mrhines
2013-04-17 23:07 ` [Qemu-devel] [PULL v4 06/11] rdma: new QEMUFileOps hooks mrhines
2013-04-17 23:07 ` [Qemu-devel] [PULL v4 07/11] rdma: introduce capability for chunk registration mrhines
2013-04-18 22:07 ` Eric Blake
2013-04-19 0:34 ` Michael R. Hines
2013-04-20 17:02 ` Michael S. Tsirkin
2013-04-21 13:19 ` Paolo Bonzini
2013-04-21 14:17 ` Michael S. Tsirkin
2013-04-21 17:19 ` Michael R. Hines
2013-04-21 19:13 ` Michael S. Tsirkin
2013-04-21 16:05 ` Michael R. Hines
2013-04-21 18:59 ` Michael S. Tsirkin
2013-04-21 19:55 ` Michael R. Hines
2013-04-21 16:06 ` Michael R. Hines
2013-04-17 23:07 ` [Qemu-devel] [PULL v4 08/11] rdma: core logic mrhines
2013-04-18 7:55 ` Paolo Bonzini
2013-04-18 13:57 ` Michael R. Hines
2013-04-18 7:58 ` Michael S. Tsirkin
2013-04-18 13:59 ` Michael R. Hines
2013-04-18 13:06 ` Michael S. Tsirkin
2013-04-18 14:14 ` Michael R. Hines
2013-04-18 13:32 ` Michael S. Tsirkin
2013-04-18 14:45 ` Michael R. Hines
2013-04-18 13:52 ` Michael S. Tsirkin
2013-04-18 15:14 ` Anthony Liguori
2013-04-18 14:53 ` [Qemu-devel] licensing of IBM contributions to QEMU (was Re: [PULL v4 08/11] rdma: core logic) Paolo Bonzini
2013-04-18 19:15 ` Michael R. Hines
2013-04-19 0:35 ` Anthony Liguori
2013-04-18 8:44 ` [Qemu-devel] [PULL v4 08/11] rdma: core logic Orit Wasserman
2013-04-18 13:54 ` Michael R. Hines
2013-04-18 15:51 ` Orit Wasserman [this message]
2013-04-18 19:41 ` Michael R. Hines
2013-04-18 22:12 ` Eric Blake
2013-04-19 0:35 ` Michael R. Hines
2013-04-17 23:07 ` [Qemu-devel] [PULL v4 09/11] rdma: send pc.ram mrhines
2013-04-17 23:07 ` [Qemu-devel] [PULL v4 10/11] rdma: print out throughput while debugging mrhines
2013-04-17 23:07 ` [Qemu-devel] [PULL v4 11/11] rdma: add documentation mrhines
2013-04-18 6:55 ` Michael S. Tsirkin
2013-04-19 0:57 ` Michael R. Hines
2013-04-17 23:39 ` [Qemu-devel] [PULL v4 00/11] rdma: migration support Anthony Liguori
2013-04-18 13:46 ` Michael R. Hines
2013-04-18 7:00 ` Michael S. Tsirkin
2013-04-18 13:49 ` Michael R. Hines
2013-04-18 13:50 ` Michael S. Tsirkin
2013-04-18 19:17 ` Michael R. Hines
2013-04-18 20:12 ` Michael S. Tsirkin
2013-04-18 21:28 ` Michael R. Hines
2013-04-18 20:33 ` Michael S. Tsirkin
2013-04-18 14:36 ` Michael R. Hines
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51701690.5070306@redhat.com \
--to=owasserm@redhat.com \
--cc=abali@us.ibm.com \
--cc=aliguori@us.ibm.com \
--cc=gokul@us.ibm.com \
--cc=mrhines@linux.vnet.ibm.com \
--cc=mrhines@us.ibm.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).