[Qemu-devel] Very slow finding extents in QCOW2-backed nbd

[Qemu-devel] Very slow finding extents in QCOW2-backed nbd

[Tim Smith]

Hi all, I have a question about the intent of the last call to 
bdrv_co_block_status() in bdrv_co_block_status(), in block/io.c about line 
2195, which looks like this:

        ret2 = bdrv_co_block_status(local_file, want_zero, local_map,
                                    *pnum, &file_pnum, NULL, NULL);
        if (ret2 >= 0) {
            /* Ignore errors.  This is just providing extra information, it
             * is useful but not necessary.
             */

I have a large (>2 TB virtual, ~600GB real) QCOW2 file which is being exported 
as a network block device, and a connected client has asked for the allocation 
information via NBD_CMD_BLOCK_STATUS. The virtual disk is quite fragmented 
(deliberatedly; this is a test case), and thus has quite a lot of extents.

Each extent is taking ~0.5s to identify, and pretty much all of that time is 
taken up in lseek(SEEK_END) from find_allocation() in block/file-posix.c which 
is getting called as a result of the bdrv_co_block_status() mentioned above, 
with the result that getting the data out through the nbd is taking an order 
of magnitude longer than it should.

I ran some tests with the if-block containing the call removed, and the only 
discernable difference was that everything went a lot faster. So I'm wondering 
what the intent is for that code, and in what circumstances it is useful?

-- 
Tim Smith <tim.smith@citrix.com>

Re: [Qemu-devel] Very slow finding extents in QCOW2-backed nbd

[Vladimir Sementsov-Ogievskiy]

28.01.2019 14:58, Tim Smith wrote:
> Hi all, I have a question about the intent of the last call to
> bdrv_co_block_status() in bdrv_co_block_status(), in block/io.c about line
> 2195, which looks like this:
> 
>          ret2 = bdrv_co_block_status(local_file, want_zero, local_map,
>                                      *pnum, &file_pnum, NULL, NULL);
>          if (ret2 >= 0) {
>              /* Ignore errors.  This is just providing extra information, it
>               * is useful but not necessary.
>               */

Hi Tim!

The question is highly discussed now on list, in short: yes it's a problem, and,
I hope, we'll soon reach a consensus about how to finally fix it.

This second block_status request is needed when we have qcow2 image with
metadata preallocation, which means that qcow2 data clusters are actually backed
by holes on filesystem level.

You can follow the discussion under the following threads:

initial discussion:
https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg05749.html

solutions:

cache lseek results, by Kevin:
https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg06271.html

detect preallocation, and don't do second block_status for not preallocated,
my last try:
https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg06598.html


> 
> I have a large (>2 TB virtual, ~600GB real) QCOW2 file which is being exported
> as a network block device, and a connected client has asked for the allocation
> information via NBD_CMD_BLOCK_STATUS. The virtual disk is quite fragmented
> (deliberatedly; this is a test case), and thus has quite a lot of extents.
> 
> Each extent is taking ~0.5s to identify, and pretty much all of that time is
> taken up in lseek(SEEK_END) from find_allocation() in block/file-posix.c which
> is getting called as a result of the bdrv_co_block_status() mentioned above,
> with the result that getting the data out through the nbd is taking an order
> of magnitude longer than it should.
> 
> I ran some tests with the if-block containing the call removed, and the only
> discernable difference was that everything went a lot faster. So I'm wondering
> what the intent is for that code, and in what circumstances it is useful?
> 


-- 
Best regards,
Vladimir

Re: [Qemu-devel] Very slow finding extents in QCOW2-backed nbd

[Tim Smith]

On Monday, 28 January 2019 14:41:35 GMT Vladimir Sementsov-Ogievskiy wrote:
> 28.01.2019 14:58, Tim Smith wrote:
> 
> > Hi all, I have a question about the intent of the last call to
> > bdrv_co_block_status() in bdrv_co_block_status(), in block/io.c about
> > line
> > 2195, which looks like this:
> > 
> > 
> >          ret2 = bdrv_co_block_status(local_file, want_zero, local_map,
> >          
> >                                      *pnum, &file_pnum, NULL, NULL);
> >          
> >          if (ret2 >= 0) {
> >          
> >              /* Ignore errors.  This is just providing extra information,
> >              it
> >              
> >               * is useful but not necessary.
> >               */
> 
> 
> Hi Tim!
> 
> The question is highly discussed now on list, in short: yes it's a problem,
> and, I hope, we'll soon reach a consensus about how to finally fix it. 
> This second block_status request is needed when we have qcow2 image with
> metadata preallocation, which means that qcow2 data clusters are actually
> backed by holes on filesystem level.
> 
> You can follow the discussion under the following threads:
> 
> initial discussion:
> https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg05749.html
> 
> solutions:
> 
> cache lseek results, by Kevin:
> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg06271.html
> 
> detect preallocation, and don't do second block_status for not
> preallocated, my last try:
> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg06598.html

Thanks, I'll keep an eye on these. My use-case does not require preallocation 
so I'll carry a patch for a bit and see how it works out.

-- 
Tim Smith <tim.smith@citrix.com>