[Qemu-devel] [PATCH 1/1] block: add cache mode with direct IO and without flushes

[Qemu-devel] [PATCH 1/1] block: add cache mode with direct IO and without flushes

[Denis V. Lunev]

This mode could be very useful for flush bottlenecks evaluation and for
running non-persistent VMs, when host crash is considered not fatal.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
---
 block.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block.c b/block.c
index 30d64e6..877165c 100644
--- a/block.c
+++ b/block.c
@@ -641,6 +641,9 @@ int bdrv_parse_cache_mode(const char *mode, int *flags, bool *writethrough)
     if (!strcmp(mode, "off") || !strcmp(mode, "none")) {
         *writethrough = false;
         *flags |= BDRV_O_NOCACHE;
+    } else if (!strcmp(mode, "none-unsafe") || !strcmp(mode, "off-unsafe")) {
+        *writethrough = false;
+        *flags |= BDRV_O_NOCACHE | BDRV_O_NO_FLUSH;
     } else if (!strcmp(mode, "directsync")) {
         *writethrough = true;
         *flags |= BDRV_O_NOCACHE;
-- 
2.5.0

Re: [Qemu-devel] [PATCH 1/1] block: add cache mode with direct IO and without flushes

[Kevin Wolf]

Am 15.09.2016 um 15:19 hat Denis V. Lunev geschrieben:
> This mode could be very useful for flush bottlenecks evaluation and for
> running non-persistent VMs, when host crash is considered not fatal.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Max Reitz <mreitz@redhat.com>

Why not just specify the individual options?

    -drive file=...,cache.direct=on,cache.no-flush=on

Kevin

>  block.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/block.c b/block.c
> index 30d64e6..877165c 100644
> --- a/block.c
> +++ b/block.c
> @@ -641,6 +641,9 @@ int bdrv_parse_cache_mode(const char *mode, int *flags, bool *writethrough)
>      if (!strcmp(mode, "off") || !strcmp(mode, "none")) {
>          *writethrough = false;
>          *flags |= BDRV_O_NOCACHE;
> +    } else if (!strcmp(mode, "none-unsafe") || !strcmp(mode, "off-unsafe")) {
> +        *writethrough = false;
> +        *flags |= BDRV_O_NOCACHE | BDRV_O_NO_FLUSH;
>      } else if (!strcmp(mode, "directsync")) {
>          *writethrough = true;
>          *flags |= BDRV_O_NOCACHE;
> -- 
> 2.5.0
> 

Re: [Qemu-devel] [PATCH 1/1] block: add cache mode with direct IO and without flushes

[Denis V. Lunev]

On 09/15/2016 07:09 PM, Kevin Wolf wrote:
> Am 15.09.2016 um 15:19 hat Denis V. Lunev geschrieben:
>> This mode could be very useful for flush bottlenecks evaluation and for
>> running non-persistent VMs, when host crash is considered not fatal.
>>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Kevin Wolf <kwolf@redhat.com>
>> CC: Max Reitz <mreitz@redhat.com>
> Why not just specify the individual options?
>
>     -drive file=...,cache.direct=on,cache.no-flush=on
>
> Kevin
then we need to have three options:
  cache.direct=on/off
  cache.flush=on/off (default on)
  cache.writethrough=on/off
What will have preference, old style option, new style option, rightmost
option?

Den

Re: [Qemu-devel] [PATCH 1/1] block: add cache mode with direct IO and without flushes

[Kevin Wolf]

Am 15.09.2016 um 18:39 hat Denis V. Lunev geschrieben:
> On 09/15/2016 07:09 PM, Kevin Wolf wrote:
> > Am 15.09.2016 um 15:19 hat Denis V. Lunev geschrieben:
> >> This mode could be very useful for flush bottlenecks evaluation and for
> >> running non-persistent VMs, when host crash is considered not fatal.
> >>
> >> Signed-off-by: Denis V. Lunev <den@openvz.org>
> >> CC: Kevin Wolf <kwolf@redhat.com>
> >> CC: Max Reitz <mreitz@redhat.com>
> > Why not just specify the individual options?
> >
> >     -drive file=...,cache.direct=on,cache.no-flush=on
> >
> > Kevin
> then we need to have three options:
>   cache.direct=on/off
>   cache.flush=on/off (default on)
>   cache.writethrough=on/off
> What will have preference, old style option, new style option, rightmost
> option?

The individual options take precedence, see the code that handles the
"cache=..." option in drive_init():

    value = qemu_opt_get(all_opts, "cache");
    if (value) {
        int flags = 0;
        bool writethrough;

        if (bdrv_parse_cache_mode(value, &flags, &writethrough) != 0) {
            error_report("invalid cache option");
            return NULL;
        }

        /* Specific options take precedence */
        if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_WB)) {
            qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_WB,
                              !writethrough, &error_abort);
        }
        if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_DIRECT)) {
            qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_DIRECT,
                              !!(flags & BDRV_O_NOCACHE), &error_abort);
        }
        if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_NO_FLUSH)) {
            qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_NO_FLUSH,
                              !!(flags & BDRV_O_NO_FLUSH), &error_abort);
        }
        qemu_opt_unset(all_opts, "cache");
    }

Kevin

Re: [Qemu-devel] [PATCH 1/1] block: add cache mode with direct IO and without flushes

[Denis V. Lunev]

On 09/16/2016 11:08 AM, Kevin Wolf wrote:
> Am 15.09.2016 um 18:39 hat Denis V. Lunev geschrieben:
>> On 09/15/2016 07:09 PM, Kevin Wolf wrote:
>>> Am 15.09.2016 um 15:19 hat Denis V. Lunev geschrieben:
>>>> This mode could be very useful for flush bottlenecks evaluation and for
>>>> running non-persistent VMs, when host crash is considered not fatal.
>>>>
>>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>>> CC: Kevin Wolf <kwolf@redhat.com>
>>>> CC: Max Reitz <mreitz@redhat.com>
>>> Why not just specify the individual options?
>>>
>>>     -drive file=...,cache.direct=on,cache.no-flush=on
>>>
>>> Kevin
>> then we need to have three options:
>>   cache.direct=on/off
>>   cache.flush=on/off (default on)
>>   cache.writethrough=on/off
>> What will have preference, old style option, new style option, rightmost
>> option?
> The individual options take precedence, see the code that handles the
> "cache=..." option in drive_init():
>
>     value = qemu_opt_get(all_opts, "cache");
>     if (value) {
>         int flags = 0;
>         bool writethrough;
>
>         if (bdrv_parse_cache_mode(value, &flags, &writethrough) != 0) {
>             error_report("invalid cache option");
>             return NULL;
>         }
>
>         /* Specific options take precedence */
>         if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_WB)) {
>             qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_WB,
>                               !writethrough, &error_abort);
>         }
>         if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_DIRECT)) {
>             qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_DIRECT,
>                               !!(flags & BDRV_O_NOCACHE), &error_abort);
>         }
>         if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_NO_FLUSH)) {
>             qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_NO_FLUSH,
>                               !!(flags & BDRV_O_NO_FLUSH), &error_abort);
>         }
>         qemu_opt_unset(all_opts, "cache");
>     }
>
> Kevin
thank for an idea!

Den

Re: [Qemu-devel] [PATCH 1/1] block: add cache mode with direct IO and without flushes

[Kevin Wolf]

Am 16.09.2016 um 10:15 hat Denis V. Lunev geschrieben:
> On 09/16/2016 11:08 AM, Kevin Wolf wrote:
> > Am 15.09.2016 um 18:39 hat Denis V. Lunev geschrieben:
> >> On 09/15/2016 07:09 PM, Kevin Wolf wrote:
> >>> Am 15.09.2016 um 15:19 hat Denis V. Lunev geschrieben:
> >>>> This mode could be very useful for flush bottlenecks evaluation and for
> >>>> running non-persistent VMs, when host crash is considered not fatal.
> >>>>
> >>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
> >>>> CC: Kevin Wolf <kwolf@redhat.com>
> >>>> CC: Max Reitz <mreitz@redhat.com>
> >>> Why not just specify the individual options?
> >>>
> >>>     -drive file=...,cache.direct=on,cache.no-flush=on
> >>>
> >>> Kevin
> >> then we need to have three options:
> >>   cache.direct=on/off
> >>   cache.flush=on/off (default on)
> >>   cache.writethrough=on/off
> >> What will have preference, old style option, new style option, rightmost
> >> option?
> > The individual options take precedence, see the code that handles the
> > "cache=..." option in drive_init():
> >
> >     value = qemu_opt_get(all_opts, "cache");
> >     if (value) {
> >         int flags = 0;
> >         bool writethrough;
> >
> >         if (bdrv_parse_cache_mode(value, &flags, &writethrough) != 0) {
> >             error_report("invalid cache option");
> >             return NULL;
> >         }
> >
> >         /* Specific options take precedence */
> >         if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_WB)) {
> >             qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_WB,
> >                               !writethrough, &error_abort);
> >         }
> >         if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_DIRECT)) {
> >             qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_DIRECT,
> >                               !!(flags & BDRV_O_NOCACHE), &error_abort);
> >         }
> >         if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_NO_FLUSH)) {
> >             qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_NO_FLUSH,
> >                               !!(flags & BDRV_O_NO_FLUSH), &error_abort);
> >         }
> >         qemu_opt_unset(all_opts, "cache");
> >     }
> >
> > Kevin
> thank for an idea!

Just to be clear, everything that I mentioned is working today, so it's
not just an idea that still needs to implemented.

You just need to check the actual names of the options. Instead of your
proposed writethrough/flush options, we have the reversed options
writeback/no-flush.

Kevin

Re: [Qemu-devel] [PATCH 1/1] block: add cache mode with direct IO and without flushes

[Denis V. Lunev]

On 09/16/2016 11:22 AM, Kevin Wolf wrote:
> Am 16.09.2016 um 10:15 hat Denis V. Lunev geschrieben:
>> On 09/16/2016 11:08 AM, Kevin Wolf wrote:
>>> Am 15.09.2016 um 18:39 hat Denis V. Lunev geschrieben:
>>>> On 09/15/2016 07:09 PM, Kevin Wolf wrote:
>>>>> Am 15.09.2016 um 15:19 hat Denis V. Lunev geschrieben:
>>>>>> This mode could be very useful for flush bottlenecks evaluation and for
>>>>>> running non-persistent VMs, when host crash is considered not fatal.
>>>>>>
>>>>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>>>>> CC: Kevin Wolf <kwolf@redhat.com>
>>>>>> CC: Max Reitz <mreitz@redhat.com>
>>>>> Why not just specify the individual options?
>>>>>
>>>>>     -drive file=...,cache.direct=on,cache.no-flush=on
>>>>>
>>>>> Kevin
>>>> then we need to have three options:
>>>>   cache.direct=on/off
>>>>   cache.flush=on/off (default on)
>>>>   cache.writethrough=on/off
>>>> What will have preference, old style option, new style option, rightmost
>>>> option?
>>> The individual options take precedence, see the code that handles the
>>> "cache=..." option in drive_init():
>>>
>>>     value = qemu_opt_get(all_opts, "cache");
>>>     if (value) {
>>>         int flags = 0;
>>>         bool writethrough;
>>>
>>>         if (bdrv_parse_cache_mode(value, &flags, &writethrough) != 0) {
>>>             error_report("invalid cache option");
>>>             return NULL;
>>>         }
>>>
>>>         /* Specific options take precedence */
>>>         if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_WB)) {
>>>             qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_WB,
>>>                               !writethrough, &error_abort);
>>>         }
>>>         if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_DIRECT)) {
>>>             qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_DIRECT,
>>>                               !!(flags & BDRV_O_NOCACHE), &error_abort);
>>>         }
>>>         if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_NO_FLUSH)) {
>>>             qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_NO_FLUSH,
>>>                               !!(flags & BDRV_O_NO_FLUSH), &error_abort);
>>>         }
>>>         qemu_opt_unset(all_opts, "cache");
>>>     }
>>>
>>> Kevin
>> thank for an idea!
> Just to be clear, everything that I mentioned is working today, so it's
> not just an idea that still needs to implemented.
>
> You just need to check the actual names of the options. Instead of your
> proposed writethrough/flush options, we have the reversed options
> writeback/no-flush.
>
> Kevin
oops, I have missed that. Thank you very much

Den