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