[libvirt] [PATCH v3 1/2] conf: Add support of zero-detection for disks

[Peter Krempa]

On Thu, Apr 28, 2016 at 16:56:55 +0200, Martin Kletzander wrote:
> This option allows or disallows detection of zero-writes if it is set to
> "on" or "off", respectively.  It can be also set to "unmap" in which
> case it will try discarding that part of image based on the value of the
> "discard" option.

Neither this ...

> 
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
>  docs/formatdomain.html.in                          | 10 +++++
>  docs/schemas/domaincommon.rng                      | 12 ++++++
>  src/conf/domain_conf.c                             | 21 +++++++++-
>  src/conf/domain_conf.h                             | 11 ++++++
>  src/libvirt_private.syms                           |  2 +
>  .../qemuxml2argv-disk-drive-detect-zeroes.xml      | 45 ++++++++++++++++++++++
>  .../qemuxml2xmlout-disk-drive-detect-zeroes.xml    |  1 +
>  tests/qemuxml2xmltest.c                            |  1 +
>  8 files changed, 101 insertions(+), 2 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-detect-zeroes.xml
>  create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-detect-zeroes.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index d2db53b41823..3d09f511a8c4 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2564,6 +2564,16 @@
>              are numbered from 1 to the domain iothreads value.
>              <span class='since'>Since 1.2.8 (QEMU only)</span>
>            </li>
> +          <li>
> +            The optional <code>detect_zeroes</code> attribute controls
> +            whether to detect zero write requests.  The value can be "off",
> +            "on" or "unmap".  First two values turn the detection off and
> +            on, respectively, the third value turns the detection on and

... nor this does document the "on" option in a way that would describe
what it actually does.

Without the documentation it's not really clear what to expect in qemu
or what to map this to in other hypervisors.


> +            tries to discard such areas from the image based on the value
> +            of <code>discard</code> above (it will act as "on" if
> +            <code>discard</code> is set to "ignore").

This is behavior of qemu so it should be marked as so.

> +            <span class='since'>Since 1.3.1 (QEMU and KVM only)</span>

Sorry for delaying the review. The version is unfortunately incorrect.

Also why "KVM only"? Does qemu reject it in TCG mode? Shouldn't we then
forbid trying to use it in such case (comment for next patch).

Additionally it's not clear what happens if the storage technology does
not support unmaping of the sectors.

> +          </li>
>          </ul>
>        </dd>
>        <dt><code>backenddomain</code></dt>

[...]

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index d8bed670f243..1a336d08cda5 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c

[...]

> @@ -7062,7 +7068,7 @@ virDomainDiskDefDriverParseXML(virDomainDiskDefPtr def,
>          } else {
>              if ((def->src->format = virStorageFileFormatTypeFromString(tmp)) <= 0) {
>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                               _("unknown driver format value '%s'"), tmp);
> +                               _("unknown driver type value '%s'"), tmp);

Unrelated change.

>                  goto cleanup;
>              }
>          }

[...]

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160429/39bbff69/attachment-0001.sig>