1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] qemu: distinguish pr disk before qemuHotplugRemoveManagedPR

Jie Wang posted 1 patch 4 years, 10 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1560864506-79243-1-git-send-email-wangjie88@huawei.com
src/qemu/qemu_hotplug.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
[libvirt] [PATCH] qemu: distinguish pr disk before qemuHotplugRemoveManagedPR
Posted by Jie Wang 4 years, 10 months ago 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: distinguish pr disk before qemuHotplugRemoveManagedPR
Posted by Peter Krempa 4 years, 10 months ago 
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: distinguish pr disk before qemuHotplugRemoveManagedPR
Posted by wangjie (P) 4 years, 10 months ago 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: distinguish pr disk before qemuHotplugRemoveManagedPR
Posted by wangjie (P) 4 years, 10 months ago 
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;

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.