Description of problem: When a device lease represented by <lease> element is hot unplugged, VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED is not generated. Device leases also do not have aliases so they wouldn't be identifiable even if the event was provided. Version-Release number of selected component (if applicable): 4.5.0-10.el7_6.2 How reproducible: 100% Steps to Reproduce: 1. Start a VM. 2. Hot plug <lease> to it. 3. Hot unplug the lease. 4. Check for VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED event. Actual results: The event is not received. Expected results: The event is received. Additional info: Although leases are not real devices and are handled internally by libvirt without QEMU involvement, they are presented to the user basically the same way as other devices. One would expect that they have aliases and virDomainDetachDevice works similarly to other devices. It's impractical for the user to handle them differently from other devices.
Can be reproduced with: libvirt-4.5.0-10.el7_6.2.x86_64 qemu-kvm-rhev-2.12.0-18.el7_6.1.x86_64 sanlock-3.6.0-1.el7.x86_64 sanlock-lib-3.6.0-1.el7.x86_64 libvirt-lock-sanlock-4.5.0-10.el7_6.2.x86_64 Steps: 1. sanlock setup: (1)Enable libvirt using sanlock in selinux. # setsebool -P virt_use_sanlock 1 (2)Set configurations for sanlock # cat /etc/libvirt/qemu.conf lock_manager = "sanlock" # cat /etc/libvirt/qemu-sanlock.conf user = "sanlock" group = "sanlock" host_id = 1 auto_disk_leases = 0 disk_lease_dir = "/var/lib/libvirt/sanlock" require_lease_for_disks = 0 (3) Restart services # systemctl start sanlock # systemctl restart libvirtd (4) Prepare lockspace and lease file for sanlock # truncate -s 1M /var/lib/libvirt/sanlock/TEST_LS # sanlock direct init -s TEST_LS:0:/var/lib/libvirt/sanlock/TEST_LS:0 init done 0 # chown sanlock:sanlock /var/lib/libvirt/sanlock/TEST_LS # restorecon -R -v /var/lib/libvirt/sanlock # sanlock client host_status -s TEST_LS # truncate -s 1M /var/lib/libvirt/sanlock/test-disk-resource-lock # sanlock direct init -r TEST_LS:test-disk-resource-lock:/var/lib/libvirt/sanlock/test-disk-resource-lock:0 init done 0 # chown sanlock:sanlock /var/lib/libvirt/sanlock/test-disk-resource-lock # sanlock client add_lockspace -s TEST_LS:1:/var/lib/libvirt/sanlock/TEST_LS:0 add_lockspace add_lockspace done 0 2.Prepare lease xml: # cat /root/xml-bak/lease0.xml <lease> <lockspace>TEST_LS</lockspace> <key>test-disk-resource-lock</key> <target path='/var/lib/libvirt/sanlock/test-disk-resource-lock'/> </lease> 3.hotplug the lease device to a running guest: [terminal 1]# virsh event --loop --all [terminal 2]# virsh attach-device yan-V-local /root/xml-bak/lease0.xml Device attached successfully # virsh dumpxml yan-V-local |grep lease -3 ... <lease> <lockspace>TEST_LS</lockspace> <key>test-disk-resource-lock</key> <target path='/var/lib/libvirt/sanlock/test-disk-resource-lock'/> </lease> ... # virsh detach-device yan-V-local /root/xml-bak/lease0.xml Device detached successfully # virsh dumpxml yan-V-local |grep lease -3 4. Check the events: [terminal 1]# virsh event --loop --all (no any events captured) Additional info: Check libvirtd.log, no any EVENT received from qemu such as hotunplug a disk: 2018-10-17 10:46:42.950+0000: 14565: info : qemuMonitorJSONIOProcessLine:212 : QEMU_MONITOR_RECV_EVENT: mon=0x7fda5002e2c0 event={"timestamp": {"seconds": 1539773202, "microseconds": 950196}, "event": "DEVICE_DELETED", "data": {"device": "virtio-disk1", "path": "/machine/peripheral/virtio-disk1"}}
Additional for the alias issue: 1. Try to edit guest xml with alias for lease device: <lease> <lockspace>TEST_LS</lockspace> <key>test-disk-resource-lock</key> <alias name='ua-42a403f7-b6b8-4e12-85dd-4dfe06d9073a'/> <target path='/var/lib/libvirt/sanlock/test-disk-resource-lock'/> </lease> # virsh edit yan-V-local error: XML document failed to validate against schema: Unable to validate doc against /usr/share/libvirt/schemas/domain.rng Extra element devices in interleave Element domain failed to validate content Failed. Try again? [y,n,i,f,?]: 2. # cat /usr/share/libvirt/schemas/domaincommon.rng|grep '<define name="lease">' -A25 <define name="lease"> <element name="lease"> <interleave> <element name="lockspace"> <text/> </element> <element name="key"> <text/> </element> <element name="target"> <attribute name="path"> <text/> </attribute> <optional> <attribute name="offset"> <ref name="unsignedInt"/> </attribute> </optional> </element> </interleave> </element> </define> ... No <ref name="alias"/> for it.
(In reply to Milan Zamazal from comment #0) > Description of problem: > > When a device lease represented by <lease> element is hot unplugged, > VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED is not generated. Device leases also do > not have aliases so they wouldn't be identifiable even if the event was > provided. > This exactly is the problem. Leases do not have an alias and since DEVICE_REMOVED event consists solely of virDomainPtr and device alias I don't quite see how we could bend the event to carry some sensible information on lease detach. But what we can do is introduce another type of event (say LEASE_REMOVED) which would be issued on a lease detach (hand in hand we would have to have LEASE_ADDED). Would this be enough for you? What is it exactly that you want to achieve?
I'd like to handle device removal the same way as other device hot unplugs. Special events for leases wouldn't help much -- I could replace sleep with waiting for the event, but that's all. I would have still to check domain XML to identify the removed lease and to handle leases in a different way than other devices. Why can't aliases be added to leases?
(In reply to Milan Zamazal from comment #6) > I'd like to handle device removal the same way as other device hot unplugs. > Special events for leases wouldn't help much -- I could replace sleep with > waiting for the event, but that's all. I would have still to check domain > XML to identify the removed lease and to handle leases in a different way > than other devices. Why would you? The LEASE_REMOVED event would have enough arguments to uniquely identify the lease. In fact it can have all three strings (lockpace, key and target path). > > Why can't aliases be added to leases? We can add user aliases for leases [1], but that won't help. What event should libvirt issue if no alias was set in the domain XML? 1: Well sort of. Aliases are basically object ID's for qemu. And lease is not something that qemu sees. IOW, alias for a lease is not visible to anybody - sanlock won't use/see it. Nor virtlockd.
(In reply to Michal Privoznik from comment #7) > The LEASE_REMOVED event would have enough arguments to > uniquely identify the lease. In fact it can have all three strings > (lockpace, key and target path). I see. OK, then having such an event would be helpful. > We can add user aliases for leases [1], but that won't help. What event > should libvirt issue if no alias was set in the domain XML? Doesn't libvirt add its own aliases to devices without user aliases? Nevertheless considering that we don't add user aliases to leases nowadays, we couldn't utilize them easily in the future anyway. So having LEASE_* events as you suggest should be fine.
(In reply to Milan Zamazal from comment #8) > > Doesn't libvirt add its own aliases to devices without user aliases? Not to all devices. Only those which will appear in qemu. Leases don't have aliases. Thus there is nothing to generate. > Nevertheless considering that we don't add user aliases to leases nowadays, > we couldn't utilize them easily in the future anyway. So having LEASE_* > events as you suggest should be fine. Okay. Let's go this way then.
I've proposed patches on the list: https://www.redhat.com/archives/libvir-list/2019-April/msg00466.html
Another try: https://www.redhat.com/archives/libvir-list/2020-May/msg00944.html
After evaluating this issue, there are no plans to address it further or fix it in an upcoming release. Therefore, it is being closed. If plans change such that this issue will be fixed in an upcoming release, then the bug can be reopened.