All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Julien Grall <julien@xen.org>
To: Juergen Gross <jgross@suse.com>, xen-devel@lists.xenproject.org
Cc: 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 08/10] tools/xenstored: Extend restore code to handle multiple input buffer
Date: Thu, 24 Jun 2021 10:42:17 +0200	[thread overview]
Message-ID: <c6ad91cd-f531-5c19-e246-bb8c4ce304f2@xen.org> (raw)
In-Reply-To: <444688a1-180f-7cf3-3461-0001030471d4@suse.com>

Hi Juergen,

On 24/06/2021 10:30, Juergen Gross wrote:
> On 16.06.21 16:43, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> Currently, the restore code is considering the stream will contain at
>> most one in-flight request per connection. In a follow-up changes, we
>> will want to transfer multiple in-flight requests.
>>
>> The function read_state_buffered() is now extended to restore multiple
>> in-flight request. Complete requests will be queued as delayed
>> requests, if there a partial request (only the last one can) then it
>> will used as the current in-flight request.
>>
>> Note that we want to bypass the quota check for delayed requests as
>> the new Xenstore may have a lower limit.
>>
>> Lastly, there is no need to change the specification as there was
>> no restriction on the number of in-flight requests preserved.
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>> ---
>>   tools/xenstore/xenstored_core.c | 56 ++++++++++++++++++++++++++++-----
>>   1 file changed, 48 insertions(+), 8 deletions(-)
>>
>> diff --git a/tools/xenstore/xenstored_core.c 
>> b/tools/xenstore/xenstored_core.c
>> index a5084a5b173d..5b7ab7f74013 100644
>> --- a/tools/xenstore/xenstored_core.c
>> +++ b/tools/xenstore/xenstored_core.c
>> @@ -1486,6 +1486,10 @@ static void process_message(struct connection 
>> *conn, struct buffered_data *in)
>>       enum xsd_sockmsg_type type = in->hdr.msg.type;
>>       int ret;
>> +    /* At least send_error() and send_reply() expects conn->in == in */
>> +    assert(conn->in == in);
>> +    trace_io(conn, in, 0);
>> +
>>       if ((unsigned int)type >= XS_TYPE_COUNT || !wire_funcs[type].func) 
> {
>>           eprintf("Client unknown operation %i", type);
>>           send_error(conn, ENOSYS);
>> @@ -1515,6 +1519,23 @@ static void process_message(struct connection 
>> *conn, struct buffered_data *in)
>>       conn->transaction = NULL;
>>   }
>> +static bool process_delayed_message(struct delayed_request *req)
>> +{
>> +    struct connection *conn = req->data;
>> +    struct buffered_data *saved_in = conn->in;
>> +
>> +    /*
>> +     * Part of process_message() expects conn->in to contains the
>> +     * processed response. So save the current conn->in and restore it
>> +     * afterwards.
>> +     */
>> +    conn->in = req->in;
>> +    process_message(req->data, req->in);
>> +    conn->in = saved_in;
> 
> This pattern was added to do_lu_start() already, and it will be needed
> for each other function being called via call_delayed().
> 
> Maybe it would be better to add conn explicitly to struct
> delayed_request (or even better: replace data with conn) and to do the
> conn->in saving/setting/restoring in call_delayed() instead?

This was my original approach, but I abandoned it because in the case of 
do_lu_start() we need to save the original conn->in in the stream (see 
patch #3 for more details).

If we overwrite conn->in in call_delayed(), then we need a different way 
to find the original conn->in. I couldn't find a nice way and decided to 
settle with the duplication.

Cheers,

-- 
Julien Grall


  reply	other threads:[~2021-06-24  8:42 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
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 [this message]
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=c6ad91cd-f531-5c19-e246-bb8c4ce304f2@xen.org \
    --to=julien@xen.org \
    --cc=doebel@amazon.de \
    --cc=iwj@xenproject.org \
    --cc=jgrall@amazon.com \
    --cc=jgross@suse.com \
    --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.