examples/object-events/event-test.c | 34 +++++++++++ include/libvirt/libvirt-domain.h | 33 +++++++++++ src/conf/domain_event.c | 89 +++++++++++++++++++++++++++++ src/conf/domain_event.h | 12 ++++ src/libvirt_private.syms | 2 + src/qemu/qemu_driver.c | 43 +++++++------- src/qemu/qemu_hotplug.c | 7 +++ src/remote/remote_daemon_dispatch.c | 40 +++++++++++++ src/remote/remote_driver.c | 32 +++++++++++ src/remote/remote_protocol.x | 16 +++++- src/remote_protocol-structs | 8 +++ tools/virsh-domain.c | 31 ++++++++++ 12 files changed, 325 insertions(+), 22 deletions(-)
Unfortunately, we can't emit VIR_DOMAIN_EVENT_ID_DEVICE_ADDED or VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED event because that carries device alias within itself and leases don't have one. Michal Prívozník (3): qemuDomainAttachDeviceLive: Rework event emitting Introduce VIR_DOMAIN_EVENT_ID_LEASE_CHANGE qemu: Emit event on lease attach/detach examples/object-events/event-test.c | 34 +++++++++++ include/libvirt/libvirt-domain.h | 33 +++++++++++ src/conf/domain_event.c | 89 +++++++++++++++++++++++++++++ src/conf/domain_event.h | 12 ++++ src/libvirt_private.syms | 2 + src/qemu/qemu_driver.c | 43 +++++++------- src/qemu/qemu_hotplug.c | 7 +++ src/remote/remote_daemon_dispatch.c | 40 +++++++++++++ src/remote/remote_driver.c | 32 +++++++++++ src/remote/remote_protocol.x | 16 +++++- src/remote_protocol-structs | 8 +++ tools/virsh-domain.c | 31 ++++++++++ 12 files changed, 325 insertions(+), 22 deletions(-) -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 4/5/19 3:57 AM, Michal Privoznik wrote: > Unfortunately, we can't emit VIR_DOMAIN_EVENT_ID_DEVICE_ADDED or > VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED event because that carries device > alias within itself and leases don't have one. > Hmm. I understand that <leases> aren't really devices so it doesn't have any direct reason to give it an info structure and assign it an alias. But that's really an argument for why they shouldn't have been a device to begin with. I presume we did it that way to take advantage of the existing hotplug APIs. But then adding a special purpose event seems like going in the opposite direction. How invasive is it to add an 'info' bit to lease devices, allocate aliases with the rest of qemu devices, maybe add an ADDRESS_TYPE_LEASE or something that is just an explicit no op for the ADDRESS consumers. My guess is it's not too bad but there could be dragons lurking There's already places internally where this would simplify things: it's annoying that device iterator code already needs to play games like 'check the address of all devices, oh except leases and graphics which don't have info, oh and hostdev info is a pointer for some reason' - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 4/16/19 1:39 AM, Cole Robinson wrote: > On 4/5/19 3:57 AM, Michal Privoznik wrote: >> Unfortunately, we can't emit VIR_DOMAIN_EVENT_ID_DEVICE_ADDED or >> VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED event because that carries device >> alias within itself and leases don't have one. >> > > Hmm. I understand that <leases> aren't really devices so it doesn't have > any direct reason to give it an info structure and assign it an alias. > But that's really an argument for why they shouldn't have been a device > to begin with. I presume we did it that way to take advantage of the > existing hotplug APIs. But then adding a special purpose event seems > like going in the opposite direction. > > How invasive is it to add an 'info' bit to lease devices, allocate > aliases with the rest of qemu devices, maybe add an ADDRESS_TYPE_LEASE > or something that is just an explicit no op for the ADDRESS consumers. > My guess is it's not too bad but there could be dragons lurking > > There's already places internally where this would simplify things: it's > annoying that device iterator code already needs to play games like > 'check the address of all devices, oh except leases and graphics which > don't have info, oh and hostdev info is a pointer for some reason' Yeah, it's annoying, but I remember discussion from when I was introducing user aliases. I wanted to just have some dummy element that would not mean anything to libvirt but users could set it and then match devices using it as a mark. It was rejected because it doesn't map onto anything in qemu. Well, nor do leases, nor would their address. I don't have a preference here really. But if we wanted to invent addresees for leases then I guess we would have to do it for already running domains too (when daemon restarts). I guess we need somebody else's input here. Dan/Peter? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Apr 23, 2019 at 03:06:22PM +0200, Michal Privoznik wrote: > On 4/16/19 1:39 AM, Cole Robinson wrote: > > On 4/5/19 3:57 AM, Michal Privoznik wrote: > > > Unfortunately, we can't emit VIR_DOMAIN_EVENT_ID_DEVICE_ADDED or > > > VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED event because that carries device > > > alias within itself and leases don't have one. > > > > > > > Hmm. I understand that <leases> aren't really devices so it doesn't have > > any direct reason to give it an info structure and assign it an alias. > > But that's really an argument for why they shouldn't have been a device > > to begin with. I presume we did it that way to take advantage of the > > existing hotplug APIs. But then adding a special purpose event seems > > like going in the opposite direction. > > > > How invasive is it to add an 'info' bit to lease devices, allocate > > aliases with the rest of qemu devices, maybe add an ADDRESS_TYPE_LEASE > > or something that is just an explicit no op for the ADDRESS consumers. > > My guess is it's not too bad but there could be dragons lurking > > > > There's already places internally where this would simplify things: it's > > annoying that device iterator code already needs to play games like > > 'check the address of all devices, oh except leases and graphics which > > don't have info, oh and hostdev info is a pointer for some reason' > > Yeah, it's annoying, but I remember discussion from when I was introducing > user aliases. I wanted to just have some dummy element that would not mean > anything to libvirt but users could set it and then match devices using it > as a mark. It was rejected because it doesn't map onto anything in qemu. > Well, nor do leases, nor would their address. > > I don't have a preference here really. But if we wanted to invent addresees > for leases then I guess we would have to do it for already running domains > too (when daemon restarts). My preference is for the new event as this series does. We shouldn't try to force something to be more like a device when it clearly isn't. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 4/23/19 9:32 AM, Daniel P. Berrangé wrote: > On Tue, Apr 23, 2019 at 03:06:22PM +0200, Michal Privoznik wrote: >> On 4/16/19 1:39 AM, Cole Robinson wrote: >>> On 4/5/19 3:57 AM, Michal Privoznik wrote: >>>> Unfortunately, we can't emit VIR_DOMAIN_EVENT_ID_DEVICE_ADDED or >>>> VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED event because that carries device >>>> alias within itself and leases don't have one. >>>> >>> >>> Hmm. I understand that <leases> aren't really devices so it doesn't have >>> any direct reason to give it an info structure and assign it an alias. >>> But that's really an argument for why they shouldn't have been a device >>> to begin with. I presume we did it that way to take advantage of the >>> existing hotplug APIs. But then adding a special purpose event seems >>> like going in the opposite direction. >>> >>> How invasive is it to add an 'info' bit to lease devices, allocate >>> aliases with the rest of qemu devices, maybe add an ADDRESS_TYPE_LEASE >>> or something that is just an explicit no op for the ADDRESS consumers. >>> My guess is it's not too bad but there could be dragons lurking >>> >>> There's already places internally where this would simplify things: it's >>> annoying that device iterator code already needs to play games like >>> 'check the address of all devices, oh except leases and graphics which >>> don't have info, oh and hostdev info is a pointer for some reason' >> >> Yeah, it's annoying, but I remember discussion from when I was introducing >> user aliases. I wanted to just have some dummy element that would not mean >> anything to libvirt but users could set it and then match devices using it >> as a mark. It was rejected because it doesn't map onto anything in qemu. >> Well, nor do leases, nor would their address. >> >> I don't have a preference here really. But if we wanted to invent addresees >> for leases then I guess we would have to do it for already running domains >> too (when daemon restarts). > > My preference is for the new event as this series does. We shouldn't try > to force something to be more like a device when it clearly isn't. IMO that ship has sailed. From the API perspective it's largely already a device: it appears in the <devices> block of the domain, it's added/removed/updated with the *Device* APIs. Yes it shouldn't output or accept an <address>, but that's basically its current state. If we add an .info struct internally I suspect it won't take much extra work to preserve that behavior. For info alias I think we should be adding one anyways. From my reading the primary motivation of the user aliases support is that it gives API users a unique way to identify the device, which seems just as valid for leases as anything else listed in <devices>. BTW I think all that logic applies to <graphics> devices as well. They don't have .info internally, so don't get aliases. If there's a future when <graphics> is hotpluggable we are going to have to add a custom event for that too. The alias uniqueness bit applies as well, though not too much an issue in practice for qemu because we enforce that only a single <graphics> for each @type is allowed (but that's an artificial distinction AFAICT, qemu looks like it can support multiple vnc server instances for example) Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.