[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v7 for-2.12 24/25] block/curl: Implement bdrv_re
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH v7 for-2.12 24/25] block/curl: Implement bdrv_refresh_filename() |
Date: |
Fri, 8 Dec 2017 14:47:52 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
On 2017-12-05 11:31, Alberto Garcia wrote:
> On Mon 04 Dec 2017 07:26:02 PM CET, Max Reitz wrote:
>>>> +static void curl_refresh_filename(BlockDriverState *bs)
>>>> +{
>>>> + BDRVCURLState *s = bs->opaque;
>>>> +
>>>> + if (!s->sslverify || s->cookie ||
>>>> + s->username || s->password || s->proxyusername ||
>>>> s->proxypassword)
>>>> + {
>>>
>>> Is !s->sslverify negative because that setting is true by default?
>>
>> Yes, exactly. If it's false, you'd need to override it (and you can't
>> do that through a plain filename).
>
> I think this is not the only case in this series, but I'm not very
> comfortable with the idea that this condition and the default value of
> the setting are implicity dependent on each other. If you change one and
> forget to change the other things will break.
Well, yes, but...
> I understand that the default value is never supposed to change so in
> practice I don't see this breaking,
Yes.
> but is it perhaps worth adding tests
> for all these cases?
In theory, sure. In practice, adding a curl test case seems hard.
Well, I could connect to, I don't know, qemu.org or something and then
just test things there (with the index used as a raw image), but if I
add one test for the curl protocol, nobody is ever going to run them...
And in theory, that's not how a curl test should work. In theory, you'd
expect the user to set up a localhost server with some known root
directory and then _make_test_img etc. would set up images there. But
that way, really nobody is ever going to run them.
And just adding a test for all protocols that then accesses qemu.org
feels somehow wrong, too... Maybe if I add it to a network group, that
wouldn't feel so bad.
Also, adding macros for the default values could help, I think.
Max
signature.asc
Description: OpenPGP digital signature