11:44 a.m.
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(a)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
12:22 p.m.
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(a)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
9:19 a.m.
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(a)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
.
10:06 a.m.
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
10:55 a.m.
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
9:07 a.m.
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
1937
days inactive
1940
days old
6 comments
3 participants
participants (3)
-
Jie Wang -
Michal Privoznik -
wangjie (P)