From: Li Zhang <lizhang@suse.de>
To: quintela@redhat.com
Cc: qemu-devel@nongnu.org, dgilbert@redhat.com, cfontana@suse.de
Subject: Re: [PATCH v2 1/1] multifd: Remove some redundant code
Date: Thu, 27 Jan 2022 19:11:35 +0100 [thread overview]
Message-ID: <d3563b01-6855-078a-4909-271c7c612dc0@suse.de> (raw)
In-Reply-To: <87bkzxld33.fsf@secure.mitica>
On 1/27/22 10:53 AM, Juan Quintela wrote:
> Li Zhang <lizhang@suse.de> wrote:
>> Clean up some unnecessary code
>>
>> Signed-off-by: Li Zhang <lizhang@suse.de>
>
> Hi
>
>> ---
>> migration/multifd.c | 12 +++---------
>> 1 file changed, 3 insertions(+), 9 deletions(-)
>>
>> diff --git a/migration/multifd.c b/migration/multifd.c
>> index 3242f688e5..212be1ed04 100644
>> --- a/migration/multifd.c
>> +++ b/migration/multifd.c
>> @@ -854,19 +854,16 @@ static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
>> Error *local_err = NULL;
>>
>> trace_multifd_new_send_channel_async(p->id);
>> - if (qio_task_propagate_error(task, &local_err)) {
>> - goto cleanup;
>> - } else {
>> + if (!qio_task_propagate_error(task, &local_err)) {
>> p->c = QIO_CHANNEL(sioc);
>> qio_channel_set_delay(p->c, false);
>> p->running = true;
>> if (!multifd_channel_connect(p, sioc, local_err)) {
>> - goto cleanup;
>> + multifd_new_send_channel_cleanup(p, sioc, local_err);
>> }
>> return;
>> }
>>
>> -cleanup:
>> multifd_new_send_channel_cleanup(p, sioc, local_err);
>> }
>>
>
> Once there, why are we duplicating the call to
> multifd_new_send_channel_cleanup()
>
> What about:
>
> static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
> {
> MultiFDSendParams *p = opaque;
> QIOChannel *sioc = QIO_CHANNEL(qio_task_get_source(task));
> Error *local_err = NULL;
>
> trace_multifd_new_send_channel_async(p->id);
> if (!qio_task_propagate_error(task, &local_err)) {
> p->c = QIO_CHANNEL(sioc);
> qio_channel_set_delay(p->c, false);
> p->running = true;
> if (multifd_channel_connect(p, sioc, local_err)) {
> return;
> }
> }
> multifd_new_send_channel_cleanup(p, sioc, local_err);
> }
>
> What do you think?
Hi Juan,
Sorry about the code. I check it again, it still needs to cleaup
if it fails on multifd_channel_connect(). I just remove the goto
and put the cleanup function there.
The second cleanup is called if (qio_task_propagate_error(task,
&local_err)) ), otherwise, it won't cleanup and return normally.
If removing the first cleanup and move the return, the logic is not
right. It is that: when no error happens, it still calls the second
cleanup.
Thanks
Li
>
> Later, Juan.
>
>
>> @@ -1078,10 +1075,7 @@ static void *multifd_recv_thread(void *opaque)
>>
>> ret = qio_channel_read_all_eof(p->c, (void *)p->packet,
>> p->packet_len, &local_err);
>> - if (ret == 0) { /* EOF */
>> - break;
>> - }
>> - if (ret == -1) { /* Error */
>> + if (ret == 0 || ret == -1) { /* 0: EOF -1: Error */
>> break;
>> }
>
> This bit is ok.
>
next prev parent reply other threads:[~2022-01-27 18:47 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-17 10:12 [PATCH v2 1/1] multifd: Remove some redundant code Li Zhang
2022-01-06 10:06 ` Li Zhang
2022-01-27 9:53 ` Juan Quintela
2022-01-27 9:56 ` Li Zhang
2022-01-27 18:11 ` Li Zhang [this message]
2022-01-27 18:31 ` Li Zhang
2022-01-27 19:10 ` Li Zhang
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=d3563b01-6855-078a-4909-271c7c612dc0@suse.de \
--to=lizhang@suse.de \
--cc=cfontana@suse.de \
--cc=dgilbert@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).