[pve-devel] [PATCH] Add patch to improve qmrestore to RBD, activating writeback cache.

[Alexandre DERUMIER]

I wonder if this couldn't be fixed directly in rbd.c block driver


currently:

block/rbd.c

    if (flags & BDRV_O_NOCACHE) {
        rados_conf_set(s->cluster, "rbd_cache", "false");
    } else {
        rados_conf_set(s->cluster, "rbd_cache", "true");
    }


and in block.c

int bdrv_parse_cache_mode(const char *mode, int *flags, bool *writethrough)
{
    *flags &= ~BDRV_O_CACHE_MASK;

    if (!strcmp(mode, "off") || !strcmp(mode, "none")) {
        *writethrough = false;
        *flags |= BDRV_O_NOCACHE;
    } else if (!strcmp(mode, "directsync")) {
        *writethrough = true;
        *flags |= BDRV_O_NOCACHE;
    } else if (!strcmp(mode, "writeback")) {
        *writethrough = false;
    } else if (!strcmp(mode, "unsafe")) {
        *writethrough = false;
        *flags |= BDRV_O_NO_FLUSH;
    } else if (!strcmp(mode, "writethrough")) {
        *writethrough = true;
    } else {
        return -1;
    }

    return 0;
}



as in qemu-img convert we don't send flush and in vma restore too, we use BDRV_O_NO_FLUSH.


I think we should add in rbd.c something like

   if (flags & BDRV_O_NOCACHE) {
       rados_conf_set(s->cluster, "rbd_cache", "false");
   } else {
       rados_conf_set(s->cluster, "rbd_cache", "true");
   }

+   if (flags & BDRV_O_NO_FLUSH) {
+        rados_conf_set(s->cluster, "rbd_cache_writethrough_until_flush", "false");
+   }


I really think it's the right way (or it's impossible to use unsafe cache option) and could be pushed to qemu upsteam.


----- Mail original -----
De: "Eneko Lacunza" <elacunza at binovo.es>
À: "pve-devel" <pve-devel at pve.proxmox.com>
Envoyé: Mardi 26 Juillet 2016 16:05:52
Objet: Re: [pve-devel] [PATCH] Add patch to improve qmrestore to RBD, activating writeback cache.

Hi, 

I'm unsure about wether it would be better to have one patch or two 
independent patches? 

El 26/07/16 a las 16:02, Alexandre DERUMIER escribió: 
> Hi, 
> 
> I think that qemu-img convert have the same problem 
> 
> I have found a commit from 2015 
> https://git.greensocs.com/fkonrad/mttcg/commit/80ccf93b884a2edab5ec62634758e942bba81b7c 
> 
> By default it don't send flush (cache=unsafe), and the commit add a flush on image closing, because some storage like sheepdog, 
> can't do the flush by themself. 
> 
> maybe can we add the same flush just after the opening in convert. 
> 
> 
> 
> ----- Mail original ----- 
> De: "Eneko Lacunza" <elacunza at binovo.es> 
> À: "pve-devel" <pve-devel at pve.proxmox.com> 
> Cc: "Eneko Lacunza" <elacunza at pve-test.binovo.net> 
> Envoyé: Mardi 26 Juillet 2016 15:18:55 
> Objet: [pve-devel] [PATCH] Add patch to improve qmrestore to RBD, activating writeback cache. 
> 
> From: Eneko Lacunza <elacunza at pve-test.binovo.net> 
> 
> Signed-off-by: Eneko Lacunza <elacunza at pve-test.binovo.net> 
> --- 
> .../0054-vma-force-enable-rbd-cache-for-qmrestore.patch | 17 +++++++++++++++++ 
> debian/patches/series | 1 + 
> 2 files changed, 18 insertions(+) 
> create mode 100644 debian/patches/pve/0054-vma-force-enable-rbd-cache-for-qmrestore.patch 
> 
> diff --git a/debian/patches/pve/0054-vma-force-enable-rbd-cache-for-qmrestore.patch b/debian/patches/pve/0054-vma-force-enable-rbd-cache-for-qmrestore.patch 
> new file mode 100644 
> index 0000000..d9722c7 
> --- /dev/null 
> +++ b/debian/patches/pve/0054-vma-force-enable-rbd-cache-for-qmrestore.patch 
> @@ -0,0 +1,17 @@ 
> +Issue a bogus flush so that Ceph activates rbd cache, accelerating qmrestore to RBD. 
> +--- 
> +Index: b/vma.c 
> +=================================================================== 
> +--- a/vma.c 
> ++++ b/vma.c 
> +@@ -335,6 +335,9 @@ static int extract_content(int argc, cha 
> + 
> + BlockDriverState *bs = blk_bs(blk); 
> + 
> ++ /* This is needed to activate rbd cache (writeback/coalesce) */ 
> ++ bdrv_flush(bs); 
> ++ 
> + if (vma_reader_register_bs(vmar, i, bs, write_zero, &errp) < 0) { 
> + g_error("%s", error_get_pretty(errp)); 
> + } 
> + 
> diff --git a/debian/patches/series b/debian/patches/series 
> index 3614309..c858a30 100644 
> --- a/debian/patches/series 
> +++ b/debian/patches/series 
> @@ -51,6 +51,7 @@ pve/0050-fix-possible-unitialised-return-value.patch 
> pve/0051-net-NET_CLIENT_OPTIONS_KIND_MAX-changed.patch 
> pve/0052-vnc-refactor-to-QIOChannelSocket.patch 
> pve/0053-vma-use-BlockBackend-on-extract.patch 
> +pve/0054-vma-force-enable-rbd-cache-for-qmrestore.patch 
> #see https://bugs.launchpad.net/qemu/+bug/1488363?comments=all 
> extra/0001-Revert-target-i386-disable-LINT0-after-reset.patch 
> extra/0001-i386-kvmvapic-initialise-imm32-variable.patch 


-- 
Zuzendari Teknikoa / Director Técnico 
Binovo IT Human Project, S.L. 
Telf. 943493611 
943324914 
Astigarraga bidea 2, planta 6 dcha., ofi. 3-2; 20180 Oiartzun (Gipuzkoa) 
www.binovo.es 

_______________________________________________ 
pve-devel mailing list 
pve-devel at pve.proxmox.com 
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel