Re: [PATCHv2 04/14] Fix vmdef usage after domain crash in monitor on device detach

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01/12/2015 10:58 PM, John Ferlan wrote:
> 
> 
> On 01/07/2015 10:42 AM, Ján Tomko wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1161024
>>
>> Skip audit and removing the device from live def because
>> it has already been cleaned up.
>> ---
>>  src/qemu/qemu_hotplug.c | 59 ++++++++++++++++++++++++++++++-------------------
>>  1 file changed, 36 insertions(+), 23 deletions(-)
>>
> 
> Similar Audit concerns w/ 3/14...
> 
> 
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index ce05f80..c480dcd 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
>> @@ -2986,19 +2986,22 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
>>      qemuDomainObjEnterMonitor(driver, vm);
>>      if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
>>          if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) {
>> -            qemuDomainObjExitMonitor(driver, vm);
>> +            if (qemuDomainObjExitMonitor(driver, vm) < 0)
>> +                goto cleanup;
>>              virDomainAuditDisk(vm, detach->src, NULL, "detach", false);
>>              goto cleanup;
>>          }
>>      } else {
>>          if (qemuMonitorRemovePCIDevice(priv->mon,
>>                                         &detach->info.addr.pci) < 0) {
>> -            qemuDomainObjExitMonitor(driver, vm);
>> +            if (qemuDomainObjExitMonitor(driver, vm) < 0)
>> +                goto cleanup;
>>              virDomainAuditDisk(vm, detach->src, NULL, "detach", false);
>>              goto cleanup;
>>          }
>>      }
>> -    qemuDomainObjExitMonitor(driver, vm);
>> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
>> +        goto cleanup;
>>  
>>      rc = qemuDomainWaitForDeviceRemoval(vm);
>>      if (rc == 0 || rc == 1)
>> @@ -3038,11 +3041,13 @@ qemuDomainDetachDiskDevice(virQEMUDriverPtr driver,
>>  
>>      qemuDomainObjEnterMonitor(driver, vm);
>>      if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) {
>> -        qemuDomainObjExitMonitor(driver, vm);
>> +        if (qemuDomainObjExitMonitor(driver, vm) < 0)
>> +            goto cleanup;
>>          virDomainAuditDisk(vm, detach->src, NULL, "detach", false);
>>          goto cleanup;
>>      }
>> -    qemuDomainObjExitMonitor(driver, vm);
>> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
>> +        goto cleanup;
>>  
>>      rc = qemuDomainWaitForDeviceRemoval(vm);
>>      if (rc == 0 || rc == 1)
>> @@ -3219,17 +3224,20 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
>>      qemuDomainObjEnterMonitor(driver, vm);
>>      if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
>>          if (qemuMonitorDelDevice(priv->mon, detach->info.alias)) {
>> -            qemuDomainObjExitMonitor(driver, vm);
>> +            if (qemuDomainObjExitMonitor(driver, vm) < 0)
>> +                goto cleanup;
>>              goto cleanup;
>>          }
>>      } else {
>>          if (qemuMonitorRemovePCIDevice(priv->mon,
>>                                         &detach->info.addr.pci) < 0) {
>> -            qemuDomainObjExitMonitor(driver, vm);
>> +            if (qemuDomainObjExitMonitor(driver, vm) < 0)
>> +                goto cleanup;
>>              goto cleanup;
>>          }
>>      }
>> -    qemuDomainObjExitMonitor(driver, vm);
>> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
>> +        goto cleanup;
>>  
>>      rc = qemuDomainWaitForDeviceRemoval(vm);
>>      if (rc == 0 || rc == 1)
>> @@ -3274,7 +3282,8 @@ qemuDomainDetachHostPCIDevice(virQEMUDriverPtr driver,
>>      } else {
>>          ret = qemuMonitorRemovePCIDevice(priv->mon, &detach->info->addr.pci);
>>      }
>> -    qemuDomainObjExitMonitor(driver, vm);
>> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
>> +        ret = -1;
>>  
>>      return ret;
> 
> Or just ret qemuDomainObjExitMonitor(driver, vm);

That would return 0 in the case of RemovePCIDevice failing, but not crashing
the domain.

>> @@ -3374,7 +3381,8 @@ qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver,
>>      }
>>  
>>      if (ret < 0) {
>> -        virDomainAuditHostdev(vm, detach, "detach", false);
>> +        if (virDomainObjIsActive(vm))
>> +            virDomainAuditHostdev(vm, detach, "detach", false);
> 
> Hmm.... well this makes my comments in 3/14 appear to be unnecessary,
> since there's an explicit check for active vm... Although the Vcpu
> failure will still call virDomainAuditVcpu


'detach' here is a pointer to the device in the vm->def, so it could be freed
in the meantime if the domain crashed.

Jan

Attachment: signature.asc
Description: OpenPGP digital signature

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]