[v9] Optimize VMDK I/O by allocating multiple clusters

[Qemu-devel] [PATCH v9 0/8] Optimize VMDK I/O by allocating multiple clusters

Posted by Ashijeet Acharya 6 years, 6 months ago

Previously posted series patches:
v1 - http://lists.nongnu.org/archive/html/qemu-devel/2017-03/msg02044.html
v2 - http://lists.nongnu.org/archive/html/qemu-devel/2017-03/msg05080.html
v3 - http://lists.nongnu.org/archive/html/qemu-devel/2017-04/msg00074.html
v4 - http://lists.nongnu.org/archive/html/qemu-devel/2017-04/msg03851.html
v5 - http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg00929.html
v6 - http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg00947.html
v7 - http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg06600.html
v8 - http://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg08623.html

This series helps to optimize the I/O performance of VMDK driver.

Patch 1 helps us to move vmdk_find_offset_in_cluster.

Patch 2 & 3 perform a simple function re-naming tasks.

Patch 4 is used to factor out metadata loading code and implement it in separate
functions. This will help us to avoid code duplication in future patches of this
series.

Patch 5 helps to set the upper limit of the bytes handled in one cycle.

Patch 6 adds new functions to help us allocate multiple clusters according to
the size requested, perform COW if required and return the offset of the first
newly allocated cluster.

Patch 7 changes the metadata update code to update the L2 tables for multiple
clusters at once.

Patch 8 helps us to finally change vmdk_get_cluster_offset() to find cluster
offset only as cluster allocation task is now handled by vmdk_alloc_clusters()

Optimization test results:

This patch series improves 128 KB sequential write performance to an
empty VMDK file by 54%

Benchmark command: ./qemu-img bench -w -c 1024 -s 128K -d 1 -t none -f
vmdk test.vmdk

Changes in v9:
- rebase the series

Changes in v8:
- fix minor variable naming issue in patch 6

Changes in v7:
- comment the use of MIN() in calculating skip_end_bytes
- use extent->cluster_sectors instead of 128
- place check for m_data != NULL
- use g_new0(VmdkMetaData, 1) instead of g_malloc0(sizeof(*m_data))

Changes in v6:
- rename total_alloc_clusters as alloc_clusters_counter (fam)

Changes in v5:
- fix commit message and comment in patch 4 (fam)
- add vmdk_ prefix to handle_alloc() (fam)
- fix alignment issue in patch 6 (fam)
- use BDRV_SECTOR_BITS (fam)
- fix endianness calculation in patch 7 (fam)

Changes in v4:
- fix commit message in patch 1 (fam)
- drop size_to_clusters() function (fam)
- fix grammatical errors in function documentations (fam)
- factor out metadata loading coding in a separate patch (patch 4) (fam)
- rename vmdk_alloc_cluster_offset() to vmdk_alloc_clusters() (fam)
- break patch 4(in v3) into separate patches (patch 3 and 8) (fam)
- rename extent_size to extent_end (fam)
- use QEMU_ALIGN_UP instead of vmdk_align_offset. (fam)
- drop next and simply do m_data = m_data->next (fam)

Changes in v3:
- move size_to_clusters() from patch 1 to 3 (fam)
- use DIV_ROUND_UP in size_to_clusters (fam)
- make patch 2 compilable (fam)
- rename vmdk_L2update as vmdk_l2update and use UINT32_MAX (fam)
- combine patch 3 and patch 4 (as in v2) to make them compilable (fam)
- call bdrv_pwrite_sync() for batches of atmost 512 clusters at once (fam)

Changes in v2:
- segregate the ugly Patch 1 in v1 into 6 readable and sensible patches
- include benchmark test results in v2

Ashijeet Acharya (8):
  vmdk: Move vmdk_find_offset_in_cluster() to the top
  vmdk: Rename get_whole_cluster() to vmdk_perform_cow()
  vmdk: Rename get_cluster_offset() to vmdk_get_cluster_offset()
  vmdk: Factor out metadata loading code out of
    vmdk_get_cluster_offset()
  vmdk: Set maximum bytes allocated in one cycle
  vmdk: New functions to assist allocating multiple clusters
  vmdk: Update metadata for multiple clusters
  vmdk: Make vmdk_get_cluster_offset() return cluster offset only

 block/vmdk.c | 538 +++++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 416 insertions(+), 122 deletions(-)

-- 
2.13.5

Re: [Qemu-devel] [PATCH v9 0/8] Optimize VMDK I/O by allocating multiple clusters

Posted by Fam Zheng 6 years, 6 months ago

On Mon, 10/09 18:29, Ashijeet Acharya wrote:
> Optimization test results:
> 
> This patch series improves 128 KB sequential write performance to an
> empty VMDK file by 54%
> 
> Benchmark command: ./qemu-img bench -w -c 1024 -s 128K -d 1 -t none -f
> vmdk test.vmdk
> 
> Changes in v9:
> - rebase the series

Thanks, looks good to me, applied:

https://github.com/famz/qemu/tree/staging

Fam

Re: [Qemu-devel] [PATCH v9 0/8] Optimize VMDK I/O by allocating multiple clusters

Posted by Fam Zheng 6 years, 5 months ago

On Mon, 10/09 22:12, Fam Zheng wrote:
> On Mon, 10/09 18:29, Ashijeet Acharya wrote:
> > Optimization test results:
> > 
> > This patch series improves 128 KB sequential write performance to an
> > empty VMDK file by 54%
> > 
> > Benchmark command: ./qemu-img bench -w -c 1024 -s 128K -d 1 -t none -f
> > vmdk test.vmdk
> > 
> > Changes in v9:
> > - rebase the series
> 
> Thanks, looks good to me, applied:
> 
> https://github.com/famz/qemu/tree/staging

Ashijeet: I've been testing my branch and it seems installing Fedora/CentOS to a
VMDK image is broken with your patches applied. Both guest and QEMU are
responsive, but the installing of packages stops to make any progress at some
point:

Installing rootfiles.noarch (317/318)
Installing langpacks-en.noarch (318/318)
Performing post-installation setup tasks
Configuring fedora-release.noarch
Configuring filesystem.x86_64
Configuring GeoIP-GeoLite-data.noarch
Configuring python3.x86_64
Configuring fedora-logos.x86_64
Configuring kernel-core.x86_64

# hang here

Can you reproduce this on your machine?

My command line is something like this:

qemu-system-x86_64 -enable-kvm -cpu host -m 1G -qmp unix:/home/fam/.q/qemu-8DOC9EF4/qmp,server,nowait -name 8DOC9EF4 -netdev user,id=vnet,hostfwd=:0.0.0.0:10022-:22 -device virtio-net-pci,netdev=vnet -drive file=/var/tmp/test2.vmdk,if=none,id=drive-1,cache=none,aio=native -device virtio-blk-pci,drive=drive-1 -cdrom /stor/iso/CentOS-6.9-x86_64-minimal.iso -pidfile /home/fam/.q/qemu-8DOC9EF4/pid

qemu.git master doesn't have this problem. So I'll drop this series from the
pull request until it is resolved.

Fam

Re: [Qemu-devel] [PATCH v9 0/8] Optimize VMDK I/O by allocating multiple clusters

Posted by Ashijeet Acharya 6 years, 5 months ago

On Fri, Oct 20, 2017 at 11:58 Fam Zheng <famz@redhat.com> wrote:

> On Mon, 10/09 22:12, Fam Zheng wrote:
> > On Mon, 10/09 18:29, Ashijeet Acharya wrote:
> > > Optimization test results:
> > >
> > > This patch series improves 128 KB sequential write performance to an
> > > empty VMDK file by 54%
> > >
> > > Benchmark command: ./qemu-img bench -w -c 1024 -s 128K -d 1 -t none -f
> > > vmdk test.vmdk
> > >
> > > Changes in v9:
> > > - rebase the series
> >
> > Thanks, looks good to me, applied:
> >
> > https://github.com/famz/qemu/tree/staging
>
> Ashijeet: I've been testing my branch and it seems installing
> Fedora/CentOS to a
> VMDK image is broken with your patches applied. Both guest and QEMU are
> responsive, but the installing of packages stops to make any progress at
> some
> point:
>
> Installing rootfiles.noarch (317/318)
> Installing langpacks-en.noarch (318/318)
> Performing post-installation setup tasks
> Configuring fedora-release.noarch
> Configuring filesystem.x86_64
> Configuring GeoIP-GeoLite-data.noarch
> Configuring python3.x86_64
> Configuring fedora-logos.x86_64
> Configuring kernel-core.x86_64
>
> # hang here
>
> Can you reproduce this on your machine?
>
> My command line is something like this:
>
> qemu-system-x86_64 -enable-kvm -cpu host -m 1G -qmp
> unix:/home/fam/.q/qemu-8DOC9EF4/qmp,server,nowait -name 8DOC9EF4 -netdev
> user,id=vnet,hostfwd=:0.0.0.0:10022-:22 -device
> virtio-net-pci,netdev=vnet -drive
> file=/var/tmp/test2.vmdk,if=none,id=drive-1,cache=none,aio=native -device
> virtio-blk-pci,drive=drive-1 -cdrom /stor/iso/CentOS-6.9-x86_64-minimal.iso
> -pidfile /home/fam/.q/qemu-8DOC9EF4/pid
>
> qemu.git master doesn't have this problem. So I'll drop this series from
> the
> pull request until it is resolved.


Fam: Alright, I will look into this but I cannot give you a deadline
unfortunately. I will try my best to resolve this as soon as though.

Ashijeet

>
>
> Fam
>

Re: [Qemu-devel] [PATCH v9 0/8] Optimize VMDK I/O by allocating multiple clusters

Posted by John Snow 6 years, 5 months ago


On 10/20/2017 02:53 AM, Ashijeet Acharya wrote:
> 
> On Fri, Oct 20, 2017 at 11:58 Fam Zheng <famz@redhat.com
> <mailto:famz@redhat.com>> wrote:
> 
>     On Mon, 10/09 22:12, Fam Zheng wrote:
>     > On Mon, 10/09 18:29, Ashijeet Acharya wrote:
>     > > Optimization test results:
>     > >
>     > > This patch series improves 128 KB sequential write performance to an
>     > > empty VMDK file by 54%
>     > >
>     > > Benchmark command: ./qemu-img bench -w -c 1024 -s 128K -d 1 -t
>     none -f
>     > > vmdk test.vmdk
>     > >
>     > > Changes in v9:
>     > > - rebase the series
>     >
>     > Thanks, looks good to me, applied:
>     >
>     > https://github.com/famz/qemu/tree/staging
> 
>     Ashijeet: I've been testing my branch and it seems installing
>     Fedora/CentOS to a
>     VMDK image is broken with your patches applied. Both guest and QEMU are
>     responsive, but the installing of packages stops to make any
>     progress at some
>     point:
> 
>     Installing rootfiles.noarch (317/318)
>     Installing langpacks-en.noarch (318/318)
>     Performing post-installation setup tasks
>     Configuring fedora-release.noarch
>     Configuring filesystem.x86_64
>     Configuring GeoIP-GeoLite-data.noarch
>     Configuring python3.x86_64
>     Configuring fedora-logos.x86_64
>     Configuring kernel-core.x86_64
> 
>     # hang here
> 
>     Can you reproduce this on your machine?
> 
>     My command line is something like this:
> 
>     qemu-system-x86_64 -enable-kvm -cpu host -m 1G -qmp
>     unix:/home/fam/.q/qemu-8DOC9EF4/qmp,server,nowait -name 8DOC9EF4
>     -netdev user,id=vnet,hostfwd=:0.0.0.0 <http://0.0.0.0>:10022-:22
>     -device virtio-net-pci,netdev=vnet -drive
>     file=/var/tmp/test2.vmdk,if=none,id=drive-1,cache=none,aio=native
>     -device virtio-blk-pci,drive=drive-1 -cdrom
>     /stor/iso/CentOS-6.9-x86_64-minimal.iso -pidfile
>     /home/fam/.q/qemu-8DOC9EF4/pid
> 
>     qemu.git master doesn't have this problem. So I'll drop this series
>     from the
>     pull request until it is resolved.
> 
> 
> Fam: Alright, I will look into this but I cannot give you a deadline
> unfortunately. I will try my best to resolve this as soon as though.
> 
> Ashijeet
> 
> 
> 
>     Fam
> 

Do we need to temporarily roll this back for the 2.11 release if it
cannot be addressed in time?

Re: [Qemu-devel] [PATCH v9 0/8] Optimize VMDK I/O by allocating multiple clusters

Posted by Fam Zheng 6 years, 5 months ago

On Thu, 11/02 17:02, John Snow wrote:
> 
> 
> On 10/20/2017 02:53 AM, Ashijeet Acharya wrote:
> > 
> > On Fri, Oct 20, 2017 at 11:58 Fam Zheng <famz@redhat.com
> > <mailto:famz@redhat.com>> wrote:
> > 
> >     On Mon, 10/09 22:12, Fam Zheng wrote:
> >     > On Mon, 10/09 18:29, Ashijeet Acharya wrote:
> >     > > Optimization test results:
> >     > >
> >     > > This patch series improves 128 KB sequential write performance to an
> >     > > empty VMDK file by 54%
> >     > >
> >     > > Benchmark command: ./qemu-img bench -w -c 1024 -s 128K -d 1 -t
> >     none -f
> >     > > vmdk test.vmdk
> >     > >
> >     > > Changes in v9:
> >     > > - rebase the series
> >     >
> >     > Thanks, looks good to me, applied:
> >     >
> >     > https://github.com/famz/qemu/tree/staging
> > 
> >     Ashijeet: I've been testing my branch and it seems installing
> >     Fedora/CentOS to a
> >     VMDK image is broken with your patches applied. Both guest and QEMU are
> >     responsive, but the installing of packages stops to make any
> >     progress at some
> >     point:
> > 
> >     Installing rootfiles.noarch (317/318)
> >     Installing langpacks-en.noarch (318/318)
> >     Performing post-installation setup tasks
> >     Configuring fedora-release.noarch
> >     Configuring filesystem.x86_64
> >     Configuring GeoIP-GeoLite-data.noarch
> >     Configuring python3.x86_64
> >     Configuring fedora-logos.x86_64
> >     Configuring kernel-core.x86_64
> > 
> >     # hang here
> > 
> >     Can you reproduce this on your machine?
> > 
> >     My command line is something like this:
> > 
> >     qemu-system-x86_64 -enable-kvm -cpu host -m 1G -qmp
> >     unix:/home/fam/.q/qemu-8DOC9EF4/qmp,server,nowait -name 8DOC9EF4
> >     -netdev user,id=vnet,hostfwd=:0.0.0.0 <http://0.0.0.0>:10022-:22
> >     -device virtio-net-pci,netdev=vnet -drive
> >     file=/var/tmp/test2.vmdk,if=none,id=drive-1,cache=none,aio=native
> >     -device virtio-blk-pci,drive=drive-1 -cdrom
> >     /stor/iso/CentOS-6.9-x86_64-minimal.iso -pidfile
> >     /home/fam/.q/qemu-8DOC9EF4/pid
> > 
> >     qemu.git master doesn't have this problem. So I'll drop this series
> >     from the
> >     pull request until it is resolved.
> > 
> > 
> > Fam: Alright, I will look into this but I cannot give you a deadline
> > unfortunately. I will try my best to resolve this as soon as though.
> > 
> > Ashijeet
> > 
> > 
> > 
> >     Fam
> > 
> 
> Do we need to temporarily roll this back for the 2.11 release if it
> cannot be addressed in time?

Hi John, it's not merged yet, I dropped it from my pull request.

Fam

Re: [Qemu-devel] [PATCH v9 0/8] Optimize VMDK I/O by allocating multiple clusters

Posted by John Snow 6 years, 5 months ago


On 11/02/2017 11:50 PM, Fam Zheng wrote:
> On Thu, 11/02 17:02, John Snow wrote:
>>
>>
>> On 10/20/2017 02:53 AM, Ashijeet Acharya wrote:
>>>
>>> On Fri, Oct 20, 2017 at 11:58 Fam Zheng <famz@redhat.com
>>> <mailto:famz@redhat.com>> wrote:
>>>
>>>     On Mon, 10/09 22:12, Fam Zheng wrote:
>>>     > On Mon, 10/09 18:29, Ashijeet Acharya wrote:
>>>     > > Optimization test results:
>>>     > >
>>>     > > This patch series improves 128 KB sequential write performance to an
>>>     > > empty VMDK file by 54%
>>>     > >
>>>     > > Benchmark command: ./qemu-img bench -w -c 1024 -s 128K -d 1 -t
>>>     none -f
>>>     > > vmdk test.vmdk
>>>     > >
>>>     > > Changes in v9:
>>>     > > - rebase the series
>>>     >
>>>     > Thanks, looks good to me, applied:
>>>     >
>>>     > https://github.com/famz/qemu/tree/staging
>>>
>>>     Ashijeet: I've been testing my branch and it seems installing
>>>     Fedora/CentOS to a
>>>     VMDK image is broken with your patches applied. Both guest and QEMU are
>>>     responsive, but the installing of packages stops to make any
>>>     progress at some
>>>     point:
>>>
>>>     Installing rootfiles.noarch (317/318)
>>>     Installing langpacks-en.noarch (318/318)
>>>     Performing post-installation setup tasks
>>>     Configuring fedora-release.noarch
>>>     Configuring filesystem.x86_64
>>>     Configuring GeoIP-GeoLite-data.noarch
>>>     Configuring python3.x86_64
>>>     Configuring fedora-logos.x86_64
>>>     Configuring kernel-core.x86_64
>>>
>>>     # hang here
>>>
>>>     Can you reproduce this on your machine?
>>>
>>>     My command line is something like this:
>>>
>>>     qemu-system-x86_64 -enable-kvm -cpu host -m 1G -qmp
>>>     unix:/home/fam/.q/qemu-8DOC9EF4/qmp,server,nowait -name 8DOC9EF4
>>>     -netdev user,id=vnet,hostfwd=:0.0.0.0 <http://0.0.0.0>:10022-:22
>>>     -device virtio-net-pci,netdev=vnet -drive
>>>     file=/var/tmp/test2.vmdk,if=none,id=drive-1,cache=none,aio=native
>>>     -device virtio-blk-pci,drive=drive-1 -cdrom
>>>     /stor/iso/CentOS-6.9-x86_64-minimal.iso -pidfile
>>>     /home/fam/.q/qemu-8DOC9EF4/pid
>>>
>>>     qemu.git master doesn't have this problem. So I'll drop this series
>>>     from the
>>>     pull request until it is resolved.
>>>
>>>
>>> Fam: Alright, I will look into this but I cannot give you a deadline
>>> unfortunately. I will try my best to resolve this as soon as though.
>>>
>>> Ashijeet
>>>
>>>
>>>
>>>     Fam
>>>
>>
>> Do we need to temporarily roll this back for the 2.11 release if it
>> cannot be addressed in time?
> 
> Hi John, it's not merged yet, I dropped it from my pull request.
> 
> Fam
> 
OK thanks, just trying to get back up to speed and making sure things
that found their way into my "pay attention to this" folder are taken
care of.

:)

--js