From: Luca Fancellu <luca.fancellu@arm.com>
To: Juergen Gross <jgross@suse.com>
Cc: Julien Grall <julien@xen.org>,
xen-devel@lists.xenproject.org, raphning@amazon.co.uk,
doebel@amazon.de, Julien Grall <jgrall@amazon.com>,
Ian Jackson <iwj@xenproject.org>, Wei Liu <wl@xen.org>
Subject: Re: [PATCH 03/10] tools/xenstore: Don't assume conn->in points to the LU request
Date: Thu, 24 Jun 2021 08:56:56 +0100 [thread overview]
Message-ID: <10F0635C-2AA7-42A4-B4BB-A9DD869113C9@arm.com> (raw)
In-Reply-To: <2e28c385-a8ba-1dc3-c41e-8b39f624947c@suse.com>
> On 24 Jun 2021, at 08:34, Juergen Gross <jgross@suse.com> wrote:
>
> On 24.06.21 09:32, Juergen Gross wrote:
>> On 16.06.21 16:43, Julien Grall wrote:
>>> From: Julien Grall <jgrall@amazon.com>
>>>
>>> call_delayed() is currently assuming that conn->in is NULL when
>>> handling delayed request. However, the connection is not paused.
>>> Therefore new request can be processed and conn->in may be non-NULL
>>> if we have only received a partial request.
>>>
>>> Furthermore, as we overwrite conn->in, the current partial request
>>> will not be transferred. This will result to corrupt the connection.
>>>
>>> Rather than updating conn->in, stash the LU request in lu_status and
>>> let each callback for delayed request to update conn->in when
>>> necessary.
>>>
>>> To keep a sane interface, the code to write the "OK" response the
>>> LU request is moved in xenstored_core.c.
>>>
>>> Fixes: c5ca1404b4 ("tools/xenstore: add support for delaying execution
>
>>> of a xenstore request")
>>> Fixes: ed6eebf17d ("tools/xenstore: dump the xenstore state for live update")
>>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>> With dropping the conn parameter from call_delayed as already
>> mentioned by Luca you can add my:
>
> Oh, please drop my request to delete the conn parameter, as it is being
> used in patch 4 again.
>
>> Reviewed-by: Juergen Gross <jgross@suse.com>
>
> This stands, of course.
Hi Juergen,
I’m sorry but I don’t see when the parameter is used, in patch 4 we have this call:
line 2344:
if (delayed_requests) {
list_for_each_entry(conn, &connections, list) {
struct delayed_request *req, *tmp;
list_for_each_entry_safe(req, tmp,
&conn->delayed, list)
call_delayed(conn, req);
}
}
But call_delayed is still this one:
Line 273:
static void call_delayed(struct connection *conn, struct delayed_request *req)
{
if (req->func(req)) {
undelay_request(req);
talloc_set_destructor(req, NULL);
}
}
Am I missing something?
Cheers,
Luca
>
>
> Juergen
> <OpenPGP_0xB0DE9DD628BF132F.asc>
next prev parent reply other threads:[~2021-06-24 7:57 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-16 14:43 [PATCH 00/10] tools/xenstored: Bug fixes + Improve Live-Update Julien Grall
2021-06-16 14:43 ` [PATCH 01/10] MAINTAINERS: Add myself as reviewers for tools/xenstore Julien Grall
2021-06-16 15:15 ` Juergen Gross
2021-06-16 14:43 ` [PATCH 02/10] tools/xenstored: Introduce lu_get_connection() and use it Julien Grall
2021-06-21 8:21 ` Luca Fancellu
2021-06-24 7:21 ` Juergen Gross
2021-06-16 14:43 ` [PATCH 03/10] tools/xenstore: Don't assume conn->in points to the LU request Julien Grall
2021-06-21 8:55 ` Luca Fancellu
2021-06-24 8:06 ` Julien Grall
2021-06-24 7:32 ` Juergen Gross
2021-06-24 7:34 ` Juergen Gross
2021-06-24 7:56 ` Luca Fancellu [this message]
2021-06-24 8:05 ` Juergen Gross
2021-06-16 14:43 ` [PATCH 04/10] tools/xenstored: Limit the number of requests a connection can delay Julien Grall
2021-06-21 9:02 ` Luca Fancellu
2021-06-24 7:35 ` Juergen Gross
2021-06-16 14:43 ` [PATCH 05/10] tools/xenstored: xenstored_core.h should include fcntl.h Julien Grall
2021-06-21 9:03 ` Luca Fancellu
2021-06-24 7:36 ` Juergen Gross
2021-06-16 14:43 ` [PATCH 06/10] tools/xenstored: Introduce a wrapper for conn->funcs->can_{read, write} Julien Grall
2021-06-21 9:10 ` Luca Fancellu
2021-06-24 7:39 ` Juergen Gross
2021-06-16 14:43 ` [PATCH 07/10] tools/xenstored: delay_request: don't assume conn->in == in Julien Grall
2021-06-21 9:12 ` Luca Fancellu
2021-06-24 7:44 ` Juergen Gross
2021-06-24 7:58 ` Julien Grall
2021-06-16 14:43 ` [PATCH 08/10] tools/xenstored: Extend restore code to handle multiple input buffer Julien Grall
2021-06-21 9:21 ` Luca Fancellu
2021-06-24 8:30 ` Juergen Gross
2021-06-24 8:42 ` Julien Grall
2021-06-24 9:20 ` Juergen Gross
2021-06-16 14:43 ` [PATCH 09/10] tools/xenstored: Dump delayed requests Julien Grall
2021-06-21 9:27 ` Luca Fancellu
2021-06-24 8:41 ` Juergen Gross
2021-06-24 10:28 ` Julien Grall
2021-06-24 10:45 ` Juergen Gross
2021-06-24 10:46 ` Julien Grall
2021-06-24 11:02 ` Juergen Gross
2021-06-24 11:17 ` Julien Grall
2021-06-16 14:43 ` [PATCH 10/10] tools/xenstored: Delay new transaction while Live-Update is pending Julien Grall
2021-06-21 9:30 ` Luca Fancellu
2021-06-24 9:23 ` Juergen Gross
2021-06-24 10:43 ` [PATCH 00/10] tools/xenstored: Bug fixes + Improve Live-Update Julien Grall
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=10F0635C-2AA7-42A4-B4BB-A9DD869113C9@arm.com \
--to=luca.fancellu@arm.com \
--cc=doebel@amazon.de \
--cc=iwj@xenproject.org \
--cc=jgrall@amazon.com \
--cc=jgross@suse.com \
--cc=julien@xen.org \
--cc=raphning@amazon.co.uk \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.