From: Kevin Wolf <kwolf@redhat.com>
To: John Snow <jsnow@redhat.com>
Cc: Eric Blake <eblake@redhat.com>,
famz@redhat.com, qemu-block@nongnu.org,
Jeff Cody <jcody@redhat.com>,
qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>,
Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v5 01/23] block: Allow NULL file for bdrv_get_block_status()
Date: Wed, 11 Oct 2017 10:42:14 +0200 [thread overview]
Message-ID: <20171011084214.GB4593@dhcp-200-186.str.redhat.com> (raw)
In-Reply-To: <06af49fb-6e94-d8c0-3701-7579e55c92a5@redhat.com>
Am 10.10.2017 um 21:24 hat John Snow geschrieben:
>
>
> On 10/10/2017 03:00 PM, Eric Blake wrote:
> > On 10/10/2017 09:43 AM, Eric Blake wrote:
> >
> >>>> ---
> >>>> v5: use second label for cleaner exit logic [John], use local_pnum
> >>
> >>>> @@ -1811,16 +1811,19 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
> >>>> int64_t total_sectors;
> >>>> int64_t n;
> >>>> int64_t ret, ret2;
> >>>> + BlockDriverState *local_file = NULL;
> >>>> + int local_pnum = 0;
> >>>
> >>> I don't quite see what the point of local_pnum is if we assert anyway
> >>> that the real pnum is non-NULL.
> >>
> >> I did it in parallel with fallout from John's review on v4:
> >> https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg06958.html
> >>
> >> but since it wasn't specifically asked for, and is now getting
> >> questions, I'm fine with not having it in v6.
> >
> > Okay, I re-read v4, and here's the comment (on 21/23) that led to my
> > experiment in v5 patch 1 with local_pnum:
> >
> > https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg00293.html
> >
> > and I did argue:
> >
> > https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg00311.html
>
> Well, Kevin's the boss :D
I'm not sure how renaming *pnum into local_pnum addresses your concerns?
We still update local_pnum before we undo our alignment corrections. Or
are you talking about some other part of these mails?
Kevin
next prev parent reply other threads:[~2017-10-11 8:42 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-04 2:00 [Qemu-devel] [PATCH v5 00/23] make bdrv_get_block_status byte-based Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 01/23] block: Allow NULL file for bdrv_get_block_status() Eric Blake
2017-10-10 13:59 ` Kevin Wolf
2017-10-10 14:43 ` Eric Blake
2017-10-10 19:00 ` Eric Blake
2017-10-10 19:24 ` John Snow
2017-10-11 8:42 ` Kevin Wolf [this message]
2017-10-11 17:42 ` John Snow
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 02/23] block: Add flag to avoid wasted work in bdrv_is_allocated() Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 03/23] block: Make bdrv_round_to_clusters() signature more useful Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 04/23] qcow2: Switch is_zero_sectors() to byte-based Eric Blake
2017-10-10 14:15 ` Kevin Wolf
2017-10-10 14:47 ` Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 05/23] block: Switch bdrv_make_zero() " Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 06/23] qemu-img: Switch get_block_status() " Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 07/23] block: Convert bdrv_get_block_status() to bytes Eric Blake
2017-10-10 14:46 ` Kevin Wolf
2017-10-10 15:38 ` Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 08/23] block: Switch bdrv_co_get_block_status() to byte-based Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 09/23] block: Switch BdrvCoGetBlockStatusData " Eric Blake
2017-10-09 20:07 ` [Qemu-devel] [Qemu-block] " Jeff Cody
2017-10-09 21:30 ` Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 10/23] block: Switch bdrv_common_block_status_above() " Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 11/23] block: Switch bdrv_co_get_block_status_above() " Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 12/23] block: Convert bdrv_get_block_status_above() to bytes Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 13/23] qemu-img: Simplify logic in img_compare() Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 14/23] qemu-img: Speed up compare on pre-allocated larger file Eric Blake
2017-10-11 18:33 ` Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 15/23] qemu-img: Add find_nonzero() Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 16/23] qemu-img: Drop redundant error message in compare Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 17/23] qemu-img: Change check_empty_sectors() to byte-based Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 18/23] qemu-img: Change compare_sectors() to be byte-based Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 19/23] qemu-img: Change img_rebase() " Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 20/23] qemu-img: Change img_compare() " Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 21/23] block: Align block status requests Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 22/23] block: Relax bdrv_aligned_preadv() assertion Eric Blake
2017-10-04 2:00 ` [Qemu-devel] [PATCH v5 23/23] qemu-io: Relax 'alloc' now that block-status doesn't assert Eric Blake
2017-10-10 12:58 ` [Qemu-devel] [PATCH v5 00/23] make bdrv_get_block_status byte-based Kevin Wolf
2017-10-10 14:48 ` Eric Blake
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=20171011084214.GB4593@dhcp-200-186.str.redhat.com \
--to=kwolf@redhat.com \
--cc=eblake@redhat.com \
--cc=famz@redhat.com \
--cc=jcody@redhat.com \
--cc=jsnow@redhat.com \
--cc=mreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.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).