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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
>  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

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]