From: Julien Grall <jgrall@amazon.com>

Currently, only Live-Update request can be delayed. In a follow-up,
we will want to delay more requests (e.g. transaction start).
Therefore we want to preserve delayed requests across Live-Update.

Delayed requests are just complete "in" buffer. So the code is
refactored to allow sharing the code to dump "in" buffer.

Signed-off-by: Julien Grall <jgrall@amazon.com>
---
 tools/xenstore/xenstored_core.c | 42 +++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 5b7ab7f74013..9eca58682b51 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -2403,25 +2403,47 @@ const char *dump_state_global(FILE *fp)
 	return NULL;
 }
 
+static const char *dump_input_buffered_data(FILE *fp,
+					    const struct buffered_data *in,
+					    unsigned int *total_len)
+{
+	unsigned int hlen = in->inhdr ? in->used : sizeof(in->hdr);
+
+	*total_len += hlen;
+	if (fp && fwrite(&in->hdr, hlen, 1, fp) != 1)
+		return "Dump read data error";
+	if (!in->inhdr && in->used) {
+		*total_len += in->used;
+		if (fp && fwrite(in->buffer, in->used, 1, fp) != 1)
+			return "Dump read data error";
+	}
+
+	return NULL;
+}
+
 /* Called twice: first with fp == NULL to get length, then for writing data. */
 const char *dump_state_buffered_data(FILE *fp, const struct connection *c,
 				     struct xs_state_connection *sc)
 {
 	unsigned int len = 0, used;
-	struct buffered_data *out, *in = c->in;
+	struct buffered_data *out;
 	bool partial = true;
+	struct delayed_request *req;
+	const char *ret;
 
-	if (in) {
-		len = in->inhdr ? in->used : sizeof(in->hdr);
-		if (fp && fwrite(&in->hdr, len, 1, fp) != 1)
-			return "Dump read data error";
-		if (!in->inhdr && in->used) {
-			len += in->used;
-			if (fp && fwrite(in->buffer, in->used, 1, fp) != 1)
-				return "Dump read data error";
-		}
+	/* Dump any command that was delayed */
+	list_for_each_entry(req, &c->delayed, list) {
+		if (req->func != process_delayed_message)
+			continue;
+
+		assert(!req->in->inhdr);
+		if ((ret = dump_input_buffered_data(fp, req->in, &len)))
+			return ret;
 	}
 
+	if (c->in && (ret = dump_input_buffered_data(fp, c->in, &len)))
+		return ret;
+
 	if (sc) {
 		sc->data_in_len = len;
 		sc->data_resp_len = 0;
-- 
2.17.1

> On 16 Jun 2021, at 15:43, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> Currently, only Live-Update request can be delayed. In a follow-up,
> we will want to delay more requests (e.g. transaction start).
> Therefore we want to preserve delayed requests across Live-Update.
> 
> Delayed requests are just complete "in" buffer. So the code is
> refactored to allow sharing the code to dump "in" buffer.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

> ---
> tools/xenstore/xenstored_core.c | 42 +++++++++++++++++++++++++--------
> 1 file changed, 32 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index 5b7ab7f74013..9eca58682b51 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -2403,25 +2403,47 @@ const char *dump_state_global(FILE *fp)
> 	return NULL;
> }
> 
> +static const char *dump_input_buffered_data(FILE *fp,
> +					    const struct buffered_data *in,
> +					    unsigned int *total_len)
> +{
> +	unsigned int hlen = in->inhdr ? in->used : sizeof(in->hdr);
> +
> +	*total_len += hlen;
> +	if (fp && fwrite(&in->hdr, hlen, 1, fp) != 1)
> +		return "Dump read data error";
> +	if (!in->inhdr && in->used) {
> +		*total_len += in->used;
> +		if (fp && fwrite(in->buffer, in->used, 1, fp) != 1)
> +			return "Dump read data error";
> +	}
> +
> +	return NULL;
> +}
> +
> /* Called twice: first with fp == NULL to get length, then for writing data. */
> const char *dump_state_buffered_data(FILE *fp, const struct connection *c,
> 				     struct xs_state_connection *sc)
> {
> 	unsigned int len = 0, used;
> -	struct buffered_data *out, *in = c->in;
> +	struct buffered_data *out;
> 	bool partial = true;
> +	struct delayed_request *req;
> +	const char *ret;
> 
> -	if (in) {
> -		len = in->inhdr ? in->used : sizeof(in->hdr);
> -		if (fp && fwrite(&in->hdr, len, 1, fp) != 1)
> -			return "Dump read data error";
> -		if (!in->inhdr && in->used) {
> -			len += in->used;
> -			if (fp && fwrite(in->buffer, in->used, 1, fp) != 1)
> -				return "Dump read data error";
> -		}
> +	/* Dump any command that was delayed */
> +	list_for_each_entry(req, &c->delayed, list) {
> +		if (req->func != process_delayed_message)
> +			continue;
> +
> +		assert(!req->in->inhdr);
> +		if ((ret = dump_input_buffered_data(fp, req->in, &len)))
> +			return ret;
> 	}
> 
> +	if (c->in && (ret = dump_input_buffered_data(fp, c->in, &len)))
> +		return ret;
> +
> 	if (sc) {
> 		sc->data_in_len = len;
> 		sc->data_resp_len = 0;
> -- 
> 2.17.1
> 
> 

On 16.06.21 16:43, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Currently, only Live-Update request can be delayed. In a follow-up,
> we will want to delay more requests (e.g. transaction start).
> Therefore we want to preserve delayed requests across Live-Update.
> 
> Delayed requests are just complete "in" buffer. So the code is
> refactored to allow sharing the code to dump "in" buffer.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> ---
>   tools/xenstore/xenstored_core.c | 42 +++++++++++++++++++++++++--------
>   1 file changed, 32 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index 5b7ab7f74013..9eca58682b51 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -2403,25 +2403,47 @@ const char *dump_state_global(FILE *fp)
>   	return NULL;
>   }
>   
> +static const char *dump_input_buffered_data(FILE *fp,
> +					    const struct buffered_data *in,
> +					    unsigned int *total_len)
> +{
> +	unsigned int hlen = in->inhdr ? in->used : sizeof(in->hdr);
> +
> +	*total_len += hlen;
> +	if (fp && fwrite(&in->hdr, hlen, 1, fp) != 1)
> +		return "Dump read data error";
> +	if (!in->inhdr && in->used) {
> +		*total_len += in->used;
> +		if (fp && fwrite(in->buffer, in->used, 1, fp) != 1)
> +			return "Dump read data error";
> +	}
> +
> +	return NULL;
> +}
> +
>   /* Called twice: first with fp == NULL to get length, then for writing data. */
>   const char *dump_state_buffered_data(FILE *fp, const struct connection *c,
>   				     struct xs_state_connection *sc)
>   {
>   	unsigned int len = 0, used;
> -	struct buffered_data *out, *in = c->in;
> +	struct buffered_data *out;
>   	bool partial = true;
> +	struct delayed_request *req;
> +	const char *ret;
>   
> -	if (in) {
> -		len = in->inhdr ? in->used : sizeof(in->hdr);
> -		if (fp && fwrite(&in->hdr, len, 1, fp) != 1)
> -			return "Dump read data error";
> -		if (!in->inhdr && in->used) {
> -			len += in->used;
> -			if (fp && fwrite(in->buffer, in->used, 1, fp) != 1)
> -				return "Dump read data error";
> -		}
> +	/* Dump any command that was delayed */
> +	list_for_each_entry(req, &c->delayed, list) {

Please add a comment here that the following if() serves especially to
avoid dumping the data for do_lu_start().

> +		if (req->func != process_delayed_message)
> +			continue;
> +
> +		assert(!req->in->inhdr);
> +		if ((ret = dump_input_buffered_data(fp, req->in, &len)))
> +			return ret;
>   	}
>   
> +	if (c->in && (ret = dump_input_buffered_data(fp, c->in, &len)))
> +		return ret;
> +
>   	if (sc) {
>   		sc->data_in_len = len;
>   		sc->data_resp_len = 0;
> 

Juergen

Hi Juergen,

On 24/06/2021 10:41, Juergen Gross wrote:
> On 16.06.21 16:43, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> Currently, only Live-Update request can be delayed. In a follow-up,
>> we will want to delay more requests (e.g. transaction start).
>> Therefore we want to preserve delayed requests across Live-Update.
>>
>> Delayed requests are just complete "in" buffer. So the code is
>> refactored to allow sharing the code to dump "in" buffer.
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>> ---
>>   tools/xenstore/xenstored_core.c | 42 +++++++++++++++++++++++++--------
>>   1 file changed, 32 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/xenstore/xenstored_core.c 
>> b/tools/xenstore/xenstored_core.c
>> index 5b7ab7f74013..9eca58682b51 100644
>> --- a/tools/xenstore/xenstored_core.c
>> +++ b/tools/xenstore/xenstored_core.c
>> @@ -2403,25 +2403,47 @@ const char *dump_state_global(FILE *fp)
>>       return NULL;
>>   }
>> +static const char *dump_input_buffered_data(FILE *fp,
>> +                        const struct buffered_data *in,
>> +                        unsigned int *total_len)
>> +{
>> +    unsigned int hlen = in->inhdr ? in->used : sizeof(in->hdr);
>> +
>> +    *total_len += hlen;
>> +    if (fp && fwrite(&in->hdr, hlen, 1, fp) != 1)
>> +        return "Dump read data error";
>> +    if (!in->inhdr && in->used) {
>> +        *total_len += in->used;
>> +        if (fp && fwrite(in->buffer, in->used, 1, fp) != 1)
>> +            return "Dump read data error";
>> +    }
>> +
>> +    return NULL;
>> +}
>> +
>>   /* Called twice: first with fp == NULL to get length, then for 
>> writing data. */
>>   const char *dump_state_buffered_data(FILE *fp, const struct 
>> connection *c,
>>                        struct xs_state_connection *sc)
>>   {
>>       unsigned int len = 0, used;
>> -    struct buffered_data *out, *in = c->in;
>> +    struct buffered_data *out;
>>       bool partial = true;
>> +    struct delayed_request *req;
>> +    const char *ret;
>> -    if (in) {
>> -        len = in->inhdr ? in->used : sizeof(in->hdr);
>> -        if (fp && fwrite(&in->hdr, len, 1, fp) != 1)
>> -            return "Dump read data error";
>> -        if (!in->inhdr && in->used) {
>> -            len += in->used;
>> -            if (fp && fwrite(in->buffer, in->used, 1, fp) != 1)
>> -                return "Dump read data error";
>> -        }
>> +    /* Dump any command that was delayed */
>> +    list_for_each_entry(req, &c->delayed, list) {
> 
> Please add a comment here that the following if() serves especially to
> avoid dumping the data for do_lu_start().

Would you be happy to give your acked-by/reviewed-by if I add the 
following on commit?

"
We only want to preserve commands that wasn't processed at all. All the 
other delayed requests (such as do_lu_start()) must be processed before 
Live-Update.
"

> 
>> +        if (req->func != process_delayed_message)
>> +            continue;
>> +
>> +        assert(!req->in->inhdr);
>> +        if ((ret = dump_input_buffered_data(fp, req->in, &len)))
>> +            return ret;
>>       }
>> +    if (c->in && (ret = dump_input_buffered_data(fp, c->in, &len)))
>> +        return ret;
>> +
>>       if (sc) {
>>           sc->data_in_len = len;
>>           sc->data_resp_len = 0;
>>
> 
> Juergen

Cheers,

-- 
Julien Grall

On 24.06.21 12:28, Julien Grall wrote:
> Hi Juergen,
> 
> On 24/06/2021 10:41, Juergen Gross wrote:
>> On 16.06.21 16:43, Julien Grall wrote:
>>> From: Julien Grall <jgrall@amazon.com>
>>>
>>> Currently, only Live-Update request can be delayed. In a follow-up,
>>> we will want to delay more requests (e.g. transaction start).
>>> Therefore we want to preserve delayed requests across Live-Update.
>>>
>>> Delayed requests are just complete "in" buffer. So the code is
>>> refactored to allow sharing the code to dump "in" buffer.
>>>
>>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>>> ---
>>>   tools/xenstore/xenstored_core.c | 42 +++++++++++++++++++++++++--------
>>>   1 file changed, 32 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/tools/xenstore/xenstored_core.c 
>>> b/tools/xenstore/xenstored_core.c
>>> index 5b7ab7f74013..9eca58682b51 100644
>>> --- a/tools/xenstore/xenstored_core.c
>>> +++ b/tools/xenstore/xenstored_core.c
>>> @@ -2403,25 +2403,47 @@ const char *dump_state_global(FILE *fp)
>>>       return NULL;
>>>   }
>>> +static const char *dump_input_buffered_data(FILE *fp,
>>> +                        conststruct buffered_data *in,
>>> +                        unsigned int *total_len)
>>> +{
>>> +    unsigned int hlen = in->inhdr ? in->used : sizeof(in->hdr);
>>> +
>>> +    *total_len += hlen;
>>> +    if (fp && fwrite(&in->hdr, hlen, 1, fp) != 1)
>>> +        return "Dump read data error";
>>> +    if (!in->inhdr && in->used) {
>>> +        *total_len += in->used;
>>> +        if (fp && fwrite(in->buffer,in->used, 1, fp) != 1)
>>> +            return "Dump read data error";
>>> +    }
>>> +
>>> +    return NULL;
>>> +}
>>> +
>>>   /* Called twice: first with fp == NULL to get length, then 
for 
>>> writing data. */
>>>   const char *dump_state_buffered_data(FILE *fp, const struct 
>>> connection *c,
>>>                        struct xs_state_connection *sc)
>>>   {
>>>       unsigned int len = 0, used;
>>> -    struct buffered_data *out, *in = c->in;
>>> +    struct buffered_data *out;
>>>       bool partial = true;
>>> +    struct delayed_request *req;
>>> +    const char *ret;
>>> -    if (in) {
>>> -        len = in->inhdr ? in->used: sizeof(in->hdr);
>>> -        if (fp && fwrite(&in->hdr, len, 1, fp) != 1)
>>> -            return "Dump read data error";
>>> -        if (!in->inhdr && in->used) {
>>> -            len += in->used;
>>> -            if (fp && fwrite(in->buffer, in->used, 1, fp) != 1)
>>> -                return "Dump read data error";
>>> -        }
>>> +    /* Dump any command that was delayed */
>>> +    list_for_each_entry(req, &c->delayed, list) {
>>
>> Please add a comment here that the following if() serves especially to
>> avoid dumping the data for do_lu_start().
> 
> Would you be happy to give your acked-by/reviewed-by if I add the 
> following on commit?
> 
> "
> We only want to preserve commands that wasn't processed at all. All the 


s/wasn't/weren't/

> other delayed requests (such as do_lu_start()) must be processed before 

> Live-Update.
> "

With that change I'm fine.


Juergen

Hi Juergen,

On 24/06/2021 12:45, Juergen Gross wrote:
> On 24.06.21 12:28, Julien Grall wrote:
>> Hi Juergen,
>>
>> On 24/06/2021 10:41, Juergen Gross wrote:
>>> On 16.06.21 16:43, Julien Grall wrote:
>>>> From: Julien Grall <jgrall@amazon.com>
>>>>
>>>> Currently, only Live-Update request can be delayed. In a follow-up,
>>>> we will want to delay more requests (e.g. transaction start).
>>>> Therefore we want to preserve delayed requests across Live-Update.
>>>>
>>>> Delayed requests are just complete "in" buffer. So the code is
>>>> refactored to allow sharing the code to dump "in" buffer.
>>>>
>>>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>>>> ---
>>>>   tools/xenstore/xenstored_core.c | 42 
>>>> +++++++++++++++++++++++++--------
>>>>   1 file changed, 32 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/tools/xenstore/xenstored_core.c 
>>>> b/tools/xenstore/xenstored_core.c
>>>> index 5b7ab7f74013..9eca58682b51 100644
>>>> --- a/tools/xenstore/xenstored_core.c
>>>> +++ b/tools/xenstore/xenstored_core.c
>>>> @@ -2403,25 +2403,47 @@ const char *dump_state_global(FILE *fp)
>>>>       return NULL;
>>>>   }
>>>> +static const char *dump_input_buffered_data(FILE *fp,
>>>> +                        conststruct buffered_data *in,
>>>> +                        unsigned int *total_len)
>>>> +{
>>>> +    unsigned int hlen = in->inhdr ? in->used : sizeof(in->hdr);
>>>> +
>>>> +    *total_len += hlen;
>>>> +    if (fp && fwrite(&in->hdr, hlen, 1, fp) != 1)
>>>> +        return "Dump read data error";
>>>> +    if (!in->inhdr && in->used) {
>>>> +        *total_len += in->used;
>>>> +        if (fp && fwrite(in->buffer,in->used, 1, fp) != 1)
>>>> +            return "Dump read data error";
>>>> +    }
>>>> +
>>>> +    return NULL;
>>>> +}
>>>> +
>>>>   /* Called twice: first with fp == NULL to get length, then 
> for
>>>> writing data. */
>>>>   const char *dump_state_buffered_data(FILE *fp, const struct 
>>>> connection *c,
>>>>                        struct xs_state_connection *sc)
>>>>   {
>>>>       unsigned int len = 0, used;
>>>> -    struct buffered_data *out, *in = c->in;
>>>> +    struct buffered_data *out;
>>>>       bool partial = true;
>>>> +    struct delayed_request *req;
>>>> +    const char *ret;
>>>> -    if (in) {
>>>> -        len = in->inhdr ? in->used: sizeof(in->hdr);
>>>> -        if (fp && fwrite(&in->hdr, len, 1, fp) != 1)
>>>> -            return "Dump read data error";
>>>> -        if (!in->inhdr && in->used) {
>>>> -            len += in->used;
>>>> -            if (fp && fwrite(in->buffer, in->used, 1, fp) != 1)
>>>> -                return "Dump read data error";
>>>> -        }
>>>> +    /* Dump any command that was delayed */
>>>> +    list_for_each_entry(req, &c->delayed, list) {
>>>
>>> Please add a comment here that the following if() serves especially to
>>> avoid dumping the data for do_lu_start().
>>
>> Would you be happy to give your acked-by/reviewed-by if I add the 
>> following on commit?
>>
>> "
>> We only want to preserve commands that wasn't processed at all. All the 
> 
> 
> s/wasn't/weren't/

I will do.

> 
>> other delayed requests (such as do_lu_start()) must be processed before 
> 
>> Live-Update.
>> "
> 
> With that change I'm fine.

Can I translate that to an acked-by? :)

Cheers,

-- 
Julien Grall

On 24.06.21 12:46, Julien Grall wrote:
> Hi Juergen,
> 
> On 24/06/2021 12:45, Juergen Gross wrote:
>> On 24.06.21 12:28, Julien Grall wrote:
>>> Hi Juergen,
>>>
>>> On 24/06/2021 10:41, Juergen Gross wrote:
>>>> On 16.06.21 16:43, Julien Grall wrote:
>>>>> From: Julien Grall <jgrall@amazon.com>
>>>>>
>>>>> Currently, only Live-Update request can be delayed. In a follow-up,
>>>>> we will want to delay more requests (e.g. transaction start).
>>>>> Therefore we want to preserve delayed requests across Live-Update.
>>>>>
>>>>> Delayed requests are just complete "in" buffer. So the code is
>>>>> refactored to allow sharing the code to dump "in" buffer.
>>>>>
>>>>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>>>>> ---
>>>>>   tools/xenstore/xenstored_core.c | 42 
>>>>> +++++++++++++++++++++++++--------
>>>>>   1 file changed, 32 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/tools/xenstore/xenstored_core.c 
>>>>> b/tools/xenstore/xenstored_core.c
>>>>> index 5b7ab7f74013..9eca58682b51 100644
>>>>> --- a/tools/xenstore/xenstored_core.c
>>>>> +++ b/tools/xenstore/xenstored_core.c
>>>>> @@ -2403,25 +2403,47 @@ const char *dump_state_global(FILE *fp)
>>>>>       return NULL;
>>>>>   }
>>>>> +static const char *dump_input_buffered_data(FILE *fp,
>>>>> +                        conststruct buffered_data *in,
>>>>> +                        unsigned int *total_len)
>>>>> +{
>>>>> +    unsigned int hlen = in->inhdr ? in->used : sizeof(in->hdr);
>>>>> +
>>>>> +    *total_len += hlen;
>>>>> +    if (fp && fwrite(&in->hdr, hlen, 1, fp) != 1)
>>>>> +        return "Dump read data error";
>>>>> +    if (!in->inhdr && in->used) {
>>>>> +        *total_len += in->used;
>>>>> +        if (fp && fwrite(in->buffer,in->used, 1, fp) != 1)
>>>>> +            
return "Dump read data error";
>>>>> +    }
>>>>> +
>>>>> +    return NULL;
>>>>> +}
>>>>> +
>>>>>   /* Called twice: first with fp == NULL to get length, then 
>> for
>>>>> writing data. */
>>>>>   const char *dump_state_buffered_data(FILE *fp, const struct 
>>>>> connection *c,
>>>>>                        struct xs_state_connection *sc)
>>>>>   {
>>>>>       unsigned int len = 0, used;
>>>>> -    struct buffered_data *out, *in = c->in;
>>>>> +    struct buffered_data *out;
>>>>>       bool partial = true;
>>>>> +    struct delayed_request *req;
>>>>> +    const char *ret;
>>>>> -    if (in) {
>>>>> -        len = in->inhdr ? in->used: sizeof(in->hdr);
>>>>> -        if (fp && fwrite(&in->hdr,len, 1, fp) != 1)
>>>>> -            
return "Dump read data error";
>>>>> -        if (!in->inhdr && in->used) {
>>>>> -            
len += in->used;
>>>>> -            
if(fp && fwrite(in->buffer, in->used, 1, fp) != 1)
>>>>> -                return "Dump read data error";
>>>>> -        }
>>>>> +    /* Dump any command that was delayed */
>>>>> +    list_for_each_entry(req, &c->delayed, list) {
>>>>
>>>> Please add a comment here that the following if() serves especially to
>>>> avoid dumping the data for do_lu_start().
>>>
>>> Would you be happy to give your acked-by/reviewed-by if I add the 
>>> following on commit?
>>>
>>> "
>>> We only want to preserve commands that wasn't processed at all. All the 
>>
>>
>> s/wasn't/weren't/
> 
> I will do.
> 
>>
>>> other delayed requests (such as do_lu_start()) must be processed before 
>>
>>> Live-Update.
>>> "
>>
>> With that change I'm fine.
> 
> Can I translate that to an acked-by? :)

Make it a "Reviewed-by:" :-)


Juergen

On 24/06/2021 13:02, Juergen Gross wrote:
> On 24.06.21 12:46, Julien Grall wrote:
>> Hi Juergen,
>>
>> On 24/06/2021 12:45, Juergen Gross wrote:
>>> On 24.06.21 12:28, Julien Grall wrote:
>>>> Hi Juergen,
>>>>
>>>> On 24/06/2021 10:41, Juergen Gross wrote:
>>>>> On 16.06.21 16:43, Julien Grall wrote:
>>>>>> From: Julien Grall <jgrall@amazon.com>
>>>>>>
>>>>>> Currently, only Live-Update request can be delayed. In a follow-up,
>>>>>> we will want to delay more requests (e.g. transaction start).
>>>>>> Therefore we want to preserve delayed requests across Live-Update.
>>>>>>
>>>>>> Delayed requests are just complete "in" buffer. So the code is
>>>>>> refactored to allow sharing the code to dump "in" buffer.
>>>>>>
>>>>>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>>>>>> ---
>>>>>>   tools/xenstore/xenstored_core.c | 42 
>>>>>> +++++++++++++++++++++++++--------
>>>>>>   1 file changed, 32 insertions(+), 10 deletions(-)
>>>>>>
>>>>>> diff --git a/tools/xenstore/xenstored_core.c 
>>>>>> b/tools/xenstore/xenstored_core.c
>>>>>> index 5b7ab7f74013..9eca58682b51 100644
>>>>>> --- a/tools/xenstore/xenstored_core.c
>>>>>> +++ b/tools/xenstore/xenstored_core.c
>>>>>> @@ -2403,25 +2403,47 @@ const char *dump_state_global(FILE *fp)
>>>>>>       return NULL;
>>>>>>   }
>>>>>> +static const char *dump_input_buffered_data(FILE *fp,
>>>>>> +                        conststruct buffered_data *in,
>>>>>> +                        unsigned int *total_len)
>>>>>> +{
>>>>>> +    unsigned int hlen = in->inhdr ? in->used : sizeof(in->hdr);
>>>>>> +
>>>>>> +    *total_len += hlen;
>>>>>> +    if (fp && fwrite(&in->hdr, hlen, 1, fp) != 1)
>>>>>> +        return "Dump read data error";
>>>>>> +    if (!in->inhdr && in->used) {
>>>>>> +        *total_len += in->used;
>>>>>> +        if (fp && fwrite(in->buffer,in->used, 1, fp) != 1)
>>>>>> + 
> return "Dump read data error";
>>>>>> +    }
>>>>>> +
>>>>>> +    return NULL;
>>>>>> +}
>>>>>> +
>>>>>>   /* Called twice: first with fp == NULL to get length, then 
>>> for
>>>>>> writing data. */
>>>>>>   const char *dump_state_buffered_data(FILE *fp, const struct 
>>>>>> connection *c,
>>>>>>                        struct xs_state_connection *sc)
>>>>>>   {
>>>>>>       unsigned int len = 0, used;
>>>>>> -    struct buffered_data *out, *in = c->in;
>>>>>> +    struct buffered_data *out;
>>>>>>       bool partial = true;
>>>>>> +    struct delayed_request *req;
>>>>>> +    const char *ret;
>>>>>> -    if (in) {
>>>>>> -        len = in->inhdr ? in->used: sizeof(in->hdr);
>>>>>> -        if (fp && fwrite(&in->hdr,len, 1, fp) != 1)
>>>>>> - 
> return "Dump read data error";
>>>>>> -        if (!in->inhdr && in->used) {
>>>>>> - 
> len += in->used;
>>>>>> - 
> if(fp && fwrite(in->buffer, in->used, 1, fp) != 1)
>>>>>> -                return "Dump read data error";
>>>>>> -        }
>>>>>> +    /* Dump any command that was delayed */
>>>>>> +    list_for_each_entry(req, &c->delayed, list) {
>>>>>
>>>>> Please add a comment here that the following if() serves especially to
>>>>> avoid dumping the data for do_lu_start().
>>>>
>>>> Would you be happy to give your acked-by/reviewed-by if I add the 
>>>> following on commit?
>>>>
>>>> "
>>>> We only want to preserve commands that wasn't processed at all. All the 
>>>
>>>
>>> s/wasn't/weren't/
>>
>> I will do.
>>
>>>
>>>> other delayed requests (such as do_lu_start()) must be processed before 
>>>
>>>> Live-Update.
>>>> "
>>>
>>> With that change I'm fine.
>>
>> Can I translate that to an acked-by? :)
> 
> Make it a "Reviewed-by:" :-)

Thanks! I have committed it.

Cheers,

-- 
Julien Grall