[libvirt] [PATCH] qemu: fix pr-helper0 remain
if libvirt receive DISCONNECTED event and set prDaemonRunning to false, and qemuDomainRemoveDiskDevice is performing in the meantime. qemuDomainRemoveDiskDevice will return directly by prDaemonRunning check, so the pr-helper0 object will remain. I think it is no need to check prDaemonRunning in qemuHotplugRemoveManagedPR. Signed-off-by: Jie Wang <wangjie88@huawei.com> --- src/qemu/qemu_hotplug.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 34249bd030..5e4a929738 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -465,8 +465,7 @@ qemuHotplugRemoveManagedPR(virQEMUDriverPtr driver, virErrorPtr orig_err; int ret = -1; - if (!priv->prDaemonRunning || - virDomainDefHasManagedPR(vm->def)) + if (virDomainDefHasManagedPR(vm->def)) return 0; virErrorPreserveLast(&orig_err); -- 2.16.2.windows.1
On 5/29/19 11:44 AM, Jie Wang wrote:
if libvirt receive DISCONNECTED event and set prDaemonRunning to false, and qemuDomainRemoveDiskDevice is performing in the meantime. qemuDomainRemoveDiskDevice will return directly by prDaemonRunning check, so the pr-helper0 object will remain. I think it is no need to check prDaemonRunning in qemuHotplugRemoveManagedPR.
Signed-off-by: Jie Wang <wangjie88@huawei.com> --- src/qemu/qemu_hotplug.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 34249bd030..5e4a929738 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -465,8 +465,7 @@ qemuHotplugRemoveManagedPR(virQEMUDriverPtr driver, virErrorPtr orig_err; int ret = -1;
- if (!priv->prDaemonRunning || - virDomainDefHasManagedPR(vm->def)) + if (virDomainDefHasManagedPR(vm->def)) return 0;
virErrorPreserveLast(&orig_err);
Right, because we unlock the domain object while talking on monitor. Well, in that case I guess we should try harder and also kill the pr-helper process. IOW qemuProcessKillManagedPRDaemon() could use the same treatment. Michal
Hi, Michal: Do you mean we should also remove "if (!priv->prDaemonRunning)" in qemuProcessKillManagedPRDaemon ? I don't understand why do that, can you tell me more details? On 2019/5/30 18:22, Michal Privoznik wrote:
On 5/29/19 11:44 AM, Jie Wang wrote:
if libvirt receive DISCONNECTED event and set prDaemonRunning to false, and qemuDomainRemoveDiskDevice is performing in the meantime. qemuDomainRemoveDiskDevice will return directly by prDaemonRunning check, so the pr-helper0 object will remain. I think it is no need to check prDaemonRunning in qemuHotplugRemoveManagedPR.
Signed-off-by: Jie Wang <wangjie88@huawei.com> --- src/qemu/qemu_hotplug.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 34249bd030..5e4a929738 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -465,8 +465,7 @@ qemuHotplugRemoveManagedPR(virQEMUDriverPtr driver, virErrorPtr orig_err; int ret = -1; - if (!priv->prDaemonRunning || - virDomainDefHasManagedPR(vm->def)) + if (virDomainDefHasManagedPR(vm->def)) return 0; virErrorPreserveLast(&orig_err);
Right, because we unlock the domain object while talking on monitor. Well, in that case I guess we should try harder and also kill the pr-helper process. IOW qemuProcessKillManagedPRDaemon() could use the same treatment.
Michal
.
Hi, Michal: Sorry, I don't think out which case make us to do that, what are the risks? can you give me an example? On 2019/5/31 15:21, Michal Privoznik wrote:
On 5/31/19 9:19 AM, wangjie (P) wrote:
Hi, Michal:
Do you mean we should also remove "if (!priv->prDaemonRunning)" in qemuProcessKillManagedPRDaemon ? I don't understand why do that, can you tell me more details?
Yes. I mean exactly that.
Michal
On 5/31/19 10:06 AM, wangjie (P) wrote:
Hi, Michal:
Sorry, I don't think out which case make us to do that, what are the risks? can you give me an example?
Well, qemuProcessKillManagedPRDaemon() not only kills pr-helper process but it also cleans up after it. If we receive DISCONNECTED event then qemu is probably unable to talk to pr-helper process even if it is running. It makes sense then to kill it and start a new one. If it is not running then we might clean up after it. Michal
Thanks，Michal I understand what you mean， and I have sent the second patch “PATCH v2”，please review it. :) Jie On 2019/5/31 16:55, Michal Privoznik wrote:
On 5/31/19 10:06 AM, wangjie (P) wrote:
Hi, Michal:
Sorry, I don't think out which case make us to do that, what are the risks? can you give me an example?
Well, qemuProcessKillManagedPRDaemon() not only kills pr-helper process but it also cleans up after it. If we receive DISCONNECTED event then qemu is probably unable to talk to pr-helper process even if it is running. It makes sense then to kill it and start a new one. If it is not running then we might clean up after it.
Michal
participants (3)
-
Jie Wang -
Michal Privoznik -
wangjie (P)