[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [v9 Patch 4/6]Qemu: Add commandline -drive option 'host
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [v9 Patch 4/6]Qemu: Add commandline -drive option 'hostcache' |
Date: |
Thu, 17 Nov 2011 14:11:12 +0000 |
On Thu, Nov 17, 2011 at 5:18 AM, Supriya Kannery
<address@hidden> wrote:
> On 11/17/2011 01:36 AM, Stefan Hajnoczi wrote:
>>
>> On Fri, Nov 11, 2011 at 6:48 AM, Supriya Kannery
>> <address@hidden> wrote:
>>>
>>> + if ((hostcache = qemu_opt_get_bool(opts, "hostcache", -1)) !=
>>> -1) {
>>
>> This does not work. qemu_opt_get_bool() takes a bool default argument
>> and returns a bool. (bool)-1 == true. But (int)true == 1 and you
>> cannot expect it to ever equal -1.
>>
>> Try this:
>>
>> if (qemu_opt_get(opts, "hostcache")&&
>> !qemu_opt_get_bool(opts, "hostcache", false)) {
>> bdrv_flags |= BDRV_O_NOCACHE;
>> }
>>
>> Stefan
>>
>
> Thanks! for pointing this.
> Does the following look ok?
>
> if ((hostcache = qemu_opt_get_bool(opts, "hostcache", 1) == 0) {
> bdrv_flags |= BDRV_O_NOCACHE;
> }
>
> If either "hostcache" is not at all specified or it is specified
> as "on", qemu_opt_get_bool will return 1, which can be ignored
> as bdrv_flags is initialized to 0.
It depends on the overall way this should work. I think this captures
all the cases:
1. cache= and hostcache= may not be used together.
2. cache= sets bdrv_flags.
3. hostcache= may |= BDRV_O_NOCACHE.
4. No option defaults to cache=writethrough (bdrv_flags &= ~BDRV_O_CACHE_MASK).
The code you posted will work although I find it a bit weird how it
also includes case #4. IMO it's cleanest to just do case #3 by
testing whether or not the hostcache= option is set.
BTW, is there a check for case #1 in your patch series. I thought I
saw one earlier but now I can't find it.
Stefan
- Re: [Qemu-devel] [v9 Patch 2/6]Qemu: Error classes for file reopen and data sync failure, (continued)
- [Qemu-devel] [v9 Patch 4/6]Qemu: Add commandline -drive option 'hostcache', Supriya Kannery, 2011/11/11
- Re: [Qemu-devel] [v9 Patch 4/6]Qemu: Add commandline -drive option 'hostcache', Stefan Hajnoczi, 2011/11/16
- Re: [Qemu-devel] [v9 Patch 4/6]Qemu: Add commandline -drive option 'hostcache', Supriya Kannery, 2011/11/17
- Re: [Qemu-devel] [v9 Patch 4/6]Qemu: Add commandline -drive option 'hostcache',
Stefan Hajnoczi <=
- Re: [Qemu-devel] [v9 Patch 4/6]Qemu: Add commandline -drive option 'hostcache', supriya kannery, 2011/11/21
- Re: [Qemu-devel] [v9 Patch 4/6]Qemu: Add commandline -drive option 'hostcache', Stefan Hajnoczi, 2011/11/21
- Re: [Qemu-devel] [v9 Patch 4/6]Qemu: Add commandline -drive option 'hostcache', supriya kannery, 2011/11/22
- Re: [Qemu-devel] [v9 Patch 4/6]Qemu: Add commandline -drive option 'hostcache', Kevin Wolf, 2011/11/22
- Re: [Qemu-devel] [v9 Patch 4/6]Qemu: Add commandline -drive option 'hostcache', Stefan Hajnoczi, 2011/11/22
- Re: [Qemu-devel] [v9 Patch 4/6]Qemu: Add commandline -drive option 'hostcache', Kevin Wolf, 2011/11/22
[Qemu-devel] [v9 Patch 5/6]Qemu: Framework for reopening images safely, Supriya Kannery, 2011/11/11