Re: [Xen-devel] [PATCH v2 3/3] livepatch: Sync cache of build-id before using it first time.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.08.16 at 22:53, <konrad.wilk@xxxxxxxxxx> wrote:
> On Fri, Aug 05, 2016 at 09:40:43AM -0600, Jan Beulich wrote:
>> >>> On 04.08.16 at 17:49, <konrad.wilk@xxxxxxxxxx> wrote:
>> > --- a/xen/include/xen/livepatch.h
>> > +++ b/xen/include/xen/livepatch.h
>> > @@ -44,7 +44,7 @@ unsigned long livepatch_symbols_lookup_by_name(const 
>> > char 
> 
>> > *symname);
>> >  bool_t is_patch(const void *addr);
>> >  int xen_build_id_check(const Elf_Note *n, unsigned int n_sz,
>> >                         const void **p, unsigned int *len);
>> > -
>> > +void xen_build_init(void);
>> >  /* Arch hooks. */
>> 
>> Please don't ditch a blank line like this. But this is the wrong header
>> anyway, or else you'd have to make version.c include it (which would
>> again seem odd). And as I now realize that same aspect applies to
>> xen_build_id_check() too - this needs to move into xen/version.h.
> 
> Back in April (feels like eons ago) with
> "xsplice: Stacking build-id dependency checking."
> 
> I have this comment (on v7 of it) - 
> https://lists.xen.org/archives/html/xen-devel/2016-04/msg01492.html: 
> " Moved xen_build_id_check definition to xsplice.h from version.h
>         (and dropping the #include's in version.h)"
> 
> on that patch.
> 
> Digging in the archive, the reason is outlined in:
> https://lists.xen.org/archives/html/xen-devel/2016-04/msg00407.html 
> where:
> 
>        @@ -17,4 +17,7 @@ const char *xen_deny(void);
>       >  #include <xen/types.h>
>       >  int xen_build_id(const void **p, unsigned int *len);
>       >  
>       > +#include <xen/elfstructs.h>
>       > +int xen_build_id_check(const Elf_Note *n, const void **p, unsigned 
> int *len);
> 
>       The #include is misplaced again, and I'm rather hesitant to see
>       version.h gain this dependency. Couldn't this go into xen/elf.h?
> 
> And I somehow made it go in version.h not elf.h (I believe that was due
> to the Elf_Note being a macro - that is dependent on ELF_SIZE - which
> was not set in elf.h - and would cause compilation issues).
> 
> Anyhow if I move those two in version.h:
> 
> In file included from console.c:13:0:
> /home/konrad/ssd/konrad/xtt-x86_64/bootstrap/xen.git/xen/include/xen/version
> .h:19:30: error: unknown type name ‘Elf_Note’
>  int xen_build_id_check(const Elf_Note *n, unsigned int n_sz,
>                               ^~~~~~~~
> make[4]: *** [console.o] Error 1
> make[3]: *** [char/built_in.o] Error 2
> make[3]: *** Waiting for unfinished jobs....
> In file included from kernel.c:10:0:
> /home/konrad/ssd/konrad/xtt-x86_64/bootstrap/xen.git/xen/include/xen/version
> .h:19:30: error: unknown type name ‘Elf_Note’
>  int xen_build_id_check(const Elf_Note *n, unsigned int n_sz,
>                               ^~~~~~~~
> make[3]: *** [kernel.o] Error 1
> make[2]: *** 
> [/home/konrad/ssd/konrad/xtt-x86_64/bootstrap/xen.git/xen/common/built_in.o] 
> Err
> 
> 
> I belive I have to include the #include <xen/elfstructs.h> to deal with 
> Elf_Note macro.

That would be fine - that old comment of mine wasn't about dropping
the include, but moving it up in the file.

> P.S.
> This is the patch (copy-n-paste):

But this variant (which appears to get along without the #include) looks
fine too.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel