* [Qemu-devel] [PATCH] tpm: Fix compilation with --disable-tpm
@ 2017-10-18 8:33 Juan Quintela
2017-10-18 9:09 ` Laurent Vivier
0 siblings, 1 reply; 4+ messages in thread
From: Juan Quintela @ 2017-10-18 8:33 UTC (permalink / raw)
To: qemu-devel; +Cc: dgilbert, lvivier, peterx, Amarnath Valluri
Commit
c37cacabf2285b0731b44c1f667781fdd4f2b658
broke compilation without tpm. Just add an empty tpm_cleanup() stub.
CC: Amarnath Valluri <amarnath.valluri@intel.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
tpm.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tpm.c b/tpm.c
index 3122227156..9396bb669c 100644
--- a/tpm.c
+++ b/tpm.c
@@ -194,6 +194,12 @@ int tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
return 0;
}
+#else
+
+void tpm_cleanup(void)
+{
+}
+
#endif /* CONFIG_TPM */
static const TPMDriverOps *tpm_driver_find_by_type(enum TpmType type)
--
2.13.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] tpm: Fix compilation with --disable-tpm
2017-10-18 8:33 [Qemu-devel] [PATCH] tpm: Fix compilation with --disable-tpm Juan Quintela
@ 2017-10-18 9:09 ` Laurent Vivier
2017-10-18 9:27 ` Juan Quintela
0 siblings, 1 reply; 4+ messages in thread
From: Laurent Vivier @ 2017-10-18 9:09 UTC (permalink / raw)
To: Juan Quintela, qemu-devel; +Cc: dgilbert, peterx, Amarnath Valluri
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.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] tpm: Fix compilation with --disable-tpm
2017-10-18 9:09 ` Laurent Vivier
@ 2017-10-18 9:27 ` Juan Quintela
2017-10-18 10:04 ` Valluri, Amarnath
0 siblings, 1 reply; 4+ messages in thread
From: Juan Quintela @ 2017-10-18 9:27 UTC (permalink / raw)
To: Laurent Vivier; +Cc: qemu-devel, dgilbert, peterx, Amarnath Valluri
Laurent Vivier <lvivier@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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] tpm: Fix compilation with --disable-tpm
2017-10-18 9:27 ` Juan Quintela
@ 2017-10-18 10:04 ` Valluri, Amarnath
0 siblings, 0 replies; 4+ messages in thread
From: Valluri, Amarnath @ 2017-10-18 10:04 UTC (permalink / raw)
To: quintela, lvivier; +Cc: dgilbert, qemu-devel, peterx
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 <lvivier@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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-10-18 10:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-18 8:33 [Qemu-devel] [PATCH] tpm: Fix compilation with --disable-tpm Juan Quintela
2017-10-18 9:09 ` Laurent Vivier
2017-10-18 9:27 ` Juan Quintela
2017-10-18 10:04 ` Valluri, Amarnath
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.