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

[wangjie (P)]

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 at 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 at redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list