Re: [libvirt] [PATCH] qemu: distinguish pr disk before qemuHotplugRemoveManagedPR
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(a)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(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list