Re: [Qemu-devel] Bug#757927: [qemu-kvm] TRIM (discard=unmap) broken in 2.1

Re: [Qemu-devel] Bug#757927: [qemu-kvm] TRIM (discard=unmap) broken in 2.1

[Michael Tokarev]

[This is http://bugs.debian.org/757927 -- trim stopped working in qemu 2.1]

12.08.2014 18:46, Michael Tokarev wrote:
> 12.08.2014 17:35, Roman Mamedov wrote:
>> Package: qemu-kvm
>> Version: 2.1+dfsg-2~bpo70+2
>> Severity: normal
>>
>> Hello,
>>
>> I was able to successfully use the passthrough TRIM support with an IDE
>> interface virtual disk in Qemu-KVM version 2.0.
>>
>> However after upgrading to 2.1 and restarting my VM, TRIM now fails in it with
>> messages like those quoted below (dmesg from the guest). There are no messages
>> in dmesg related to that on the host.

BTW, I found a system here with an SSD which supports discard, and tried my
guests against it -- everything works fine when I use sata/ahci, but it breaks
indeed when using the default IDE interface.

But applying the oneliner:

> It is the same as 2.1+dfsg-1~bpo70+2 but with a one-line patch
> added:
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index db191a6..7256592 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -688,7 +688,8 @@ void ide_dma_cb(void *opaque, int ret)
>             sector_num, n, s->dma_cmd);
>  #endif
> 
> -    if (!ide_sect_range_ok(s, sector_num, n)) {
> +    if ((s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) &&
> +        !ide_sect_range_ok(s, sector_num, n)) {
>          dma_buf_commit(s);
>          ide_dma_error(s);
>          return;


fixes the prob for me.  So apparently kwolf was right pointing to that
commit 58ac3211.

I'll upload a new version soon.

Thanks,

/mjt

Re: [Qemu-devel] Bug#757927: [qemu-kvm] TRIM (discard=unmap) broken in 2.1

[Roman Mamedov]

[-- Attachment #1: Type: text/plain, Size: 1148 bytes --]

On Wed, 13 Aug 2014 10:05:51 +0400
Michael Tokarev <mjt@tls.msk.ru> wrote:

> BTW, I found a system here with an SSD which supports discard, and tried my
> guests against it -- everything works fine when I use sata/ahci, but it breaks
> indeed when using the default IDE interface.

Hello,

Thanks for working on this! Sorry for not being able to test an updated
version sooner.

Something to note, you do not need any TRIM-capable hardware such as SSD on the
host to take advantage of TRIM passthrough. Just place the RAW format VM disk
image on any filesystem which supports sparse files (e.g. Ext4 or Btrfs), and
when the guest system issues a TRIM, when using "discard=unmap" the VM image
file will be "re-sparsifyed" on the host filesystem, deallocating all the
TRIMed areas from disk storage, in effect it will take much less space on disk
than before:
http://s.lowendshare.com/7/1407910491.666.2014-07-16T130602Z-trim.png

I was only successful in using this with the virtual IDE interface disks on
Qemu/KVM 2.0, the virtio mode does not seem to support TRIM; did not try with
sata/ahci.

-- 
With respect,
Roman

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [Qemu-devel] Bug#757927: [qemu-kvm] TRIM (discard=unmap) broken in 2.1

[Michael Tokarev]

13.08.2014 10:16, Roman Mamedov wrote:

> Thanks for working on this! Sorry for not being able to test an updated
> version sooner.

You're welcome.  I learned something too ;)

[]
> I was only successful in using this with the virtual IDE interface disks on
> Qemu/KVM 2.0, the virtio mode does not seem to support TRIM; did not try with
> sata/ahci.

virtio-scsi support trim, and also sata/ahci.  virtio-blk does not, it is a
simple interface without fancy features.

Thanks,

/mjt

Re: [Qemu-devel] Bug#757927: [qemu-kvm] TRIM (discard=unmap) broken in 2.1

[Michael Tokarev]

[dropping debian bugreport]

13.08.2014 10:05, Michael Tokarev wrote:
> [This is http://bugs.debian.org/757927 -- trim stopped working in qemu 2.1]
> 
> 12.08.2014 18:46, Michael Tokarev wrote:
>> 12.08.2014 17:35, Roman Mamedov wrote:
>>> Package: qemu-kvm
>>> Version: 2.1+dfsg-2~bpo70+2
>>> Severity: normal
>>>
>>> Hello,
>>>
>>> I was able to successfully use the passthrough TRIM support with an IDE
>>> interface virtual disk in Qemu-KVM version 2.0.
>>>
>>> However after upgrading to 2.1 and restarting my VM, TRIM now fails in it with
>>> messages like those quoted below (dmesg from the guest). There are no messages
>>> in dmesg related to that on the host.
> 
> BTW, I found a system here with an SSD which supports discard, and tried my
> guests against it -- everything works fine when I use sata/ahci, but it breaks
> indeed when using the default IDE interface.
> 
> But applying the oneliner:
> 
>> It is the same as 2.1+dfsg-1~bpo70+2 but with a one-line patch
>> added:
>>
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index db191a6..7256592 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -688,7 +688,8 @@ void ide_dma_cb(void *opaque, int ret)
>>             sector_num, n, s->dma_cmd);
>>  #endif
>>
>> -    if (!ide_sect_range_ok(s, sector_num, n)) {
>> +    if ((s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) &&
>> +        !ide_sect_range_ok(s, sector_num, n)) {
>>          dma_buf_commit(s);
>>          ide_dma_error(s);
>>          return;
> 
> fixes the prob for me.  So apparently kwolf was right pointing to that
> commit 58ac3211.

But this makes me wonder: why the issue only exists with ide, but not
with ahci/sata?  The two should be sufficiently similar to trigger this
check the same way.  Maybe we have another bug somewhere, like adding
an extra sector in some path, which is triggered by 58ac3211?  Maybe
even in the guest kernel (but in this case, since it works with real
hw, we should ignore it).

Thanks,

/mjt

Re: [Qemu-devel] Bug#757927: [qemu-kvm] TRIM (discard=unmap) broken in 2.1

[Kevin Wolf]

Am 13.08.2014 um 08:41 hat Michael Tokarev geschrieben:
> [dropping debian bugreport]
> 
> 13.08.2014 10:05, Michael Tokarev wrote:
> > [This is http://bugs.debian.org/757927 -- trim stopped working in qemu 2.1]
> > 
> > 12.08.2014 18:46, Michael Tokarev wrote:
> >> 12.08.2014 17:35, Roman Mamedov wrote:
> >>> Package: qemu-kvm
> >>> Version: 2.1+dfsg-2~bpo70+2
> >>> Severity: normal
> >>>
> >>> Hello,
> >>>
> >>> I was able to successfully use the passthrough TRIM support with an IDE
> >>> interface virtual disk in Qemu-KVM version 2.0.
> >>>
> >>> However after upgrading to 2.1 and restarting my VM, TRIM now fails in it with
> >>> messages like those quoted below (dmesg from the guest). There are no messages
> >>> in dmesg related to that on the host.
> > 
> > BTW, I found a system here with an SSD which supports discard, and tried my
> > guests against it -- everything works fine when I use sata/ahci, but it breaks
> > indeed when using the default IDE interface.
> > 
> > But applying the oneliner:
> > 
> >> It is the same as 2.1+dfsg-1~bpo70+2 but with a one-line patch
> >> added:
> >>
> >> diff --git a/hw/ide/core.c b/hw/ide/core.c
> >> index db191a6..7256592 100644
> >> --- a/hw/ide/core.c
> >> +++ b/hw/ide/core.c
> >> @@ -688,7 +688,8 @@ void ide_dma_cb(void *opaque, int ret)
> >>             sector_num, n, s->dma_cmd);
> >>  #endif
> >>
> >> -    if (!ide_sect_range_ok(s, sector_num, n)) {
> >> +    if ((s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) &&
> >> +        !ide_sect_range_ok(s, sector_num, n)) {
> >>          dma_buf_commit(s);
> >>          ide_dma_error(s);
> >>          return;
> > 
> > fixes the prob for me.  So apparently kwolf was right pointing to that
> > commit 58ac3211.
> 
> But this makes me wonder: why the issue only exists with ide, but not
> with ahci/sata?  The two should be sufficiently similar to trigger this
> check the same way.  Maybe we have another bug somewhere, like adding
> an extra sector in some path, which is triggered by 58ac3211?  Maybe
> even in the guest kernel (but in this case, since it works with real
> hw, we should ignore it).

I can't give you a clear answer, but it all depends on the value of
sector_num. This is the contents of the LBA registers and unused for
TRIM (spec says it's reserved, so we shouldn't be looking at it).
Whether the problematic value in the IDE case comes from a previous
command or that's just how the driver sets things and the AHCI sets
different things I can't say. But it certainly depends on the guest
behaviour whether the bug actually triggers.

Kevin