3:52 a.m.
Hello everyone,
After performing a migration, the syslog often contains several messages like this:
"This thread seems to be the async job owner; entering monitor without asking for a
nested job is dangerous"
The message makes it sound as if the user has done something dangerous but, after looking
at the code that produces the message, it appears to be more to do with how the libvirt
job code is used.
The commit that added the warning (6eede368bc8e3df2c94c2ec1a913885ce08e99db) doesn't
explain why it might be dangerous...
qemu: Warn on possibly incorrect usage of EnterMonitor*
qemuDomainObjEnterMonitor{,WithDriver} should not be called from async
jobs, only EnterMonitorAsync variant is allowed.
... so I would appreciate some advice from people who understand that area.
Would it be appropriate to re-word the message, or perhaps change it to a debug message so
that it's not normally seen by users?
Is it important to track down the cases that are generating the warning and fix them?
(Could it cause some kind of significant problem?)
Cheers,
Sam.
4:50 a.m.
On 07/16/2014 07:52 PM, Sam Bobroff wrote:
Hello everyone,
[Can you configure your mailer to wrap long lines?]
After performing a migration, the syslog often contains several messages like this:
"This thread seems to be the async job owner; entering monitor without asking for a
nested job is dangerous"
The sign of a bug that we need to fix. What version of libvirt are you
using? We may have already fixed it.
The message makes it sound as if the user has done something dangerous but, after looking
at the code that produces the message, it appears to be more to do with how the libvirt
job code is used.
Rather, libvirt itself has done something dangerous.
The commit that added the warning (6eede368bc8e3df2c94c2ec1a913885ce08e99db) doesn't
explain why it might be dangerous...
Jiri can probably explain better, but it means there is a race condition
where libvirt can lose track of a long-running job and cause memory
corruption in its management of the guest.
Would it be appropriate to re-word the message, or perhaps change it
to a debug message so that it's not normally seen by users?
No.
Is it important to track down the cases that are generating the warning and fix them?
(Could it cause some kind of significant problem?)
Yes.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
5:29 a.m.
On 17/07/14 12:50, Eric Blake wrote:
On 07/16/2014 07:52 PM, Sam Bobroff wrote:
> Hello everyone,
[Can you configure your mailer to wrap long lines?]
[No problem, done.]
> After performing a migration, the syslog often contains several
> messages like this:
>
> "This thread seems to be the async job owner; entering monitor
> without asking for a nested job is dangerous"
The sign of a bug that we need to fix. What version of libvirt are
you using? We may have already fixed it.
I've been testing on 1.2.6, but I will re-test on the latest code from git.
> The message makes it sound as if the user has done something
> dangerous but, after looking at the code that produces the message,
> it appears to be more to do with how the libvirt job code is used.
Rather, libvirt itself has done something dangerous.
>
> The commit that added the warning
> (6eede368bc8e3df2c94c2ec1a913885ce08e99db) doesn't explain why it
> might be dangerous...
Jiri can probably explain better, but it means there is a race
condition where libvirt can lose track of a long-running job and
cause memory corruption in its management of the guest.
> Would it be appropriate to re-word the message, or perhaps change
> it to a debug message so that it's not normally seen by users?
No.
>
> Is it important to track down the cases that are generating the
> warning and fix them? (Could it cause some kind of significant
> problem?)
Yes.
Acknowledged.
If they still occur on the latest code I'll start tracking them down :-)
Cheers,
Sam.
8:51 a.m.
On Thu, Jul 17, 2014 at 13:29:52 +1000, Sam Bobroff wrote:
On 17/07/14 12:50, Eric Blake wrote:
> On 07/16/2014 07:52 PM, Sam Bobroff wrote:
>> Hello everyone,
>
> [Can you configure your mailer to wrap long lines?]
[No problem, done.]
>> After performing a migration, the syslog often contains several
>> messages like this:
>>
>> "This thread seems to be the async job owner; entering monitor
>> without asking for a nested job is dangerous"
>
> The sign of a bug that we need to fix. What version of libvirt are
> you using? We may have already fixed it.
I've been testing on 1.2.6, but I will re-test on the latest code from git.
Hmm, it what were you doing when the message appeared in the log? I
fixed wrong usage of our job tracking APIs long time ago
(e4e28220b572cf4c71d07740c24150411f0704a6 in 1.0.3) so I guess there
must be another place we don't do it correctly.
Jirka
9:54 a.m.
On 07/17/2014 08:51 AM, Jiri Denemark wrote:
On Thu, Jul 17, 2014 at 13:29:52 +1000, Sam Bobroff wrote:
> On 17/07/14 12:50, Eric Blake wrote:
>> On 07/16/2014 07:52 PM, Sam Bobroff wrote:
>>> Hello everyone,
>>
>> [Can you configure your mailer to wrap long lines?]
>
> [No problem, done.]
>
>>> After performing a migration, the syslog often contains several
>>> messages like this:
>>>
>>> "This thread seems to be the async job owner; entering monitor
>>> without asking for a nested job is dangerous"
>>
>> The sign of a bug that we need to fix. What version of libvirt are
>> you using? We may have already fixed it.
>
> I've been testing on 1.2.6, but I will re-test on the latest code from git.
Hmm, it what were you doing when the message appeared in the log? I
fixed wrong usage of our job tracking APIs long time ago
(e4e28220b572cf4c71d07740c24150411f0704a6 in 1.0.3) so I guess there
must be another place we don't do it correctly.
That commit was essentially reverted by my commit 9846402 when fixing
migration crashes: https://bugzilla.redhat.com/show_bug.cgi?id=1018267
I think the message was harmless during migration as we don't allow other jobs
(except destroy).
Jan
8:21 a.m.
On 17/07/14 17:54, Ján Tomko wrote:
On 07/17/2014 08:51 AM, Jiri Denemark wrote:
> On Thu, Jul 17, 2014 at 13:29:52 +1000, Sam Bobroff wrote:
>> On 17/07/14 12:50, Eric Blake wrote:
>>> On 07/16/2014 07:52 PM, Sam Bobroff wrote:
>>>> Hello everyone,
>>>
>>> [Can you configure your mailer to wrap long lines?]
>>
>> [No problem, done.]
>>
>>>> After performing a migration, the syslog often contains several
>>>> messages like this:
>>>>
>>>> "This thread seems to be the async job owner; entering monitor
>>>> without asking for a nested job is dangerous"
>>>
>>> The sign of a bug that we need to fix. What version of libvirt are
>>> you using? We may have already fixed it.
>>
>> I've been testing on 1.2.6, but I will re-test on the latest code from git.
>
> Hmm, it what were you doing when the message appeared in the log? I
> fixed wrong usage of our job tracking APIs long time ago
> (e4e28220b572cf4c71d07740c24150411f0704a6 in 1.0.3) so I guess there
> must be another place we don't do it correctly.
>
That commit was essentially reverted by my commit 9846402 when fixing
migration crashes: https://bugzilla.redhat.com/show_bug.cgi?id=1018267
I think the message was harmless during migration as we don't allow other jobs
(except destroy).
Jan
I've re-tested this on the latest libvirt from git (1.2.7) and I still
see several occurrences of the "dangerous" message on the destination
machine after performing a live migration with --copy-storage-all.
It seems like qemuDomainObjEnterMonitorInternal() is expecting the
asyncJob parameter to have the value of the currently running async job,
and in the cases that cause the warning message, it's being called via
qemuDomainObjEnterMonitor() which hard codes it to QEMU_ASYNC_NONE.
It looks like these calls should be changed to
qemuDomainObjEnterMonitorAsync() with the right asyncJob parameter,
which I believe is QEMU_ASYNC_JOB_MIGRATION_IN for the cases I'm looking
at (and the old value of QEMU_ASYNC_NONE for all other callers). There
aren't many cases of this, so fixing it this way is fairly simple but it
does involve passing the asyncJob through some intermediate functions.
Another approach would be to change qemuDomainObjEnterMonitorInternal()
so that it can automatically do the same thing: It could see that there
was an async job running, and that it's owned by the current thread, and
then call qemuDomainObjBeginNestedJob() (or directly call
qemuDomainObjBeginJobInternal() since we know that the checks in
qemuDomainObjBeginNestedJob() are going to succeed). It seems like this
might also simplify some other code that is doing pretty much just what
I described above.
The second approach does fix all the cases I'm looking at, and would
also fix other similar cases I haven't yet found, but I'm not sure that
it would be entirely safe or if it's circumventing some sort of safety
checking.
Any comments?
Which approach should I pursue?
Cheers,
Sam.
3717
days inactive
3722
days old
5 comments
4 participants
participants (4)
-
Eric Blake -
Jiri Denemark -
Ján Tomko -
Sam Bobroff