qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: Alberto Garcia <berto@igalia.com>,
	Anton Nefedov <anton.nefedov@virtuozzo.com>,
	Fam Zheng <famz@redhat.com>
Cc: kwolf@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org,
	mreitz@redhat.com
Subject: Re: [Qemu-devel] [Qemu-block] segfault in parallel blockjobs (iotest 30)
Date: Wed, 22 Nov 2017 10:58:31 -0500	[thread overview]
Message-ID: <0e21cf14-388e-4b51-0236-3682de9df89b@redhat.com> (raw)
In-Reply-To: <w51tvxmscay.fsf@maestria.local.igalia.com>



On 11/22/2017 07:55 AM, Alberto Garcia wrote:
> On Tue 21 Nov 2017 04:18:13 PM CET, Anton Nefedov wrote:
>>  >> keep BlockJob referenced while it is
>>  >> paused (by block_job_pause/resume_all()). That should prevent it from
>>  >> deleting the BB.
>>
>> looks kind of hacky; maybe referencing in block_job_pause() (and not
>> just pause_all) seems more correct? I think it didn't work for me
>> right away though. But I can look more.
> 
> This fixes one crash for me (but only the test_stream_commit, it doesn't
> fix iotest 030 completely), but I agree it looks kind of hacky.
> 
> Peharps replacing block_job_next() with QLIST_FOREACH_SAFE() is a good
> idea, though, I guess resuming a block job can theoretically lead to its
> destruction?
> 

Indirectly, though I think currently all implemented blockjobs defer to
the main loop first as an implementation detail of the job, and then the
.abort/.commit decision will put down the last reference that the job
keeps for itself.

(Other entities may hold a reference though, like a transaction.)

[I'd love to implement a shim that forces this, but it's not important
right now.]

Whether or not you can count on the job to be there after a resume
depends on what kind of locks or threading guarantees you can make in
the context you're asking the question.

But that sounds dangerous in case any implementation detail changes, so
maybe don't.

  reply	other threads:[~2017-11-22 15:58 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-07 16:19 [Qemu-devel] segfault in parallel blockjobs (iotest 30) Anton Nefedov
2017-11-08 13:58 ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2017-11-08 14:45 ` Alberto Garcia
2017-11-08 15:50   ` Anton Nefedov
2017-11-09  6:05     ` Fam Zheng
2017-11-09 10:03       ` Alberto Garcia
2017-11-09 16:26   ` Alberto Garcia
2017-11-10  3:02     ` Fam Zheng
2017-11-15 15:42       ` Alberto Garcia
2017-11-15 16:31         ` Anton Nefedov
2017-11-16 15:54           ` Alberto Garcia
2017-11-16 16:09             ` Anton Nefedov
2017-11-21 12:51               ` Alberto Garcia
2017-11-21 15:18                 ` Anton Nefedov
2017-11-21 15:31                   ` Alberto Garcia
2017-11-22  0:13                     ` John Snow
2017-11-22 12:55                   ` Alberto Garcia
2017-11-22 15:58                     ` John Snow [this message]
2017-11-16 21:56             ` John Snow
2017-11-17 16:16               ` Alberto Garcia
2017-11-22 15:05 ` Alberto Garcia

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=0e21cf14-388e-4b51-0236-3682de9df89b@redhat.com \
    --to=jsnow@redhat.com \
    --cc=anton.nefedov@virtuozzo.com \
    --cc=berto@igalia.com \
    --cc=famz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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).