All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Tokarev <mjt@tls.msk.ru>
To: Hanna Czenczek <hreitz@redhat.com>, qemu-block@nongnu.org
Cc: qemu-devel@nongnu.org, Kevin Wolf <kwolf@redhat.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Subject: Re: [PULL 02/17] block: Collapse padded I/O vecs exceeding IOV_MAX
Date: Tue, 6 Jun 2023 13:47:05 +0300	[thread overview]
Message-ID: <100a5e86-624f-bea1-8f53-e13072cb2eee@tls.msk.ru> (raw)
In-Reply-To: <8e681dcd-43ab-40ed-5e96-426557d87380@redhat.com>

06.06.2023 11:45, Hanna Czenczek wrote:
> On 06.06.23 10:00, Michael Tokarev wrote:
..
>> This seems to be over-complicated, both of them, no?
> 
> I would have preferred to have this discussion while the patch was still on-list for review (this specific version was for two months, counting from 
> the first version was three).  Do you think it is so complicated and thus bug-prone that we must revert this series now and try the other route?

Well. I come across this change only now when reviewing patches applied to qemu/master.
This one fixes a real bug which people were hitting, which is also quite difficult to
diagnose and which has a possibility for data corruption and other "interesting" effects,
so it is quite a natural thing to at least think about back-porting this change to
previous -stable qemu release.  Bugs like this should be fixed in -stable IMHO.

Sadly I haven't noticed this change before, sure I'd have exactly the same thoughts
by then, and would be glad to help analyzing other parts of the code with potential
of having issues with IOV_MAX-exceeding vectors.

> I can agree that perhaps the other route could have been simpler, but now we already have patches that are reviewed and in master, which solve the 
> problem.  So I won’t spend more time on tackling this issue from another angle.  If you are happy to do so, patches are always welcome.

That's a good point too.

Thanks,

/mjt



  reply	other threads:[~2023-06-06 10:47 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-05 15:45 [PULL 00/17] Block patches Hanna Czenczek
2023-06-05 15:45 ` [PULL 01/17] util/iov: Make qiov_slice() public Hanna Czenczek
2023-06-05 15:45 ` [PULL 02/17] block: Collapse padded I/O vecs exceeding IOV_MAX Hanna Czenczek
2023-06-06  8:00   ` Michael Tokarev
2023-06-06  8:45     ` Hanna Czenczek
2023-06-06 10:47       ` Michael Tokarev [this message]
2023-06-08  9:52   ` Peter Maydell
2023-06-09  7:45     ` Hanna Czenczek
2023-06-05 15:45 ` [PULL 03/17] util/iov: Remove qemu_iovec_init_extended() Hanna Czenczek
2023-06-05 15:45 ` [PULL 04/17] iotests/iov-padding: New test Hanna Czenczek
2023-06-05 15:45 ` [PULL 05/17] parallels: Out of image offset in BAT leads to image inflation Hanna Czenczek
2023-06-07  6:51   ` Michael Tokarev
2023-06-07  8:47     ` Michael Tokarev
2023-06-07 15:14     ` Hanna Czenczek
2023-06-09  8:54       ` Denis V. Lunev
2023-06-09  9:05         ` Michael Tokarev
2023-06-05 15:45 ` [PULL 06/17] parallels: Fix high_off calculation in parallels_co_check() Hanna Czenczek
2023-06-05 15:45 ` [PULL 07/17] parallels: Fix image_end_offset and data_end after out-of-image check Hanna Czenczek
2023-06-05 15:45 ` [PULL 08/17] parallels: create parallels_set_bat_entry_helper() to assign BAT value Hanna Czenczek
2023-06-05 15:45 ` [PULL 09/17] parallels: Use generic infrastructure for BAT writing in parallels_co_check() Hanna Czenczek
2023-06-05 15:45 ` [PULL 10/17] parallels: Move check of unclean image to a separate function Hanna Czenczek
2023-06-05 15:45 ` [PULL 11/17] parallels: Move check of cluster outside " Hanna Czenczek
2023-06-05 15:45 ` [PULL 12/17] parallels: Fix statistics calculation Hanna Czenczek
2023-06-05 15:45 ` [PULL 13/17] parallels: Move check of leaks to a separate function Hanna Czenczek
2023-06-05 15:45 ` [PULL 14/17] parallels: Move statistic collection " Hanna Czenczek
2023-06-05 15:45 ` [PULL 15/17] parallels: Replace qemu_co_mutex_lock by WITH_QEMU_LOCK_GUARD Hanna Czenczek
2023-06-05 15:45 ` [PULL 16/17] parallels: Incorrect condition in out-of-image check Hanna Czenczek
2023-06-05 15:45 ` [PULL 17/17] qcow2: add discard-no-unref option Hanna Czenczek
2023-06-05 19:03 ` [PULL 00/17] Block patches Richard Henderson

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=100a5e86-624f-bea1-8f53-e13072cb2eee@tls.msk.ru \
    --to=mjt@tls.msk.ru \
    --cc=hreitz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=vsementsov@yandex-team.ru \
    /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.