On Thu, Mar 14, 2019 at 15:31:48 +0100, Michal Privoznik wrote: > On 3/14/19 3:14 PM, Peter Krempa wrote: > > On Thu, Mar 14, 2019 at 14:56:48 +0100, Michal Privoznik wrote: > > > On 3/14/19 2:18 PM, Peter Krempa wrote: > > > > On Thu, Mar 14, 2019 at 13:22:38 +0100, Michal Privoznik wrote: > > > > [...] > > > > > > > > > > How can this be considered success? Also this introduces a possible > > > > regression. The DEVICE_DELETED event should be fired only after the > > > > device was entirely unplugged. Claiming success before seeing the event > > > > can lead to another race when qemu deleted the device from the internal > > > > list so that 'device_del' does not see it any more but did not finish > > > > cleanup fully. > > > > > > > > We need to start the '*Remove' handler only after the DEVICE_DELETED > > > > event was received. > > > > > > I beg to differ. If we were to report error here users would see the API > > > failing with error "Device not found". So they'd run 'virsh dumpxml' only to > > > find the device there. I don't find such behaviour sane. If one API tells me > > > a devie is not there then another one shall not tell otherwise. > > > > Well. The user semantics can be confusing here. What we can't allow > > though is that some of the steps done in the qemuDomainRemove*Device > > will fail because qemu will still have some internal reference to some > > backend object. > > I'm not quite sure I follow. qemuDomainRemove*Device will be run exactly > once. Not any more times. Running it more times is a problem, but I'm > failing to see how my patch allows that. Can you shed more light into that > please? What I've meant is that qemuDomainRemove*Device shall never be called earlier than DEVICE_DELETED is received. What I've forgot to take into account is that we will still call qemuDomainWaitForDeviceRemoval (as your comment mentions). This means that the scenario I was outlining will not happen as success from this API does not automatically mean we'll try to delete the backends of the device (which I didn't notice at first). I'd probably just delete the mention of dumping XML. I think the statement that qemuDomainWaitForDeviceRemoval should be clear enough.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
