8:17 a.m.
One can not issue monitor commands manually during async calls thru
designated API while this could be useful for testing/debugging purposes.
qemuDomainQemuMonitorCommand uses job of type QEMU_JOB_MODIFY and any async
call disable parallel execution of this type of job. The only state that is
changed is taint variable. AFAIU the only place we can mess is resetting
taint flag in qemuProcessStop routine under some async job. But this can not
happen thanx to both virDomainObjIsActive check in qemuDomainQemuMonitorCommand
and resetting active status in qemuProcessStop before taint flag.
Change job type to QEMU_JOB_QUERY and thus make the API call available for
most of async jobs.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/qemu/qemu_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 931efae..d6aa640 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15514,7 +15514,7 @@ static int qemuDomainQemuMonitorCommand(virDomainPtr domain, const
char *cmd,
if (virDomainQemuMonitorCommandEnsureACL(domain->conn, vm->def) < 0)
goto cleanup;
- if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
goto cleanup;
if (!virDomainObjIsActive(vm)) {
--
1.8.3.1
9:18 a.m.
On Wed, Jun 22, 2016 at 16:17:50 +0300, Nikolay Shirokovskiy wrote:
One can not issue monitor commands manually during async calls
thru
designated API while this could be useful for testing/debugging purposes.
qemuDomainQemuMonitorCommand uses job of type QEMU_JOB_MODIFY and any async
call disable parallel execution of this type of job. The only state that is
changed is taint variable. AFAIU the only place we can mess is resetting
taint flag in qemuProcessStop routine under some async job. But this can not
happen thanx to both virDomainObjIsActive check in qemuDomainQemuMonitorCommand
and resetting active status in qemuProcessStop before taint flag.
Change job type to QEMU_JOB_QUERY and thus make the API call available for
most of async jobs.
The reason for acquiring MODIFY job is that qemuDomainQemuMonitorCommand
can be used to call any monitor command even those that modify state.
For example, you could unplug a device during an async job. And since we
don't want anything to mess up with a domain while async job is running,
acquiring just QUERY job would be wrong.
So NACK from me.
If there is a useful querying command that you need to call, we should
just add a proper API for it.
If it's all just about testing/debugging and you perhaps even want to
modify the domain at that time, you can just create a monitor proxy
which you can stick between libvirt and qemu and then you can inject any
monitor command you wish. Or you could even inject artificial monitor
events to libvirt. A long time ago I wrote such proxy, but since the
reviewers requested some changes (which I didn't have time to do and I
even thought they were not necessary) it was never pushed to libvirt's
git. I guess I could just update a bit and repost. In the meantime, feel
free to look at it or even play with it.
The original post:
https://www.redhat.com/archives/libvir-list/2011-July/msg00028.html
The git tree with monitor-proxy last time I rebased it:
https://gitlab.com/jirkade/libvirt/commits/monitor-proxy
Jirka
9:41 a.m.
On 22.06.2016 17:18, Jiri Denemark wrote:
On Wed, Jun 22, 2016 at 16:17:50 +0300, Nikolay Shirokovskiy wrote:
> One can not issue monitor commands manually during async calls thru
> designated API while this could be useful for testing/debugging purposes.
> qemuDomainQemuMonitorCommand uses job of type QEMU_JOB_MODIFY and any async
> call disable parallel execution of this type of job. The only state that is
> changed is taint variable. AFAIU the only place we can mess is resetting
> taint flag in qemuProcessStop routine under some async job. But this can not
> happen thanx to both virDomainObjIsActive check in qemuDomainQemuMonitorCommand
> and resetting active status in qemuProcessStop before taint flag.
>
> Change job type to QEMU_JOB_QUERY and thus make the API call available for
> most of async jobs.
The reason for acquiring MODIFY job is that qemuDomainQemuMonitorCommand
can be used to call any monitor command even those that modify state.
For example, you could unplug a device during an async job. And since we
don't want anything to mess up with a domain while async job is running,
acquiring just QUERY job would be wrong.
Well this API call is tainted thus user is aware he is on his own here.
AFAIU tainted means domain *can* be messed up thru this call.
So NACK from me.
If there is a useful querying command that you need to call, we should
just add a proper API for it.
I guess qemu could have a lot of debugging stats that regular user is not
interested of. Anyway we will be always a step back from qemu in this
aspect and monitor command is intended to shrink this gap AFAIU.
If it's all just about testing/debugging and you perhaps even want to
modify the domain at that time, you can just create a monitor proxy
which you can stick between libvirt and qemu and then you can inject any
monitor command you wish. Or you could even inject artificial monitor
events to libvirt. A long time ago I wrote such proxy, but since the
reviewers requested some changes (which I didn't have time to do and I
even thought they were not necessary) it was never pushed to libvirt's
git. I guess I could just update a bit and repost. In the meantime, feel
free to look at it or even play with it.
It looks too intrusive for debugging. We lose libvirtd state if we
not prepare proxy beforehand and we usually not )) also in auto tests
we want to be as close to production as possible. Thanx for suggestion
though.
Nikolay
10:07 a.m.
On Wed, Jun 22, 2016 at 17:41:12 +0300, Nikolay Shirokovskiy wrote:
On 22.06.2016 17:18, Jiri Denemark wrote:
> On Wed, Jun 22, 2016 at 16:17:50 +0300, Nikolay Shirokovskiy wrote:
>> One can not issue monitor commands manually during async calls thru
>> designated API while this could be useful for testing/debugging purposes.
>> qemuDomainQemuMonitorCommand uses job of type QEMU_JOB_MODIFY and any async
>> call disable parallel execution of this type of job. The only state that is
>> changed is taint variable. AFAIU the only place we can mess is resetting
>> taint flag in qemuProcessStop routine under some async job. But this can not
>> happen thanx to both virDomainObjIsActive check in qemuDomainQemuMonitorCommand
>> and resetting active status in qemuProcessStop before taint flag.
>>
>> Change job type to QEMU_JOB_QUERY and thus make the API call available for
>> most of async jobs.
>
> The reason for acquiring MODIFY job is that qemuDomainQemuMonitorCommand
> can be used to call any monitor command even those that modify state.
> For example, you could unplug a device during an async job. And since we
> don't want anything to mess up with a domain while async job is running,
> acquiring just QUERY job would be wrong.
Well this API call is tainted thus user is aware he is on his own here.
AFAIU tainted means domain *can* be messed up thru this call.
Hmm, thinking about it more I think you are right. Users can already do
a lot of harm when an async job is not running so preventing them from
doing that in a specific case of async jobs is not necessary.
So self-NACK of my previous NACK and ACK to your patch :-)
Jirka
3075
days inactive
3075
days old
3 comments
2 participants
participants (2)
-
Jiri Denemark -
Nikolay Shirokovskiy