On 07/17/2014 10:57 AM, Roman Bogorodskiy wrote:
> Roman Bogorodskiy wrote:
>
>> Looks like this breaks build with clang:
>>
>> gmake[3]: Entering directory `/usr/home/novel/code/libvirt/src'
>> CC util/libvirt_util_la-virclosecallbacks.lo
>> In file included from util/virclosecallbacks.c:28:
>> In file included from ../src/util/virclosecallbacks.h:28:
>> ../src/conf/domain_conf.h:70:35: error: redefinition of typedef 'virDomainNumatune' is a C11 feature [-Werror,-Wtypedef-redefinition]
>> typedef struct _virDomainNumatune virDomainNumatune;
>> ^
>> ../src/conf/numatune_conf.h:43:35: note: previous definition is here
>> typedef struct _virDomainNumatune virDomainNumatune;
>> ^
>
> I got it fixed by the following diff:
>
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 4c9b7e8..e4d7988 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -67,9 +67,6 @@ typedef virDomainFSDef *virDomainFSDefPtr;
> typedef struct _virDomainNetDef virDomainNetDef;
> typedef virDomainNetDef *virDomainNetDefPtr;
>
> -typedef struct _virDomainNumatune virDomainNumatune;
> -typedef virDomainNumatune *virDomainNumatunePtr;
> -
> typedef struct _virDomainInputDef virDomainInputDef;
> typedef virDomainInputDef *virDomainInputDefPtr;
ACK to this hunk.
>
> @@ -1854,8 +1851,6 @@ struct _virDomainResourceDef {
> * NB: if adding to this struct, virDomainDefCheckABIStability
> * may well need an update
> */
> -typedef struct _virDomainDef virDomainDef;
> -typedef virDomainDef *virDomainDefPtr;
> struct _virDomainDef {
But this hunk feels fishy. Why does numatune_conf.h need virDomainDef?
It's a leaky abstraction - virDomainNumatuneNodeParseXML is accessing
def->cpu directly instead of limiting itself to just def->numatune.
Also, virDomainNumatuneParseXML is accessing def->placement_mode, which
argues that placement_mode should be part of def->numatune rather than
an independent variable.
Yes, this hunk solves the compiler fix, but it means that we are just
cementing that we didn't abstract things into a single object. That is,
my ideal setup would be that numatune has access to all the pieces that
it reads/modifies as parameters, but not access the entire virDomainDef,
and then we don't have a circular referencing situation and don't need
numatune_conf.h to be the source of our typedef declaration of virDomainDef.
> I didn't check it beyond build and check/syntax-check though. Anyway, it
> doesn't look quite clean to have typedefs in numatune_conf.h for the
> struct declared in domain_conf.h.
I'd be fine with your patch going in as a stop-gap compilation fix, even
if I still think that we could restructure the code better to make
numatune_conf.h self-contained and move the typedef for virDomainDef
back to domain_conf.h.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list