[PATCH] qemu-nbd: Change default cache mode to writeback

[PATCH] qemu-nbd: Change default cache mode to writeback

[Nir Soffer]

Both qemu and qemu-img use writeback cache mode by default, which is
already documented in qemu(1). qemu-nbd uses writethrough cache mode by
default, and the default cache mode is not documented.

According to the qemu-nbd(8):

   --cache=CACHE
          The  cache  mode  to be used with the file.  See the
          documentation of the emulator's -drive cache=... option for
          allowed values.

qemu(1) says:

    The default mode is cache=writeback.

So users have no reason to assume that qemu-nbd is using writethough
cache mode. The only hint is the painfully slow writing when using the
defaults.

Looking in git history, it seems that qemu used writethrough in the past
to support broken guests that did not flush data properly, or could not
flush due to limitations in qemu. But qemu-nbd clients can use
NBD_CMD_FLUSH to flush data, so using writethrough does not help anyone.

Change the default cache mode to writback, and document the default and
available values properly in the online help and manual.

With this change converting image via qemu-nbd is 3.5 times faster.

    $ qemu-img create dst.img 50g
    $ qemu-nbd -t -f raw -k /tmp/nbd.sock dst.img

Before this change:

    $ hyperfine -r3 "./qemu-img convert -p -f raw -O raw -T none -W fedora34.img nbd+unix:///?socket=/tmp/nbd.sock"
    Benchmark #1: ./qemu-img convert -p -f raw -O raw -T none -W fedora34.img nbd+unix:///?socket=/tmp/nbd.sock
      Time (mean ± σ):     83.639 s ±  5.970 s    [User: 2.733 s, System: 6.112 s]
      Range (min … max):   76.749 s … 87.245 s    3 runs

After this change:

    $ hyperfine -r3 "./qemu-img convert -p -f raw -O raw -T none -W fedora34.img nbd+unix:///?socket=/tmp/nbd.sock"
    Benchmark #1: ./qemu-img convert -p -f raw -O raw -T none -W fedora34.img nbd+unix:///?socket=/tmp/nbd.sock
      Time (mean ± σ):     23.522 s ±  0.433 s    [User: 2.083 s, System: 5.475 s]
      Range (min … max):   23.234 s … 24.019 s    3 runs

Users can avoid the issue by using --cache=writeback[1] but the defaults
should give good performance for the common use case.

[1] https://bugzilla.redhat.com/1990656

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
 docs/tools/qemu-nbd.rst | 6 ++++--
 qemu-nbd.c              | 6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/docs/tools/qemu-nbd.rst b/docs/tools/qemu-nbd.rst
index ee862fa0bc..5643da26e9 100644
--- a/docs/tools/qemu-nbd.rst
+++ b/docs/tools/qemu-nbd.rst
@@ -98,8 +98,10 @@ driver options if ``--image-opts`` is specified.
 
 .. option:: --cache=CACHE
 
-  The cache mode to be used with the file.  See the documentation of
-  the emulator's ``-drive cache=...`` option for allowed values.
+  The cache mode to be used with the file. Valid values are:
+  ``none``, ``writeback`` (the default), ``writethrough``,
+  ``directsync`` and ``unsafe``. See the documentation of
+  the emulator's ``-drive cache=...`` option for more info.
 
 .. option:: -n, --nocache
 
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 26ffbf15af..6c18fcd19a 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -135,7 +135,9 @@ static void usage(const char *name)
 "                            'snapshot.id=[ID],snapshot.name=[NAME]', or\n"
 "                            '[ID_OR_NAME]'\n"
 "  -n, --nocache             disable host cache\n"
-"      --cache=MODE          set cache mode (none, writeback, ...)\n"
+"      --cache=MODE          set cache mode used to access the disk image, the\n"
+"                            valid options are: 'none', 'writeback' (default),\n"
+"                            'writethrough', 'directsync' and 'unsafe'\n"
 "      --aio=MODE            set AIO mode (native, io_uring or threads)\n"
 "      --discard=MODE        set discard mode (ignore, unmap)\n"
 "      --detect-zeroes=MODE  set detect-zeroes mode (off, on, unmap)\n"
@@ -552,7 +554,7 @@ int main(int argc, char **argv)
     bool alloc_depth = false;
     const char *tlscredsid = NULL;
     bool imageOpts = false;
-    bool writethrough = true;
+    bool writethrough = false; /* Client will flush as needed. */
     bool fork_process = false;
     bool list = false;
     int old_stderr = -1;
-- 
2.31.1

Re: [PATCH] qemu-nbd: Change default cache mode to writeback

[Eric Blake]

On Fri, Aug 13, 2021 at 11:55:19PM +0300, Nir Soffer wrote:
> Both qemu and qemu-img use writeback cache mode by default, which is
> already documented in qemu(1). qemu-nbd uses writethrough cache mode by
> default, and the default cache mode is not documented.
> 
> According to the qemu-nbd(8):
> 
>    --cache=CACHE
>           The  cache  mode  to be used with the file.  See the
>           documentation of the emulator's -drive cache=... option for
>           allowed values.
> 
> qemu(1) says:
> 
>     The default mode is cache=writeback.
> 
> So users have no reason to assume that qemu-nbd is using writethough
> cache mode. The only hint is the painfully slow writing when using the
> defaults.

Oh, good catch.  Unfortunately too late for 6.1 proper, but I'll add
qemu-stable in cc and queue this through my NBD tree for 6.2.

> Users can avoid the issue by using --cache=writeback[1] but the defaults
> should give good performance for the common use case.
> 
> [1] https://bugzilla.redhat.com/1990656
> 
> Signed-off-by: Nir Soffer <nsoffer@redhat.com>
> ---

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH] qemu-nbd: Change default cache mode to writeback

[Nir Soffer]

On Mon, Aug 16, 2021 at 6:50 PM Eric Blake <eblake@redhat.com> wrote:
>
> On Fri, Aug 13, 2021 at 11:55:19PM +0300, Nir Soffer wrote:
> > Both qemu and qemu-img use writeback cache mode by default, which is
> > already documented in qemu(1). qemu-nbd uses writethrough cache mode by
> > default, and the default cache mode is not documented.
> >
> > According to the qemu-nbd(8):
> >
> >    --cache=CACHE
> >           The  cache  mode  to be used with the file.  See the
> >           documentation of the emulator's -drive cache=... option for
> >           allowed values.
> >
> > qemu(1) says:
> >
> >     The default mode is cache=writeback.
> >
> > So users have no reason to assume that qemu-nbd is using writethough
> > cache mode. The only hint is the painfully slow writing when using the
> > defaults.
>
> Oh, good catch.  Unfortunately too late for 6.1 proper, but I'll add
> qemu-stable in cc and queue this through my NBD tree for 6.2.

I don't see this in master, lost in your NBD tree?

> > Users can avoid the issue by using --cache=writeback[1] but the defaults
> > should give good performance for the common use case.
> >
> > [1] https://bugzilla.redhat.com/1990656
> >
> > Signed-off-by: Nir Soffer <nsoffer@redhat.com>
> > ---
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org

Re: [PATCH] qemu-nbd: Change default cache mode to writeback

[Eric Blake]

On Sun, Sep 19, 2021 at 10:56:58PM +0300, Nir Soffer wrote:
> On Mon, Aug 16, 2021 at 6:50 PM Eric Blake <eblake@redhat.com> wrote:
> >
> > On Fri, Aug 13, 2021 at 11:55:19PM +0300, Nir Soffer wrote:
> > > Both qemu and qemu-img use writeback cache mode by default, which is
> > > already documented in qemu(1). qemu-nbd uses writethrough cache mode by
> > > default, and the default cache mode is not documented.
> > >
> > > According to the qemu-nbd(8):
> > >
> > >    --cache=CACHE
> > >           The  cache  mode  to be used with the file.  See the
> > >           documentation of the emulator's -drive cache=... option for
> > >           allowed values.
> > >
> > > qemu(1) says:
> > >
> > >     The default mode is cache=writeback.
> > >
> > > So users have no reason to assume that qemu-nbd is using writethough
> > > cache mode. The only hint is the painfully slow writing when using the
> > > defaults.
> >
> > Oh, good catch.  Unfortunately too late for 6.1 proper, but I'll add
> > qemu-stable in cc and queue this through my NBD tree for 6.2.
> 
> I don't see this in master, lost in your NBD tree?

It's on my tree:
https://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/nbd

I just haven't bundled up enough patches for a pull request yet.  I'll
get one out this week, probably with a few more patches included.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org