All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	hreitz@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org
Subject: Re: [PATCH] nbd/client: Request larger block status by default
Date: Tue, 21 Sep 2021 13:08:43 -0500	[thread overview]
Message-ID: <20210921180843.6tfubsqvf56hkyro@redhat.com> (raw)
In-Reply-To: <5b9402dd-4c37-2fc6-7c65-a13c4c5e6383@virtuozzo.com>

On Tue, Sep 21, 2021 at 08:25:11PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 21.09.2021 19:17, Eric Blake wrote:
> > Now that commit 5a1cfd21 has clarified that a driver's block_status
> > can report larger *pnum than in the original request, we can take
> > advantage of that in the NBD driver.  Rather that limiting our request
> > to the server based on the maximum @bytes our caller mentioned, we
> > instead ask for as much status as possible (the minimum of our 4G
> > limit or the rest of the export); the server will still only give us
> > one extent in its answer (because we are using NBD_CMD_FLAG_REQ_ONE),
> > but now the block layer's caching of data areas can take advantage of
> > cases where the server gives us a large answer to avoid the need for
> > future NBD_CMD_BLOCK_STATUS calls.
> > 
> > Signed-off-by: Eric Blake <eblake@redhat.com>
> > ---

> 
> I remember we already discussed that, but can't find.
> 
> The problem is that it's not for free:
> 
> In server code in blockstatus_to_extents, we loop though the disk, trying to merge extents of the same type.
> 
> With full allocated qcow2, we'll have to load all L2 tables and handle them, to merge all block status into one big "allocated" extent.
> 

We don't have to loop that far.  The NBD protocol allows the server to
stop looping at whatever point makes sense, as long as it makes
progress.

> Maybe, we need some additional negotiation flag, to allow BLOCK_STATUS command with NBD_CMD_FLAG_REQ_ONE flag to return an extent larger than required when that information is available for free?

That's one possibility.  Another does not add anything to the NBD
protocol, but instead limits the code that tries to loop over block
status to deteremine a larger "allocated" answer to return to instead
stop looping after a finite number of extents have been merged
together.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



  reply	other threads:[~2021-09-21 18:10 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-21 16:17 [PATCH] nbd/client: Request larger block status by default Eric Blake
2021-09-21 17:25 ` Vladimir Sementsov-Ogievskiy
2021-09-21 18:08   ` Eric Blake [this message]
2021-09-21 19:12     ` Vladimir Sementsov-Ogievskiy
2021-09-21 20:00       ` Eric Blake
2021-09-22  7:28         ` Vladimir Sementsov-Ogievskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210921180843.6tfubsqvf56hkyro@redhat.com \
    --to=eblake@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.