QEMU-Devel Archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Cc: Kevin Wolf <kwolf@redhat.com>, qemu-block <qemu-block@nongnu.org>,
	Markus Armbruster <armbru@redhat.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Max Reitz <mreitz@redhat.com>, Nir Soffer <nsoffer@redhat.com>,
	libguestfs@redhat.com
Subject: Re: [PATCH 2/2] nbd: Add new qemu:joint-allocation metadata context
Date: Thu, 10 Jun 2021 08:47:49 -0500	[thread overview]
Message-ID: <20210610134749.orq6cgyrtrjylduc@redhat.com> (raw)
In-Reply-To: <5563d5eb-c90b-6f09-e550-3b39cd76b641@virtuozzo.com>

On Thu, Jun 10, 2021 at 03:30:17PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > > The correct fix is for ovirt to additionally use the
> > > qemu:allocation-depth metadata context added in 5.2: after all, the
> > > actual determination for what is needed to recreate a qcow2 file is
> > > not whether a cluster is sparse, but whether the allocation-depth
> > > shows the cluster to be local.  But reproducing an image is more
> > > efficient when handling known-zero clusters, which means that ovirt
> > > has to track both base:allocation and qemu:allocation-depth metadata
> > > contexts simultaneously.  While NBD_CMD_BLOCK_STATUS is just fine
> > > sending back information for two contexts in parallel, it comes with
> > > some bookkeeping overhead at the client side: the two contexts need
> > > not report the same length of replies, and it involves more network
> > > traffic.
> 
> Aren't both context described in one reply? Or what do you mean by not the same length?

The example file demonstrates this.  We have:

base.raw    ABC-
top.qcow2   -D0-
guest sees  AD00

Querying base:allocation returns:
         0       65536    3  hole,zero
     65536       65536    0  allocated
    131072      131072    3  hole,zero

Querying qemu:allocation-depth returns:
         0       65536    0  unallocated
     65536      131072    1  local
    196608       65536    0  unallocated

That is, the query starting at 64k returns different lengths (64k for
base:allocation, 128k for qemu:allocation-depth), and the client has
to process the smaller of the two regions before moving on to the next
query.  But if the client then does a query starting at 128k, it
either has to remember that it previously has information available
from the earlier qemu:allocation-depth, or repeats efforts over the
wire.

The joy of having a single metadata context return both pieces of
information at once is that the client no longer has to do this
cross-correlation between the differences in extent lengths of the
parallel contexts.

> > We discussed in the past the option to expose also the dirty status of every
> > block in the response. Again this info is available using
> > "qemu:dirty-bitmap:xxx"
> > but just like allocation depth and base allocation, merging the results is hard
> > and if we could expose also the dirty bit, this can make clients life
> > even better.
> > In this case I'm not sure "qemu:allocation" is the best name, maybe something
> > more generic like "qemu:extents" or "qemu:block-status" is even better.
> > 
> 
> Oops. Could you please describe, what's the problem with parsing several context simultaneously?

There is no inherent technical problem, just extra work.  Joining the
work at the server side is less coding effort than recoding the
boilerplate to join the work at every single client side.  And the
information is already present.  So we could just scrap this entire
RFC by stating that the information is already available, and it is
not worth qemu's effort to provide the convenience context.

Joining base:allocation and qemu:allocation-depth was easy - in fact,
since both use bdrv_block_status under the hood, we could (and
probably should!) merge it into a single qemu query.  But joining
base:allocation and qemu:dirty-bitmap:FOO will be harder, at which
point I question whether it is worth the complications.  And if you
argue that a joint context is not worthwhile without dirty bitmap(s)
being part of that joint context, then maybe this RFC is too complex
to worry about, and we should just leave the cross-correlation of
parallel contexts to be client-side, after all.


> 
> This all sound to me as we are going to implement "joint" combined conexts for every useful combination of existing contexts that user wants. So, it's a kind of workaround of inconvenient protocol we have invented in the past.
> 
> Doesn't it mean that we instead should rework, how we export several contexts? Maybe we can improve generic export of several contexts simultaneously, so that it will be convenient for the client? Than we don't need any additional combined contexts.

The NBD protocol intentionally left wiggle room for servers to report
different extent lengths across different contexts.  But other than
qemu, I don't know of any other NBD servers advertising alternate
contexts.  If we think we can reasonbly restrict the NBD protocol to
require that any server sending parallel contexts to a client MUST use
the same extent lengths for all parallel contexts (clients still have
to read multiple contexts, but the cross-correlation becomes easier
because the client doesn't have to worry about length mismatches), and
code that up in qemu, that's also something we can consider.

Or maybe even have it be an opt-in, where a client requests
NBD_OPT_ALIGN_META_CONTEXT; if the server acknowledges that option,
the client knows that it can request parallel NBD_OPT_SET_META_CONTEXT
and the extents replied to each NBD_OPT_BLOCK_STATUS will be aligned;
if the server does not acknowledge the option, then the client has the
choice of requesting at most one meta context, or else dealing with
unmatched extent lengths itself.

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



  reply	other threads:[~2021-06-10 13:48 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-09 18:01 [RFC PATCH 0/2] New NBD metacontext Eric Blake
2021-06-09 18:01 ` [PATCH 1/2] iotests: Improve and rename test 309 to nbd-qemu-allocation Eric Blake
2021-06-10 12:14   ` Vladimir Sementsov-Ogievskiy
2021-06-09 18:01 ` [PATCH 2/2] nbd: Add new qemu:joint-allocation metadata context Eric Blake
2021-06-09 23:52   ` Nir Soffer
2021-06-10 12:30     ` Vladimir Sementsov-Ogievskiy
2021-06-10 13:47       ` Eric Blake [this message]
2021-06-10 14:10         ` Vladimir Sementsov-Ogievskiy
2021-06-10 13:16     ` Nir Soffer
2021-06-10 14:04       ` Eric Blake
2021-06-10 14:43         ` Vladimir Sementsov-Ogievskiy
2021-06-10 13:31     ` Eric Blake
2021-06-11 23:39   ` Nir Soffer
2021-06-14 13:56     ` Eric Blake
2021-06-14 14:06       ` Nir Soffer
2021-06-09 21:31 ` [RFC libnbd PATCH] info: Add support for new qemu:joint-allocation Eric Blake
2021-06-09 22:20   ` Nir Soffer
2021-06-10 13:06     ` Eric Blake
2021-06-10 13:19       ` Nir Soffer

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=20210610134749.orq6cgyrtrjylduc@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=libguestfs@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=nsoffer@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).