From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:36742)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mreitz@redhat.com>) id 1Whlw9-0007JB-Qb
	for qemu-devel@nongnu.org; Tue, 06 May 2014 16:26:07 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mreitz@redhat.com>) id 1Whlw3-0007b5-IH
	for qemu-devel@nongnu.org; Tue, 06 May 2014 16:26:01 -0400
Received: from mx1.redhat.com ([209.132.183.28]:59038)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mreitz@redhat.com>) id 1Whlw3-0007ax-9V
	for qemu-devel@nongnu.org; Tue, 06 May 2014 16:25:55 -0400
Message-ID: <53693F40.5040308@redhat.com>
Date: Tue, 06 May 2014 22:00:00 +0200
From: Max Reitz <mreitz@redhat.com>
MIME-Version: 1.0
References: <1399404625-6093-1-git-send-email-mreitz@redhat.com>
	<1399404625-6093-4-git-send-email-mreitz@redhat.com>
	<53693EC1.3080303@redhat.com>
In-Reply-To: <53693EC1.3080303@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 3/4] block: Allow JSON filenames
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: Eric Blake <eblake@redhat.com>, qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, =?UTF-8?B?QmVub8OudCBDYW5ldA==?= <benoit@irqsave.net>, Stefan Hajnoczi <stefanha@redhat.com>

On 06.05.2014 21:57, Eric Blake wrote:
> On 05/06/2014 01:30 PM, Max Reitz wrote:
>> If the filename given to bdrv_open() is prefixed with "json:", parse the
>> rest as a JSON object and use the result as the options QDict.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   block.c | 41 +++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 41 insertions(+)
>>
>>   /*
>>    * Opens a disk image (raw, qcow2, vmdk, ...)
>>    *
>> @@ -1337,6 +1364,20 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
>>           options = qdict_new();
>>       }
>>   
>> +    if (filename && g_str_has_prefix(filename, "json:")) {
>> +        QDict *json_options = parse_json_filename(filename, &local_err);
>> +        if (local_err) {
>> +            ret = -EINVAL;
>> +            goto fail;
>> +        }
>> +
>> +        qdict_join(options, json_options, true);
>> +        assert(qdict_size(json_options) == 0);
> Would it be better to pass false to qdict_join(), and then raise an
> error if the user specified conflicting options?  For example (untested,
> just typing off the top of my head here),
>
> -drive
> file='json:{"driver":"qcow2","file.filename":"foo","backing.file.driver":"raw"}',backing.file.driver=qcow2
>
> looks like it specifies conflicting backing.file.driver options.
> Passing true means that qdict_join silently overwrites the value in
> options to instead be the value in the json string; passing false means
> you could flag the user error.

Yes, you're right; I'll change it.

Max

>> +        QDECREF(json_options);
>> +
>> +        filename = NULL;
>> +    }
>> +
>>       bs->options = options;
>>       options = qdict_clone_shallow(options);
>>