[libvirt] [PATCH] qemu: make monitor command API available during async jobs
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@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
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
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
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
participants (2)
-
Jiri Denemark -
Nikolay Shirokovskiy