Re: [PATCH v2 2/3] conf: support for virtio-blk-pci discard option

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

 



On Wed, Sep 08, 2021 at 11:56:02 +0200, Peter Krempa wrote:
> On Tue, Sep 07, 2021 at 15:12:09 +0800, yuxiating@xxxxxxxxxx wrote:
> > From: yuxiating <yuxiating@xxxxxxxxxx>
> > 
> > DISCARD and WRITE_ZEROES features for machine type >= 4.0 is enabled by default
> > since 5c81161f8041("virtio-blk: add "discard" and "write-zeroes" properties).
> > Virtio_blk kernel driver has a bug that causes memory corruption in
> > virtblk_setup_discard_write_zeroes();
> > af822aa68fbd ("block: virtio_blk: fix handling single range discard request")
> > has fix it.
> > However, some operating systems are not fixed and need to disabled on the
> > QEMU side.
> 
> I must say that I'm not persuaded that there's enough value in this
> workaround. Similarly users could use virtio-scsi instead as a
> workaround or fix their OS.
> 
> For me this reasoning will not be enough, but other on the list might
> think otherwise.

I want to elaborate a bit. Detecting whether a guest OS has a fix is
hard and additionally the guest can be patched without restart of the VM
where the user would lose discards even when it isn't a problem any
more.

This brings me to another thing I forgot to point out. Using
discard_enable='off' must be made incompatible with discard='unmap'.

e.g.

    <disk type='file' device='disk'>
      <driver name='qemu' type='raw' discard_enable='off' discard='unmap'/>
      <source file='/tmp/data.img'/>
      <target dev='vda' bus='virtio'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </disk>

XML must be rejected by the validator as discards would not be
propagated to the file even on an explicit request of the user.

I'm also slightly unhappy that we are stashing frontend config under
<driver> but there's prior art in doing that so that doesn't need to be
changed.




[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]

  Powered by Linux