On Thu, Jun 17, 2021 at 02:08:17PM +0200, Philippe Mathieu-Daudé wrote: > On 6/17/21 11:35 AM, Daniel P. Berrangé wrote: > > On Wed, Jun 16, 2021 at 06:22:25PM +0200, Philippe Mathieu-Daudé wrote: > >> Code consuming the "crypto/tlscreds*.h" APIs doesn't need > >> to access its internals. Move the structure definitions to > >> the "tlscredspriv.h" private header (only accessible by > >> implementations). The public headers (in include/) still > >> forward-declare the structures typedef. > >> > >> This solves a bug introduced by commit 7de2e856533 which made > >> migration/qemu-file-channel.c include "io/channel-tls.h", > >> itself sometime depends on GNUTLS, leading to build failure > >> on OSX: > >> > >> [2/35] Compiling C object > >> libmigration.fa.p/migration_qemu-file-channel.c.o > >> FAILED: libmigration.fa.p/migration_qemu-file-channel.c.o > >> cc -Ilibmigration.fa.p -I. -I.. -Iqapi [ ... ] -o > >> libmigration.fa.p/migration_qemu-file-channel.c.o -c > >> ../migration/qemu-file-channel.c > >> In file included from ../migration/qemu-file-channel.c:29: > >> In file included from include/io/channel-tls.h:26: > >> In file included from include/crypto/tlssession.h:24: > >> include/crypto/tlscreds.h:28:10: fatal error: 'gnutls/gnutls.h' file not > >> found > >> #include <gnutls/gnutls.h> > >> ^~~~~~~~~~~~~~~~~ > >> 1 error generated. > >> > >> Reported-by: Stefan Weil <s...@weilnetz.de> > >> Suggested-by: Daniel P. Berrangé <berra...@redhat.com> > >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/407 > >> Fixes: 7de2e856533 ("yank: Unregister function when using TLS migration") > >> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> > >> --- > >> crypto/tlscredspriv.h | 45 ++++++++++++++++++++++++++++++ > >> include/crypto/tls-cipher-suites.h | 6 ---- > >> include/crypto/tlscreds.h | 16 ----------- > >> include/crypto/tlscredsanon.h | 12 -------- > >> include/crypto/tlscredspsk.h | 12 -------- > >> include/crypto/tlscredsx509.h | 10 ------- > >> crypto/tls-cipher-suites.c | 7 +++++ > >> crypto/tlscredsanon.c | 2 ++ > >> crypto/tlscredspsk.c | 2 ++ > >> crypto/tlscredsx509.c | 1 + > >> crypto/tlssession.c | 1 + > >> 11 files changed, 58 insertions(+), 56 deletions(-) > > > I was expecting all these files, and tlscreds.c to include > > tlscredspriv.h, otherwise how do they see the struct > > definition they need ? > > They already include it: > > $ git grep tlscredspriv.h origin/master > origin/master:crypto/tlscreds.c:24:#include "tlscredspriv.h" > origin/master:crypto/tlscredsanon.c:23:#include "tlscredspriv.h" > origin/master:crypto/tlscredspsk.c:23:#include "tlscredspriv.h" > origin/master:crypto/tlscredsx509.c:23:#include "tlscredspriv.h" > > This is why in this patch I only added it to tls-cipher-suites.c > and tlssession.c. > > I'll add a comment about it.
No matter, I'm just being dumb and not remembering my own code. I thought tlscredspriv.h was a new header in your patch, but its just adding to an existing header. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
