[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v9 4/4] qemu-img: copy *key-secret opts when ope
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v9 4/4] qemu-img: copy *key-secret opts when opening newly created files |
Date: |
Mon, 15 May 2017 19:44:55 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 |
On 2017-05-15 19:43, Max Reitz wrote:
> On 2017-05-15 16:04, Daniel P. Berrange wrote:
>> The qemu-img dd/convert commands will create an image file and
>> then try to open it. Historically it has been possible to open
>> new files without passing any options. With encrypted files
>> though, the *key-secret options are mandatory, so we need to
>> provide those options when opening the newly created file.
>>
>> Reviewed-by: Max Reitz <address@hidden>
>> Reviewed-by: Fam Zheng <address@hidden>
>> Reviewed-by: Eric Blake <address@hidden>
>> Signed-off-by: Daniel P. Berrange <address@hidden>
>> ---
>> qemu-img.c | 42 +++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 37 insertions(+), 5 deletions(-)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index e0e3d31..dcddded 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -314,15 +314,18 @@ static BlockBackend *img_open_opts(const char *optstr,
>> }
>>
>> static BlockBackend *img_open_file(const char *filename,
>> + QDict *options,
>> const char *fmt, int flags,
>> bool writethrough, bool quiet,
>> bool force_share)
>> {
>> BlockBackend *blk;
>> Error *local_err = NULL;
>> - QDict *options = qdict_new();
>>
>> if (fmt) {
>> + if (!options) {
>> + options = qdict_new();
>> + }
>
> This is the only place where my attempted rebase and your version
> differ. I think this has to be done unconditionally, because otherwise:
>
> $ ./qemu-img info -U null-co://
> [1] 16327 segmentation fault (core dumped) ./qemu-img info -U null-co://
>
> Also, I'm not sure the R-bs apply for this patch any longer.
>
> (They do for patch 1 because it's just a contextual difference. For
> patch 2, it's a borderline case (I would drop it, but I can understand
> keeping it). For patch 3 it's more than just borderline - I would
> definitely drop the R-b, but the differences are still rather
> mechanical, so it is acceptable to keep it.
> But I think there are too many changes here in this patch to keep the
> R-bs. In fact, I'm pretty sure none of Eric, Fam and me have given an
> R-b to this segfault...)
And just saw v10... Maybe I should start working on my inbox back to
front...
Max
signature.asc
Description: OpenPGP digital signature