public/io: xs_wire: Document that new errors should be added at the end

[PATCH v3] public/io: xs_wire: Document that new errors should be added at the end

Posted by Julien Grall 3 years, 7 months ago

From: Julien Grall <jgrall@amazon.com>

Some tools (e.g. xenstored) always expect EINVAL to be first in xsd_errors.

To be conservative, mandate that new errors should be added at the end
of the array.

Signed-off-by: Julien Grall <jgrall@amazon.com>

----

Changes in v3:
    - Mandate that new errors should be added at the end.

Changes in v2:
    - New patch
---
 xen/include/public/io/xs_wire.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/include/public/io/xs_wire.h b/xen/include/public/io/xs_wire.h
index c1ec7c73e3b1..a4d954cb05aa 100644
--- a/xen/include/public/io/xs_wire.h
+++ b/xen/include/public/io/xs_wire.h
@@ -76,6 +76,7 @@ static struct xsd_errors xsd_errors[]
 __attribute__((unused))
 #endif
     = {
+    /* /!\ New errors should be added at the end of the array. */
     XSD_ERROR(EINVAL),
     XSD_ERROR(EACCES),
     XSD_ERROR(EEXIST),
-- 
2.32.0

Re: [PATCH v3] public/io: xs_wire: Document that new errors should be added at the end

Posted by Juergen Gross 3 years, 7 months ago

On 30.06.22 20:36, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Some tools (e.g. xenstored) always expect EINVAL to be first in xsd_errors.
> 
> To be conservative, mandate that new errors should be added at the end
> of the array.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

Re: [PATCH v3] public/io: xs_wire: Document that new errors should be added at the end

Posted by Julien Grall 3 years, 7 months ago

Hi Juergen,

On 01/07/2022 06:13, Juergen Gross wrote:
> On 30.06.22 20:36, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> Some tools (e.g. xenstored) always expect EINVAL to be first in 
>> xsd_errors.
>>
>> To be conservative, mandate that new errors should be added at the end
>> of the array.
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> Reviewed-by: Juergen Gross <jgross@suse.com>

Thanks! I have committed the patch.

Cheers,

-- 
Julien Grall

RE: [PATCH v3] public/io: xs_wire: Document that new errors should be added at the end

Posted by Henry Wang 3 years, 7 months ago

Hi Julien,

> -----Original Message-----
> Subject: [PATCH v3] public/io: xs_wire: Document that new errors should be
> added at the end
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> Some tools (e.g. xenstored) always expect EINVAL to be first in xsd_errors.
> 
> To be conservative, mandate that new errors should be added at the end
> of the array.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

While the change is looking good, just FYI there is an "ERROR" section in
/xen/docs/misc/xenstore.txt that points to the io/xs_wire.h. You may
want to add something there (that depends on your own taste :)) )

With or without above additional documentation:
Reviewed-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry

> 
> ----
> 
> Changes in v3:
>     - Mandate that new errors should be added at the end.
> 
> Changes in v2:
>     - New patch
> ---
>  xen/include/public/io/xs_wire.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/xen/include/public/io/xs_wire.h
> b/xen/include/public/io/xs_wire.h
> index c1ec7c73e3b1..a4d954cb05aa 100644
> --- a/xen/include/public/io/xs_wire.h
> +++ b/xen/include/public/io/xs_wire.h
> @@ -76,6 +76,7 @@ static struct xsd_errors xsd_errors[]
>  __attribute__((unused))
>  #endif
>      = {
> +    /* /!\ New errors should be added at the end of the array. */
>      XSD_ERROR(EINVAL),
>      XSD_ERROR(EACCES),
>      XSD_ERROR(EEXIST),
> --
> 2.32.0
>

Re: [PATCH v3] public/io: xs_wire: Document that new errors should be added at the end

Posted by Juergen Gross 3 years, 7 months ago

On 01.07.22 04:19, Henry Wang wrote:
> Hi Julien,
> 
>> -----Original Message-----
>> Subject: [PATCH v3] public/io: xs_wire: Document that new errors should be
>> added at the end
>>
>> From: Julien Grall <jgrall@amazon.com>
>>
>> Some tools (e.g. xenstored) always expect EINVAL to be first in xsd_errors.
>>
>> To be conservative, mandate that new errors should be added at the end
>> of the array.
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> While the change is looking good, just FYI there is an "ERROR" section in
> /xen/docs/misc/xenstore.txt that points to the io/xs_wire.h. You may
> want to add something there (that depends on your own taste :)) )

This documentation is meant for users of Xenstore, not for how to modify
it. I think the comment is all what should be added.


Juergen

RE: [PATCH v3] public/io: xs_wire: Document that new errors should be added at the end

Posted by Henry Wang 3 years, 7 months ago

Hi Juergen,

> -----Original Message-----
> On 01.07.22 04:19, Henry Wang wrote:
> > Hi Julien,
> >
> >> -----Original Message-----
> >> Subject: [PATCH v3] public/io: xs_wire: Document that new errors should
> be
> >> added at the end
> >>
> >> From: Julien Grall <jgrall@amazon.com>
> >>
> >> Some tools (e.g. xenstored) always expect EINVAL to be first in xsd_errors.
> >>
> >> To be conservative, mandate that new errors should be added at the end
> >> of the array.
> >>
> >> Signed-off-by: Julien Grall <jgrall@amazon.com>
> >
> > While the change is looking good, just FYI there is an "ERROR" section in
> > /xen/docs/misc/xenstore.txt that points to the io/xs_wire.h. You may
> > want to add something there (that depends on your own taste :)) )
> 
> This documentation is meant for users of Xenstore, not for how to modify
> it. I think the comment is all what should be added.

Ok sure, thanks for the information. Then I think this patch is perfect.

Kind regards,
Henry

> 
> 
> Juergen