From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:55224)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanb@linux.vnet.ibm.com>) id 1QIRhi-0003QC-6x
	for qemu-devel@nongnu.org; Fri, 06 May 2011 16:32:51 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <stefanb@linux.vnet.ibm.com>) id 1QIRhh-0002P4-4Z
	for qemu-devel@nongnu.org; Fri, 06 May 2011 16:32:50 -0400
Received: from e32.co.us.ibm.com ([32.97.110.150]:37286)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanb@linux.vnet.ibm.com>) id 1QIRhg-0002P0-Ru
	for qemu-devel@nongnu.org; Fri, 06 May 2011 16:32:49 -0400
Received: from d03relay03.boulder.ibm.com (d03relay03.boulder.ibm.com
	[9.17.195.228])
	by e32.co.us.ibm.com (8.14.4/8.13.1) with ESMTP id p46KLSrf021387
	for <qemu-devel@nongnu.org>; Fri, 6 May 2011 14:21:28 -0600
Received: from d03av06.boulder.ibm.com (d03av06.boulder.ibm.com [9.17.195.245])
	by d03relay03.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id
	p46KWggZ158058
	for <qemu-devel@nongnu.org>; Fri, 6 May 2011 14:32:43 -0600
Received: from d03av06.boulder.ibm.com (loopback [127.0.0.1])
	by d03av06.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP
	id p46Kbi5a031730
	for <qemu-devel@nongnu.org>; Fri, 6 May 2011 14:37:45 -0600
Message-ID: <4DC45AE5.90104@linux.vnet.ibm.com>
Date: Fri, 06 May 2011 16:32:37 -0400
From: Stefan Berger <stefanb@linux.vnet.ibm.com>
MIME-Version: 1.0
References: <20110506173224.278066589@linux.vnet.ibm.com>
	<20110506173244.297608456@linux.vnet.ibm.com>
	<20110506202332.GA24837@mail.hallyn.com>
In-Reply-To: <20110506202332.GA24837@mail.hallyn.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH V4 01/10] Support for TPM command line
	options
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Serge E. Hallyn" <serge@hallyn.com>
Cc: qemu-devel@nongnu.org, andreas.niederl@iaik.tugraz.at

On 05/06/2011 04:23 PM, Serge E. Hallyn wrote:
> Quoting Stefan Berger (stefanb@linux.vnet.ibm.com):
>> This patch adds support for TPM command line options.
>> The command line supported here (considering the libtpms based
>> backend) are
>>
>> ./qemu-... -tpm type=<type>,path=<path to blockstorage file>,
>>
>> and
>>
>> ./qemu-... -tpm ?
>>
>> where the latter works similar to -soundhw ? and shows a list of
>> available TPM backends (i.e., libtpms-based, Xen).
>>
>> Only the 'type' is interpreted in arch_init.c. Using this parameter,
>> the backend is chosen, i.e., 'builtin' for the libtpms-based
>> builtin TPM. The interpretation of the other parameters along with
>> determining whether enough parameters were provided is pushed into
>> the backend driver, which needs to implement the interface function
>> 'handle_options' and return true if the VM can be started or 'false'
>> if not enough or bad parameters were provided.
>>
>> v4:
>>   - coding style fixes
>>
>> v3:
>>   - added hw/tpm_tis.h to this patch so Qemu compiles at this stage
>>
>> Signed-off-by: Stefan Berger<stefanb@linux.vnet.ibm.com>
> Thanks, Stefan.  Two nits:
>
>> +static QemuOptsList qemu_tpm_opts = {
>> +    .name = "tpm",
>> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_tpm_opts.head),
>> +    .desc = {
>> +        {
>> +            .name = "type",
>> +            .type = QEMU_OPT_STRING,
>> +            .help = "Type of TPM backend",
>> +        },
>> +        {
>> +            .name = "path",
>> +            .type = QEMU_OPT_STRING,
>> +            .help = "Persitent storage for TPM state",
> Persistent.
Oh, typo. Will fix it.
> ...
>
>> +# else /* CONFIG_TPM */
>> +
>> +void select_tpm(const char *optarg)
>> +{
>> +    (void)optarg;
>> +}
> I realize this should never get called if !CONFIG_TPM, but that still
> doesn't seem like cause to go directly calling a potentially NULL
> string.
(void)optarg is just there to make the compiler not put out a warning 
about an unused parameter. I could have used __attribute__((unused)) 
instead but chose this one here. It's not calling anything.

Thanks!

    Stefan
> Otherwise, fwiw
>
> Acked-by: Serge Hallyn<serge.hallyn@ubuntu.com>
>
> thanks,
> -serge