Sorry for causing build break :(
I would prefer the way tpm_init() was handled in vl.c, even
tpm_cleanup() shuould be guarded behind #ifdef CONFIG_TPM.
- Amarnath
On Wed, 2017-10-18 at 11:27 +0200, Juan Quintela wrote:
> Laurent Vivier <lviv...@redhat.com> wrote:
> >
> > On 18/10/2017 10:33, Juan Quintela wrote:
> > >
> > > Commit
> > > c37cacabf2285b0731b44c1f667781fdd4f2b658
> > >
> > > broke compilation without tpm. Just add an empty tpm_cleanup()
> > > stub.
> > tpm_init() and tpm_config_parse() are managed in a different way:
> > they
> > are included in a "#ifdef CONFIG_TPM .. #endif" in vl.c
> >
> > I think you should follow the same way.
> tpm is weird (TM):
>
> in include/sysemu/tpm.h we do that in an incline function
>
> static inline TPMVersion tpm_get_version(void)
> {
> #ifdef CONFIG_TPM
> Object *obj = object_resolve_path_type("", TYPE_TPM_TIS, NULL);
>
> if (obj) {
> return tpm_tis_get_tpm_version(obj);
> }
> #endif
> return TPM_VERSION_UNSPEC;
> }
>
>
> tpm.c, we have an ifdef in the middle of the file
>
> #ifdef CONFIG_TPM
>
> #endif
>
>
> vl.c, we protect tpm_* calls with CONFIG_TPM
>
> #ifdef CONFIG_TPM
> case QEMU_OPTION_tpmdev:
> if (tpm_config_parse(qemu_find_opts("tpmdev"),
> optarg) < 0) {
> exit(1);
> }
> break;
> #endif
>
>
> but we almost never do:
>
> #ifdef CONFIG_TPM
> tpm_cleanup()
> #endif
>
> We normally create an stub function.
>
> So ......
>
> We are going to be inconsistent whatever we did.
>
> I would have vote for
>
> #ifdef CONFIG_TPM
> void tpm_cleanup(void);
> #else
> static inline void tpm_cleanup(void) {}
> #endif
>
> On the header file (it was my first solution), but having CONFIG_TPM
> on
> tpm.c sounded gross.
>
> So ....
>
> I think that now that the problem is spotted, I will left the choose
> of
> the solution to the maintaner O:-)
>
> Later, Juan.
