On Tue, Jul 18, 2023 at 9:31 PM Peter Krempa <pkre...@redhat.com> wrote:

> On Sun, Jul 16, 2023 at 22:36:21 +0800, ~hyman wrote:
> > From: Hyman Huang(黄勇) <yong.hu...@smartx.com>
> >
> > Add 'virtio_discard' and 'virtio_write_zeroes' attribute to control
> > whether discard and write-zeroes requests are handled by the
> > virtio-blk device.
> >
> > To distinguish the attributes in block drive layer, use the 'virtio'
> > prefix to indicate that these attributes are specific for virtio-blk.
> >
> > Signed-off-by: Hyman Huang(黄勇) <yong.hu...@smartx.com>
> > ---
> >  docs/formatdomain.rst             |  8 ++++++++
> >  src/conf/domain_conf.c            | 16 ++++++++++++++++
> >  src/conf/domain_conf.h            |  2 ++
> >  src/conf/schemas/domaincommon.rng | 10 ++++++++++
> >  src/conf/storage_source_conf.c    |  2 ++
> >  src/conf/storage_source_conf.h    |  2 ++
> >  src/qemu/qemu_domain.c            |  2 ++
> >  src/qemu/qemu_driver.c            |  4 +++-
> >  src/vz/vz_utils.c                 | 12 ++++++++++++
> >  9 files changed, 57 insertions(+), 1 deletion(-)
> >
> > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> > index 4af0b82569..7be12ff08e 100644
> > --- a/docs/formatdomain.rst
> > +++ b/docs/formatdomain.rst
> > @@ -3259,6 +3259,14 @@ paravirtualized driver is specified via the
> ``disk`` element.
> >        value can be either "unmap" (allow the discard request to be
> passed) or
> >        "ignore" (ignore the discard request). :since:`Since 1.0.6 (QEMU
> and KVM
> >        only)`
> > +   -  The optional ``virtio_discard`` and ``virtio_write_zeroes`` are
> attributes
> > +      that control whether discard and write-zeroes requests are
> handled by the
> > +      virtio-blk device. The feature is based on DISCARD and
> WRITE_ZEROES
> > +      commands introduced in virtio-blk protocol to improve performance
> when
> > +      using SSD backend. The value can be either 'on' or 'off'. Note
> that
> > +      ``discard`` and ``write_zeroes`` implementations in the block
> drive layer
> > +      are parts of the feature logically and should be turned on when
> enabling
> > +      the feature. :since:`Since 9.6.0 (QEMU and KVM only)`
>
> Based on current released qemu both 'discard' and 'write-zeroes' feature
> of the 'virtio-blk' device is enabled by default:
>
>  $ qemu-system-x86_64 -device virtio-blk-pci,? | grep discard
>    discard=<bool>         - on/off (default: true)
>    discard_granularity=<size> -  (default: 4294967295)
>    max-discard-sectors=<uint32> -  (default: 4194303)
>    report-discard-granularity=<bool> -  (default: true)
>  $ qemu-system-x86_64 -device virtio-blk-pci,? | grep write-zeroes
>    max-write-zeroes-sectors=<uint32> -  (default: 4194303)
>    write-zeroes=<bool>    - on/off (default: true)
>
> Do you need a way to disable this feature? Based on the description the
> default seems to be sane and actually what you'd want users to set.
>
The default is reasonable indeed. But there are still some scenarios
in the production environment that discard may  need to be disabled.
For example:
- The virtio-blk discard/write-zeroes commands was introduced in
  2017 in virtio-blk spec, so the OS distribution before 2017 can not
  use this feature. In that case, the cloud management platform(CMP)
  could recognize this issue and disable the feature in advance.

- Discard/write-zeroes may need to be configured at disk granularity
  in some scenarios. Assume that CMP support 2 distributed storage
  clusters,  one cluster supports discard and another does not.
  If the VM is configured with 2 disks - vda, vdb. Which are located in
  2 clusters respectively. Or, the VM with disks located in the
  discard-supportive cluster and want to migrate disks to another
  cluster for some reason(eg. storage capability is exhausted)
  CMP may want to turn discard off explicitly.

Though the above scenarios are quite rare and the virtio spec can
ensure the feature can be negotiated and used correctly.
CMP still wants to control the features it supports carefully and
precisely.
To summarise, IMHO, libvirt may not forbid the upper layer to
control the 2 features of the virtio-blk disk. Leaving the option
configurable for CMP or even customers.

There's one more background may need to be stated:
Current released QEMU do not provide hmp/qmp to query the
final features that are negotiated between front-end and back-end
from the hypervisor side. So if CMP wants to query what features a
guest VM uses, it has to query it inside the guest VM or hack the
qemu process. This way is inconvenient for control-planes, so the CMP
needs to control the feature as aggressively as it can.

Thank Peter for the attention to this patchset.
Yong

>
> The feature was introduced to qemu by:
>
> commit 5c81161f804144b146607f890e84613a4cbad95c
> Author: Stefano Garzarella <sgarz...@redhat.com>
> Date:   Thu Feb 21 11:33:07 2019 +0100
>
>     virtio-blk: add "discard" and "write-zeroes" properties
>
>     In order to avoid migration issues, we enable DISCARD and
>     WRITE_ZEROES features only for machine type >= 4.0
>
>     As discussed with Michael S. Tsirkin and Stefan Hajnoczi on the
>     list [1], DISCARD operation should not have security implications
>     (eg. page cache attacks), so we can enable it by default.
>
> The default was always enabled for all new machine types, so you should
> only ever need to update your machine type.
>
>

-- 
Best regards

Reply via email to