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