1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] qemu:Add support L2 table cache for qcow2 disk

Allen Do posted 1 patch 7 years, 7 months ago
Failed in applying to current master (apply log)
src/conf/domain_conf.h                               |  5 +++++
src/qemu/qemu_command.c                              |  7 +++++++
src/qemu/qemu_domain.c                               |  6 ++++++
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.args |  2 +-
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml  |  3 ++-
6 files changed, 68 insertions(+), 2 deletions(-)
[libvirt] [PATCH] qemu:Add support L2 table cache for qcow2 disk
Posted by Allen Do 7 years, 7 months ago 
The patch add support L2 table cache for qcow2 disk. L2 table cache can
improve IO read and write performance for qcow2 img.
Diff follows:
 src/conf/domain_conf.c                               | 47
+++++++++++++++++++++++++++++++++++++++++++++++
 src/conf/domain_conf.h                               |  5 +++++
 src/qemu/qemu_command.c                              |  7 +++++++
 src/qemu/qemu_domain.c                               |  6 ++++++
 tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.args |  2 +-
 tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml  |  3 ++-
 6 files changed, 68 insertions(+), 2 deletions(-)


diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index beca0be..0a6afca 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8058,6 +8058,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
     char *domain_name = NULL;
     int expected_secret_usage = -1;
     int auth_secret_usage = -1;
+    char *disk_l2_cache_size = NULL;
+    char *disk_cache_clean_interval = NULL;

     if (!(def = virDomainDiskDefNew(xmlopt)))
         return NULL;
@@ -8233,6 +8235,27 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr
xmlopt,
             }
         } else if (xmlStrEqual(cur->name, BAD_CAST "boot")) {
             /* boot is parsed as part of virDomainDeviceInfoParseXML */
+        } else if (xmlStrEqual(cur->name, BAD_CAST "diskCache")) {
+            disk_l2_cache_size =
+                virXMLPropString(cur, "disk_l2_cache_size");
+            if (disk_l2_cache_size &&
+                virStrToLong_ui(disk_l2_cache_size, NULL, 0,
+                                &def->disk_cache.disk_l2_cache_size) < 0) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("invalid disk L2 cache size '%s'"),
+                               disk_l2_cache_size);
+                goto error;
+            }
+            disk_cache_clean_interval =
+                virXMLPropString(cur, "disk_cache_clean_interval");
+            if (disk_cache_clean_interval &&
+                virStrToLong_ui(disk_cache_clean_interval, NULL, 0,
+
&def->disk_cache.disk_cache_clean_interval) < 0) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("invalid disk cache clean interval '%s'"),
+                               disk_cache_clean_interval);
+                goto error;
+            }
         }
     }

@@ -8472,6 +8495,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
     VIR_FREE(vendor);
     VIR_FREE(product);
     VIR_FREE(domain_name);
+    VIR_FREE(disk_l2_cache_size);
+    VIR_FREE(disk_cache_clean_interval);

     ctxt->node = save_ctxt;
     return def;
@@ -20646,6 +20671,27 @@ virDomainDiskBlockIoDefFormat(virBufferPtr buf,
     }
 }

+static void
+virDomainDiskCacheDefFormat(virBufferPtr buf,
+                              virDomainDiskDefPtr def)
+{
+    if (def->disk_cache.disk_l2_cache_size > 0 ||
+        def->disk_cache.disk_cache_clean_interval > 0) {
+        virBufferAddLit(buf, "<diskCache");
+        if (def->disk_cache.disk_l2_cache_size > 0) {
+            virBufferAsprintf(buf,
+                              " disk_l2_cache_size='%u'",
+                              def->disk_cache.disk_l2_cache_size);
+        }
+        if (def->disk_cache.disk_cache_clean_interval > 0) {
+            virBufferAsprintf(buf,
+                              " disk_cache_clean_interval='%u'",
+                              def->disk_cache.disk_cache_clean_interval);
+        }
+        virBufferAddLit(buf, "/>\n");
+    }
+}
+

 /* virDomainSourceDefFormatSeclabel:
  *
@@ -20990,6 +21036,7 @@ virDomainDiskDefFormat(virBufferPtr buf,

     virDomainDiskGeometryDefFormat(buf, def);
     virDomainDiskBlockIoDefFormat(buf, def);
+    virDomainDiskCacheDefFormat(buf, def);

     /* For now, mirroring is currently output-only: we only output it
      * for live domains, therefore we ignore it on input except for
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 91a33cb..83ea9d3 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -647,6 +647,11 @@ struct _virDomainDiskDef {
         unsigned int physical_block_size;
     } blockio;

+    struct {
+        unsigned int disk_l2_cache_size;
+        unsigned int disk_cache_clean_interval;
+    } disk_cache;
+
     virDomainBlockIoTuneInfo blkdeviotune;

     char *serial;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e087891..7c47cd3 100755
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1662,6 +1662,13 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
     if (qemuBuildDriveSourceStr(disk, cfg, &opt, qemuCaps) < 0)
         goto error;

+    if (disk->disk_cache.disk_l2_cache_size > 0)
+        virBufferAsprintf(&opt, "l2-cache-size=%u,",
+                disk->disk_cache.disk_l2_cache_size);
+    if (disk->disk_cache.disk_cache_clean_interval > 0)
+        virBufferAsprintf(&opt, "cache-clean-interval=%u,",
+                disk->disk_cache.disk_cache_clean_interval);
+
     if (emitDeviceSyntax)
         virBufferAddLit(&opt, "if=none");
     else
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9f165c1..de334a7 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5408,6 +5408,12 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr
disk,
     CHECK_EQ(ioeventfd, "ioeventfd", true);
     CHECK_EQ(event_idx, "event_idx", true);
     CHECK_EQ(copy_on_read, "copy_on_read", true);
+
+    CHECK_EQ(disk_cache.disk_l2_cache_size,
+             "diskCache disk_l2_cache_size", true);
+    CHECK_EQ(disk_cache.disk_cache_clean_interval,
+             "diskCache disk_cache_clean_interval", true);
+
     /* "snapshot" is a libvirt internal field and thus can be changed */
     /* startupPolicy is allowed to be updated. Therefore not checked here.
*/
     CHECK_EQ(transient, "transient", true);
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.args
b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.args
index b405242..b968302 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.args
@@ -22,7 +22,7 @@ QEMU_AUDIO_DRV=none \
 -drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,media=cdrom,\
 id=drive-ide0-1-0,readonly=on \
 -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \
--drive file=/tmp/data.img,format=raw,if=none,id=drive-virtio-disk0 \
+-drive
file=/tmp/data.img,format=qcow2,l2-cache-size=536870912,cache-clean-interval=900,if=none,id=drive-virtio-disk0
\
 -device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,\
 id=virtio-disk0 \
 -drive file=/tmp/logs.img,format=raw,if=none,id=drive-virtio-disk1 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml
index b843878..43298fb 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml
@@ -28,8 +28,9 @@
       <address type='drive' controller='0' bus='1' target='0' unit='0'/>
     </disk>
     <disk type='file' device='disk'>
-      <driver name='qemu' type='raw'/>
+      <driver name='qemu' type='qcow2'/>
       <source file='/tmp/data.img'/>
+      <diskCache disk_l2_cache_size='53687'
disk_cache_clean_interval='900' />
       <target dev='vda' bus='virtio'/>
     </disk>
     <disk type='file' device='disk'>


Thanks
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu:Add support L2 table cache for qcow2 disk
Posted by John Ferlan 7 years, 7 months ago 

On 06/27/2018 03:50 AM, Allen Do wrote:
> The patch add support L2 table cache for qcow2 disk. L2 table cache can
> improve IO read and write performance for qcow2 img.
> Diff follows:
>  src/conf/domain_conf.c                               | 47
> +++++++++++++++++++++++++++++++++++++++++++++++
>  src/conf/domain_conf.h                               |  5 +++++
>  src/qemu/qemu_command.c                              |  7 +++++++
>  src/qemu/qemu_domain.c                               |  6 ++++++
>  tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.args |  2 +-
>  tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml  |  3 ++-
>  6 files changed, 68 insertions(+), 2 deletions(-)
> 

Before you go through too much trouble though, this has been attempted
before many times and each time it's been rejected for reasons described
in responses to those patches.

See:

https://www.redhat.com/archives/libvir-list/2017-November/msg00536.html

and

https://www.redhat.com/archives/libvir-list/2017-September/msg00553.html

John

> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index beca0be..0a6afca 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -8058,6 +8058,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
>      char *domain_name = NULL;
>      int expected_secret_usage = -1;
>      int auth_secret_usage = -1;
> +    char *disk_l2_cache_size = NULL;
> +    char *disk_cache_clean_interval = NULL;
> 
>      if (!(def = virDomainDiskDefNew(xmlopt)))
>          return NULL;
> @@ -8233,6 +8235,27 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr
> xmlopt,
>              }
>          } else if (xmlStrEqual(cur->name, BAD_CAST "boot")) {
>              /* boot is parsed as part of virDomainDeviceInfoParseXML */
> +        } else if (xmlStrEqual(cur->name, BAD_CAST "diskCache")) {
> +            disk_l2_cache_size =
> +                virXMLPropString(cur, "disk_l2_cache_size");
> +            if (disk_l2_cache_size &&
> +                virStrToLong_ui(disk_l2_cache_size, NULL, 0,
> +                                &def->disk_cache.disk_l2_cache_size) < 0) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("invalid disk L2 cache size '%s'"),
> +                               disk_l2_cache_size);
> +                goto error;
> +            }
> +            disk_cache_clean_interval =
> +                virXMLPropString(cur, "disk_cache_clean_interval");
> +            if (disk_cache_clean_interval &&
> +                virStrToLong_ui(disk_cache_clean_interval, NULL, 0,
> +                               
> &def->disk_cache.disk_cache_clean_interval) < 0) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("invalid disk cache clean interval '%s'"),
> +                               disk_cache_clean_interval);
> +                goto error;
> +            }
>          }
>      }
> 
> @@ -8472,6 +8495,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
>      VIR_FREE(vendor);
>      VIR_FREE(product);
>      VIR_FREE(domain_name);
> +    VIR_FREE(disk_l2_cache_size);
> +    VIR_FREE(disk_cache_clean_interval);
> 
>      ctxt->node = save_ctxt;
>      return def;
> @@ -20646,6 +20671,27 @@ virDomainDiskBlockIoDefFormat(virBufferPtr buf,
>      }
>  }
> 
> +static void
> +virDomainDiskCacheDefFormat(virBufferPtr buf,
> +                              virDomainDiskDefPtr def)
> +{
> +    if (def->disk_cache.disk_l2_cache_size > 0 ||
> +        def->disk_cache.disk_cache_clean_interval > 0) {
> +        virBufferAddLit(buf, "<diskCache");
> +        if (def->disk_cache.disk_l2_cache_size > 0) {
> +            virBufferAsprintf(buf,
> +                              " disk_l2_cache_size='%u'",
> +                              def->disk_cache.disk_l2_cache_size);
> +        }
> +        if (def->disk_cache.disk_cache_clean_interval > 0) {
> +            virBufferAsprintf(buf,
> +                              " disk_cache_clean_interval='%u'",
> +                              def->disk_cache.disk_cache_clean_interval);
> +        }
> +        virBufferAddLit(buf, "/>\n");
> +    }
> +}
> +
> 
>  /* virDomainSourceDefFormatSeclabel:
>   *
> @@ -20990,6 +21036,7 @@ virDomainDiskDefFormat(virBufferPtr buf,
> 
>      virDomainDiskGeometryDefFormat(buf, def);
>      virDomainDiskBlockIoDefFormat(buf, def);
> +    virDomainDiskCacheDefFormat(buf, def);
> 
>      /* For now, mirroring is currently output-only: we only output it
>       * for live domains, therefore we ignore it on input except for
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 91a33cb..83ea9d3 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -647,6 +647,11 @@ struct _virDomainDiskDef {
>          unsigned int physical_block_size;
>      } blockio;
> 
> +    struct {
> +        unsigned int disk_l2_cache_size;
> +        unsigned int disk_cache_clean_interval;
> +    } disk_cache;
> +
>      virDomainBlockIoTuneInfo blkdeviotune;
> 
>      char *serial;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index e087891..7c47cd3 100755
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1662,6 +1662,13 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
>      if (qemuBuildDriveSourceStr(disk, cfg, &opt, qemuCaps) < 0)
>          goto error;
> 
> +    if (disk->disk_cache.disk_l2_cache_size > 0)
> +        virBufferAsprintf(&opt, "l2-cache-size=%u,",
> +                disk->disk_cache.disk_l2_cache_size);
> +    if (disk->disk_cache.disk_cache_clean_interval > 0)
> +        virBufferAsprintf(&opt, "cache-clean-interval=%u,",
> +                disk->disk_cache.disk_cache_clean_interval);
> +
>      if (emitDeviceSyntax)
>          virBufferAddLit(&opt, "if=none");
>      else
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 9f165c1..de334a7 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -5408,6 +5408,12 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr
> disk,
>      CHECK_EQ(ioeventfd, "ioeventfd", true);
>      CHECK_EQ(event_idx, "event_idx", true);
>      CHECK_EQ(copy_on_read, "copy_on_read", true);
> +
> +    CHECK_EQ(disk_cache.disk_l2_cache_size,
> +             "diskCache disk_l2_cache_size", true);
> +    CHECK_EQ(disk_cache.disk_cache_clean_interval,
> +             "diskCache disk_cache_clean_interval", true);
> +
>      /* "snapshot" is a libvirt internal field and thus can be changed */
>      /* startupPolicy is allowed to be updated. Therefore not checked
> here. */
>      CHECK_EQ(transient, "transient", true);
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.args
> b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.args
> index b405242..b968302 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.args
> @@ -22,7 +22,7 @@ QEMU_AUDIO_DRV=none \
>  -drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,media=cdrom,\
>  id=drive-ide0-1-0,readonly=on \
>  -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \
> --drive file=/tmp/data.img,format=raw,if=none,id=drive-virtio-disk0 \
> +-drive
> file=/tmp/data.img,format=qcow2,l2-cache-size=536870912,cache-clean-interval=900,if=none,id=drive-virtio-disk0
> \
>  -device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,\
>  id=virtio-disk0 \
>  -drive file=/tmp/logs.img,format=raw,if=none,id=drive-virtio-disk1 \
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml
> b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml
> index b843878..43298fb 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml
> @@ -28,8 +28,9 @@
>        <address type='drive' controller='0' bus='1' target='0' unit='0'/>
>      </disk>
>      <disk type='file' device='disk'>
> -      <driver name='qemu' type='raw'/>
> +      <driver name='qemu' type='qcow2'/>
>        <source file='/tmp/data.img'/>
> +      <diskCache disk_l2_cache_size='53687'
> disk_cache_clean_interval='900' />
>        <target dev='vda' bus='virtio'/>
>      </disk>
>      <disk type='file' device='disk'>
> 
> 
> Thanks
> 
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu:Add support L2 table cache for qcow2 disk
Posted by Pavel Hrdina 7 years, 7 months ago 
On Wed, Jun 27, 2018 at 03:50:00PM +0800, Allen Do wrote:
> The patch add support L2 table cache for qcow2 disk. L2 table cache can
> improve IO read and write performance for qcow2 img.
> Diff follows:
>  src/conf/domain_conf.c                               | 47
> +++++++++++++++++++++++++++++++++++++++++++++++
>  src/conf/domain_conf.h                               |  5 +++++
>  src/qemu/qemu_command.c                              |  7 +++++++
>  src/qemu/qemu_domain.c                               |  6 ++++++
>  tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.args |  2 +-
>  tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml  |  3 ++-
>  6 files changed, 68 insertions(+), 2 deletions(-)

Hi, thanks for the contribution, but this patch is obviously incorrectly
formatted and would not be possible to apply.

Please read the contribution guidelines [1] and resend the patch
properly using "git send-email".

Thanks,
Pavel

[1] <https://libvirt.org/hacking.html>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2026 Red Hat, Inc.