[libvirt] [PATCH v2 1/2] conf, docs: Add qcow2 cache configuration support

John Ferlan jferlan at redhat.com
Tue Sep 5 15:10:48 UTC 2017



On 09/03/2017 10:39 PM, Liu Qing wrote:
> Random write IOPS will drop dramatically if qcow2 l2 cache could not
> cover the whole disk. This patch gives libvirt user a chance to adjust
> the qcow2 cache configuration.
> 
> Three new qcow2 driver paramters are added. They are l2-cache-size,

parameters

> refcount-cache-size and cache-clean-interval.
> 
> The following are from qcow2-cache.txt.
> The amount of virtual disk that can be mapped by the L2 and refcount
> caches (in bytes) is:
> disk_size = l2_cache_size * cluster_size / 8
> disk_size = refcount_cache_size * cluster_size * 8 / refcount_bits
> 
> The parameter "cache-clean-interval" defines an interval (in seconds).
> All cache entries that haven't been accessed during that interval are
> removed from memory.
> 

OK - so it's in the commit message, but someone reading the docs isn't
going to read the commit message to find any useful suggestions...

> Signed-off-by: Liu Qing <liuqing at huayun.com>
> ---
>  docs/formatdomain.html.in                          | 21 +++++++++++
>  docs/schemas/domaincommon.rng                      | 24 +++++++++++++
>  src/conf/domain_conf.c                             | 30 ++++++++++++++++
>  src/qemu/qemu_command.c                            |  6 ++++
>  src/util/virstoragefile.c                          |  3 ++
>  src/util/virstoragefile.h                          |  4 +++
>  ...2argv-disk-drive-qcow2-cache-clean-interval.xml | 37 +++++++++++++++++++
>  ...qemuxml2argv-disk-drive-qcow2-l2-cache-size.xml | 37 +++++++++++++++++++
>  ...l2argv-disk-drive-qcow2-refcount-cache-size.xml | 37 +++++++++++++++++++
>  ...mlout-disk-drive-qcow2-cache-clean-interval.xml | 41 ++++++++++++++++++++++
>  ...muxml2xmlout-disk-drive-qcow2-l2-cache-size.xml | 41 ++++++++++++++++++++++
>  ...xmlout-disk-drive-qcow2-refcount-cache-size.xml | 41 ++++++++++++++++++++++
>  tests/qemuxml2xmltest.c                            |  3 ++
>  13 files changed, 325 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache-clean-interval.xml
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-l2-cache-size.xml
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-refcount-cache-size.xml
>  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-cache-clean-interval.xml
>  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-l2-cache-size.xml
>  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-refcount-cache-size.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 8ca7637..8f093b5 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3035,6 +3035,27 @@
>            <a href="#elementsVirtio">Virtio-specific options</a> can also be
>            set. (<span class="since">Since 3.5.0</span>)
>            </li>
> +          <li>
> +            The optional <code>l2_cache_size</code> attribute controls how much
> +            memory will be consumed by qcow2 l2 table cache. This option is
> +            only valid when the driver type is qcow2. The default size is 1MB.
> +            <span class='since'>Since 3.7.0</span>

What is the value listed in?  Seems to me from the sample XML it's in
bytes, but you didn't create a "sized" parameter so seeing 2097152 could
cause someone to think that's an MB value because you indicate the
default is 1MB instead of the byte equivalent.

I see that the QEMU code would error on sizing restrictions, so I
suppose we can avoid those tyeps of checks. Still just the wording here
doesn't really convey enough meaning to allow someone to "guess" a good
value to use. What's the basis of what someone should use to have a
solid guess?

> +          </li>
> +          <li>
> +            The optional <code>refcount_cache_size</code> attribute controls
> +            how much memory will be consumed by qcow2 reference count table
> +            cache. This option is only valid when the driver type is qcow2.
> +            The default size is 256KB.
> +            <span class='since'>Since 3.7.0</span>

Similar comments here w/r/t sizing and what's a good value.

I feel we've recently gone down this path adding tx_queue_size values
and not really having a good idea what to tell a user/consumer "how" to
realistically use the values.

> +          </li>
> +          <li>
> +            The optional <code>cache_clean_interval</code> attribute defines
> +            an interval (in seconds). All cache entries that haven't been
> +            accessed during that interval are removed from memory. This option
> +            is only valid when the driver type is qcow2. The defaut value is 0,

s/defaut/default

> +            which disables this feature.

So again is there a good range or suggested values? The default is to
never clean and this allows some type of cleanup. So what's the downside
of too short or too long.  Is a single second an eternity in a world
that uses milliseconds or is it too short because of cache hits? It's
almost too bad QEMU didn't implement something like "clean=aggressive"
or "clean=passive" in order to choose between 1 or 2 algorithms managed
by QEMU to perform the cleanup with "clean=never" being the default.

> +            <span class='since'>Since 3.7.0</span>
> +          </li>

It'll be at least 3.8.0 since 3.7.0 is was released yesterday.

It would seem to me that a good caveat for each of these is don't mess
with them unless you know what you're doing.  But for those that need
performance adjustments, then here's some suggestions and guidelines.

>          </ul>
>        </dd>
>        <dt><code>backenddomain</code></dt>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 06c5a91..5545179 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1797,6 +1797,15 @@
>          <ref name="driverIOThread"/>
>        </optional>
>        <optional>
> +        <ref name="qcow2_l2_cache_size"/>
> +      </optional>
> +      <optional>
> +        <ref name="qcow2_refcount_cache_size"/>
> +      </optional>
> +      <optional>
> +        <ref name="qcow2_cache_clean_interval"/>
> +      </optional>
> +      <optional>

You know - it almost seems as though this could/should a <driver>
sub-element such as:

   <qcow2_attr l2_cache_size='#' refcount_cache_size='#'
clean_interval='#'/qcow2_attr>  (or just <qcow2>)

Maybe others also have an opinion on this.  Just seems "odd" to have
attributes starting with "qcow2_" for <driver...> where it'd be required
to also ensure that the attribute "type='qcow2'".

I also see there are some other qcow2 specific attributes that may be
useful now or even in the future. But in the end, they're specific to an
implementation of the driver and aren't general attributes for a driver.

>          <ref name="detect_zeroes"/>
>        </optional>
>        <ref name="virtioOptions"/>
> @@ -1895,6 +1904,21 @@
>        </choice>
>      </attribute>
>    </define>
> +  <define name="qcow2_l2_cache_size">
> +    <attribute name='l2_cache_size'>
> +      <ref name="unsignedInt"/>
> +    </attribute>
> +  </define>
> +  <define name="qcow2_refcount_cache_size">
> +    <attribute name='refcount_cache_size'>
> +      <ref name="unsignedInt"/>
> +    </attribute>
> +  </define>
> +  <define name="qcow2_cache_clean_interval">
> +    <attribute name='cache_clean_interval'>
> +      <ref name="unsignedInt"/>
> +    </attribute>
> +  </define>
>    <define name="controller">
>      <element name="controller">
>        <optional>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index f7574d7..f07ab72 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -8600,6 +8600,30 @@ virDomainDiskDefDriverParseXML(virDomainDiskDefPtr def,
>          VIR_FREE(tmp);
>      }
>  
> +    if ((tmp = virXMLPropString(cur, "l2_cache_size")) &&
> +        (virStrToLong_ui(tmp, NULL, 10, &def->src->l2_cache_size) < 0)) {
> +        virReportError(VIR_ERR_XML_ERROR,
> +                       _("Invalid l2_cache_size attribute in disk driver element: %s"),
> +                       tmp);
> +        goto cleanup;
> +    }
> +
> +    if ((tmp = virXMLPropString(cur, "refcount_cache_size")) &&
> +        (virStrToLong_ui(tmp, NULL, 10, &def->src->refcount_cache_size) < 0)) {
> +        virReportError(VIR_ERR_XML_ERROR,
> +                       _("Invalid refcount_cache_size attribute in disk driver element: %s"),
> +                       tmp);
> +        goto cleanup;
> +    }
> +
> +    if ((tmp = virXMLPropString(cur, "cache_clean_interval")) &&
> +        (virStrToLong_ui(tmp, NULL, 10, &def->src->cache_clean_interval) < 0)) {
> +        virReportError(VIR_ERR_XML_ERROR,
> +                       _("Invalid cache_clean_interval attribute in disk driver element: %s"),
> +                       tmp);
> +        goto cleanup;
> +    }
> +

These should all be virStrToLong_ullp since you're expecting a positive
or unsigned value. Furthermore, 'int' or 'unsigned' (e.g. _ui) limits
you a bit with byte values, you should follow what QEMU has.

You may even want to consider allowing "sized" values (search for
virDomainParseScaledValue for examples) so that someone could supply the
value in something other than bytes. You still store in bytes, but
visually someone can provide 1Mb rather than being required to do the
math themselves. Of course that comes with even more XML parsing logic.

Also once all the parsing is done we should modify
virDomainDiskDefParseValidate in order to validate that these were only
added for disk->src->format == VIR_STORAGE_FILE_QCOW2 and error if added
to any other format.

>      if ((tmp = virXMLPropString(cur, "detect_zeroes")) &&
>          (def->detect_zeroes = virDomainDiskDetectZeroesTypeFromString(tmp)) <= 0) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -21897,6 +21921,12 @@ virDomainDiskDefFormat(virBufferPtr buf,
>          virBufferAsprintf(&driverBuf, " iothread='%u'", def->iothread);
>      if (def->detect_zeroes)
>          virBufferAsprintf(&driverBuf, " detect_zeroes='%s'", detect_zeroes);
> +    if (def->src->l2_cache_size > 0)
> +        virBufferAsprintf(&driverBuf, " l2_cache_size='%u'", def->src->l2_cache_size);
> +    if (def->src->refcount_cache_size > 0)
> +        virBufferAsprintf(&driverBuf, " refcount_cache_size='%u'", def->src->refcount_cache_size);
> +    if (def->src->cache_clean_interval > 0)
> +        virBufferAsprintf(&driverBuf, " cache_clean_interval='%u'", def->src->cache_clean_interval);

Again this could/would change if using a subelement as well as using
"%llu" as the format string.

Also these are some really long lines - consider someone using an 80
column wide terminal...

>  
>      virDomainVirtioOptionsFormat(&driverBuf, def->virtio);
>  
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 9a27987..7996eed 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1430,6 +1430,12 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
>              qemuformat = "luks";
>          virBufferAsprintf(buf, "format=%s,", qemuformat);
>      }
> +    if (disk->src->format == VIR_STORAGE_FILE_QCOW2 && disk->src->l2_cache_size > 0)
> +        virBufferAsprintf(buf, "l2-cache-size=%u,", disk->src->l2_cache_size);
> +    if (disk->src->format == VIR_STORAGE_FILE_QCOW2 && disk->src->refcount_cache_size > 0)
> +        virBufferAsprintf(buf, "refcount-cache-size=%u,", disk->src->refcount_cache_size);
> +    if (disk->src->format == VIR_STORAGE_FILE_QCOW2 && disk->src->cache_clean_interval > 0)
> +        virBufferAsprintf(buf, "cache-clean-interval=%u,", disk->src->cache_clean_interval);
>  

Long lines here too... But this actually belongs in the next patch, not
here as all you're doing in the first patch is the xml processing.

With the Validation done the format check wouldn't be required either.

>      ret = 0;
>  
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index fbc8245..c685331 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -2038,6 +2038,9 @@ virStorageSourceCopy(const virStorageSource *src,
>      ret->physical = src->physical;
>      ret->readonly = src->readonly;
>      ret->shared = src->shared;
> +    ret->l2_cache_size = src->l2_cache_size;
> +    ret->refcount_cache_size = src->refcount_cache_size;
> +    ret->cache_clean_interval = src->cache_clean_interval;
>  
>      /* storage driver metadata are not copied */
>      ret->drv = NULL;
> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
> index 6c388b1..e7889d9 100644
> --- a/src/util/virstoragefile.h
> +++ b/src/util/virstoragefile.h
> @@ -280,6 +280,10 @@ struct _virStorageSource {
>      /* metadata that allows identifying given storage source */
>      char *nodeformat;  /* name of the format handler object */
>      char *nodestorage; /* name of the storage object */
> +
> +    unsigned l2_cache_size; /* qcow2 l2 cache size */
> +    unsigned refcount_cache_size; /* qcow2 reference count table cache size */
> +    unsigned cache_clean_interval; /* clean unused cache entries interval */

QEMU defines these as as int64_t

    int64_t l2_cache_size;
    int64_t refcount_cache_size;
    int64_t cache_clean_interval;

But uses them as uint64_t, so that's what we should do - define them as
uint64_t

>  };
>  
>  
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache-clean-interval.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache-clean-interval.xml
> new file mode 100644
> index 0000000..1c3174d
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache-clean-interval.xml
> @@ -0,0 +1,37 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219136</memory>
> +  <currentMemory unit='KiB'>219136</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-i686</emulator>
> +    <disk type='block' device='disk'>
> +      <driver name='qemu' type='qcow2' cache='none' cache_clean_interval='900'/>

900 seconds!  I dunno, seems like a long time to me!

> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <disk type='block' device='cdrom'>
> +      <driver name='qemu' type='raw'/>
> +      <source dev='/dev/HostVG/QEMUGuest2'/>
> +      <target dev='hdc' bus='ide'/>
> +      <readonly/>
> +      <address type='drive' controller='0' bus='1' target='0' unit='0'/>
> +    </disk>
> +    <controller type='usb' index='0'/>
> +    <controller type='ide' index='0'/>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <memballoon model='none'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-l2-cache-size.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-l2-cache-size.xml
> new file mode 100644
> index 0000000..ebe4c3c
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-l2-cache-size.xml
> @@ -0,0 +1,37 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219136</memory>
> +  <currentMemory unit='KiB'>219136</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-i686</emulator>
> +    <disk type='block' device='disk'>
> +      <driver name='qemu' type='qcow2' cache='none' l2_cache_size='2097152'/>

a/k/a 2 Mb

> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <disk type='block' device='cdrom'>
> +      <driver name='qemu' type='raw'/>
> +      <source dev='/dev/HostVG/QEMUGuest2'/>
> +      <target dev='hdc' bus='ide'/>
> +      <readonly/>
> +      <address type='drive' controller='0' bus='1' target='0' unit='0'/>
> +    </disk>
> +    <controller type='usb' index='0'/>
> +    <controller type='ide' index='0'/>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <memballoon model='none'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-refcount-cache-size.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-refcount-cache-size.xml
> new file mode 100644
> index 0000000..e8835e2
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-refcount-cache-size.xml
> @@ -0,0 +1,37 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219136</memory>
> +  <currentMemory unit='KiB'>219136</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-i686</emulator>
> +    <disk type='block' device='disk'>
> +      <driver name='qemu' type='qcow2' cache='none' refcount_cache_size='524288'/>

a/k/a 512 Kb

> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <disk type='block' device='cdrom'>
> +      <driver name='qemu' type='raw'/>
> +      <source dev='/dev/HostVG/QEMUGuest2'/>
> +      <target dev='hdc' bus='ide'/>
> +      <readonly/>
> +      <address type='drive' controller='0' bus='1' target='0' unit='0'/>
> +    </disk>
> +    <controller type='usb' index='0'/>
> +    <controller type='ide' index='0'/>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <memballoon model='none'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-cache-clean-interval.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-cache-clean-interval.xml
> new file mode 100644
> index 0000000..6c1934f
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-cache-clean-interval.xml
> @@ -0,0 +1,41 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219136</memory>
> +  <currentMemory unit='KiB'>219136</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-i686</emulator>
> +    <disk type='block' device='disk'>
> +      <driver name='qemu' type='qcow2' cache='none' cache_clean_interval='900'/>
> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <disk type='block' device='cdrom'>
> +      <driver name='qemu' type='raw'/>
> +      <source dev='/dev/HostVG/QEMUGuest2'/>
> +      <target dev='hdc' bus='ide'/>
> +      <readonly/>
> +      <address type='drive' controller='0' bus='1' target='0' unit='0'/>
> +    </disk>
> +    <controller type='usb' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
> +    </controller>
> +    <controller type='ide' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
> +    </controller>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <memballoon model='none'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-l2-cache-size.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-l2-cache-size.xml
> new file mode 100644
> index 0000000..4da0e4d
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-l2-cache-size.xml
> @@ -0,0 +1,41 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219136</memory>
> +  <currentMemory unit='KiB'>219136</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-i686</emulator>
> +    <disk type='block' device='disk'>
> +      <driver name='qemu' type='qcow2' cache='none' l2_cache_size='2097152'/>

a/k/a 2 Mb

> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <disk type='block' device='cdrom'>
> +      <driver name='qemu' type='raw'/>
> +      <source dev='/dev/HostVG/QEMUGuest2'/>
> +      <target dev='hdc' bus='ide'/>
> +      <readonly/>
> +      <address type='drive' controller='0' bus='1' target='0' unit='0'/>
> +    </disk>
> +    <controller type='usb' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
> +    </controller>
> +    <controller type='ide' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
> +    </controller>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <memballoon model='none'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-refcount-cache-size.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-refcount-cache-size.xml
> new file mode 100644
> index 0000000..bda4574
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-refcount-cache-size.xml
> @@ -0,0 +1,41 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219136</memory>
> +  <currentMemory unit='KiB'>219136</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-i686</emulator>
> +    <disk type='block' device='disk'>
> +      <driver name='qemu' type='qcow2' cache='none' refcount_cache_size='524288'/>

a/k/a 512 Kb

> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <disk type='block' device='cdrom'>
> +      <driver name='qemu' type='raw'/>
> +      <source dev='/dev/HostVG/QEMUGuest2'/>
> +      <target dev='hdc' bus='ide'/>
> +      <readonly/>
> +      <address type='drive' controller='0' bus='1' target='0' unit='0'/>
> +    </disk>
> +    <controller type='usb' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
> +    </controller>
> +    <controller type='ide' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
> +    </controller>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <memballoon model='none'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 311b713..a1af6e5 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -461,6 +461,9 @@ mymain(void)
>      DO_TEST("disk-drive-cache-v2-none", NONE);
>      DO_TEST("disk-drive-cache-directsync", NONE);
>      DO_TEST("disk-drive-cache-unsafe", NONE);
> +    DO_TEST("disk-drive-qcow2-l2-cache-size", NONE);
> +    DO_TEST("disk-drive-qcow2-refcount-cache-size", NONE);
> +    DO_TEST("disk-drive-qcow2-cache-clean-interval", NONE);

FWIW: You could have gone with just 1 test, but all 3 are fine since
they show they can be used separately. In the long run, we're just
trying to validate properly read and formatted values.

Doing them separately could also imply that each parameter should be
added in it's own separate patch...

John

>      DO_TEST("disk-drive-network-nbd", NONE);
>      DO_TEST("disk-drive-network-nbd-export", NONE);
>      DO_TEST("disk-drive-network-nbd-ipv6", NONE);
> 




More information about the libvir-list mailing list