From: Eric Blake <eblake@redhat.com>
To: Nir Soffer <nirsof@gmail.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
qemu-block <qemu-block@nongnu.org>,
QEMU Developers <qemu-devel@nongnu.org>,
Max Reitz <mreitz@redhat.com>, Nir Soffer <nsoffer@redhat.com>
Subject: Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole
Date: Fri, 11 Jun 2021 16:41:04 -0500 [thread overview]
Message-ID: <20210611214104.2bptve2o4zyqgnvh@redhat.com> (raw)
In-Reply-To: <CAMr-obsNTUzEf96U=Fgqwa1cOqpC8q8cTJ78cm1Yx3c+PbhvMg@mail.gmail.com>
On Sat, Jun 12, 2021 at 12:23:06AM +0300, Nir Soffer wrote:
> > Otherwise, you do have a point: "depth":1 in isolation is ambiguous
> > between "not allocated anywhere in this 1-element chain" and
> > "allocated at the first backing file in this chain of length 2 or
> > more". At which point you can indeed use "qemu-img info" to determine
> > the backing chain depth. How painful is that extra step? Does it
> > justify the addition of a new optional "backing":true to any portion
> > of the file that was beyond the end of the chain (and omit that line
> > for all other regions, rather than printing "backing":false)?
>
> Dealing with depth: N + 1 is not that painful, but also not great.
>
> I think it is worth a little more effort, and it will save time in the long term
> for users and for developers. Better APIs need simpler and shorter
> documentation and require less support.
>
> I'm not sure about backing: false, maybe absent: true to match libnbd?
In the patch [1], I did "backing":true if the cluster was not found in
the chain, and omitted the bool altogether when the cluster is
present. If we like the name "absent":true better than
"backing":true, that's an easy change. The libnbd change for nbdinfo
to report 'absent' instead of 'unallocated' has not yet been released,
so we have some leeway on naming choices.
[1] https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg03067.html
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
prev parent reply other threads:[~2021-06-11 21:42 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-07 20:22 [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole Nir Soffer
2021-06-07 21:22 ` Eric Blake
2021-06-07 22:04 ` Eric Blake
2021-06-08 16:38 ` Nir Soffer
2021-06-08 18:45 ` Eric Blake
2021-06-08 20:42 ` Nir Soffer
2021-06-10 18:34 ` Eric Blake
2021-06-10 20:09 ` Nir Soffer
2021-06-10 20:46 ` Eric Blake
2021-06-11 8:09 ` Kevin Wolf
2021-06-11 8:14 ` Vladimir Sementsov-Ogievskiy
2021-06-11 9:05 ` Nir Soffer
2021-06-11 11:14 ` Vladimir Sementsov-Ogievskiy
2021-06-11 11:21 ` Kevin Wolf
2021-06-11 13:04 ` Vladimir Sementsov-Ogievskiy
2021-06-11 13:31 ` Eric Blake
2021-06-11 13:28 ` Eric Blake
2021-06-11 17:35 ` Nir Soffer
2021-06-11 18:34 ` Eric Blake
2021-06-11 21:23 ` Nir Soffer
2021-06-11 21:41 ` Eric Blake [this message]
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=20210611214104.2bptve2o4zyqgnvh@redhat.com \
--to=eblake@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=nirsof@gmail.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 NNTP newsgroup(s).