[libvirt] [PATCH] qemu: distinguish pr disk before qemuHotplugRemoveManagedPR
when a disk without PR perform attach or detach operation, need not call qemuHotplugRemoveManagedPR, otherwise, it will print err log about PR, let us fix it. Signed-off-by: Jie Wang <wangjie88@huawei.com> --- src/qemu/qemu_hotplug.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index efda539..7ef92d0 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -928,7 +928,9 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -2; - if (qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) + + if (virStorageSourceChainHasManagedPR(disk->src) && + qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) ret = -2; virDomainAuditDisk(vm, NULL, disk->src, "attach", false); @@ -4481,7 +4483,8 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, dev.data.disk = disk; ignore_value(qemuRemoveSharedDevice(driver, &dev, vm->def->name)); - if (qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) + if (virStorageSourceChainHasManagedPR(disk->src) && + qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) goto cleanup; ret = 0; -- 1.8.3.1
On Tue, Jun 18, 2019 at 21:28:26 +0800, Jie Wang wrote:
when a disk without PR perform attach or detach operation, need not call qemuHotplugRemoveManagedPR, otherwise, it will print err log about PR, let us fix it.
Could you please elaborate which error log?
Signed-off-by: Jie Wang <wangjie88@huawei.com> --- src/qemu/qemu_hotplug.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index efda539..7ef92d0 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -928,7 +928,9 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -2; - if (qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)a + + if (virStorageSourceChainHasManagedPR(disk->src) && + qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
So is this a regression from your last patch? This function was supposed to remove the PR daemon only when it was added. At any rate, a better fix will be to remember if this function added the managed PR daemon and remove only in such case.
ret = -2;
virDomainAuditDisk(vm, NULL, disk->src, "attach", false); @@ -4481,7 +4483,8 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, dev.data.disk = disk; ignore_value(qemuRemoveSharedDevice(driver, &dev, vm->def->name));
- if (qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
This function already checks that the VM is using the PR daemon and thus does nothing in such case. So I don't see a point in this change.
+ if (virStorageSourceChainHasManagedPR(disk->src) && + qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) goto cleanup;
ret = 0; -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 2019/6/19 14:16, Peter Krempa wrote:
On Tue, Jun 18, 2019 at 21:28:26 +0800, Jie Wang wrote:
when a disk without PR perform attach or detach operation, need not call qemuHotplugRemoveManagedPR, otherwise, it will print err log about PR, let us fix it. Could you please elaborate which error log?
a disk without PR perform detach also call qemuMonitorDelObject to delete ‘pr-helper0’ and print err log as follows： “internal error： unable to execute QEMU command ‘object-del’: object 'pr-helper0 ' not found”
Signed-off-by: Jie Wang <wangjie88@huawei.com> --- src/qemu/qemu_hotplug.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index efda539..7ef92d0 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -928,7 +928,9 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -2; - if (qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)a + + if (virStorageSourceChainHasManagedPR(disk->src) && + qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) So is this a regression from your last patch? This function was supposed to remove the PR daemon only when it was added.
yes, my last patch (commit 7a232286b9d8c19ad62cb93c19e4651894447743) remove priv->prDaemonRunning check， and trigger this problem.
At any rate, a better fix will be to remember if this function added the managed PR daemon and remove only in such case.
yeah，it's better to do so, but qemuHotplugRemoveManagedPR not pass the parameter(virDomainDiskDefPtr) to judge if the disk which will be detached with PR. so I had to judge before call qemuHotplugRemoveManagedPR.
ret = -2;
virDomainAuditDisk(vm, NULL, disk->src, "attach", false); @@ -4481,7 +4483,8 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, dev.data.disk = disk; ignore_value(qemuRemoveSharedDevice(driver, &dev, vm->def->name));
- if (qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
This function already checks that the VM is using the PR daemon and thus does nothing in such case. So I don't see a point in this change.
if a disk without PR perform hotplug, not add PR daemon, and goto exit_monitor later, it will also call qemuHotplugRemoveManagedPR and print err log as follows: “internal error： unable to execute QEMU command ‘object-del’: object 'pr-helper0 ' not found” So, we also need to judge before call qemuHotplugRemoveManagedPR here.
+ if (virStorageSourceChainHasManagedPR(disk->src) && + qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) goto cleanup;
ret = 0; -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Ping On 2019/6/18 21:28, Jie Wang wrote:
when a disk without PR perform attach or detach operation, need not call qemuHotplugRemoveManagedPR, otherwise, it will print err log about PR, let us fix it.
Signed-off-by: Jie Wang <wangjie88@huawei.com> --- src/qemu/qemu_hotplug.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index efda539..7ef92d0 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -928,7 +928,9 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -2; - if (qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) + + if (virStorageSourceChainHasManagedPR(disk->src) && + qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) ret = -2;
virDomainAuditDisk(vm, NULL, disk->src, "attach", false); @@ -4481,7 +4483,8 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, dev.data.disk = disk; ignore_value(qemuRemoveSharedDevice(driver, &dev, vm->def->name));
- if (qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) + if (virStorageSourceChainHasManagedPR(disk->src) && + qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) goto cleanup;
ret = 0;
participants (3)
-
Jie Wang -
Peter Krempa -
wangjie (P)