Re: [libvirt] [PATCH 1/2] Add VIR_DOMAIN_EVENT_ID_DEVICE_ADDED event

On Mon, Apr 13, 2015 at 01:59:39PM -0400, John Ferlan wrote: > > > On 04/04/2015 01:16 PM, Ján Tomko wrote: >> The counterpart to VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED. >> >> https://bugzilla.redhat.com/show_bug.cgi?id=1206114 >> --- >> daemon/remote.c | 37 +++++++++++++++++++ >> include/libvirt/libvirt-domain.h | 18 ++++++++++ >> src/conf/domain_event.c | 77 ++++++++++++++++++++++++++++++++++++++++ >> src/conf/domain_event.h | 6 ++++ >> src/libvirt_private.syms | 2 ++ >> src/remote/remote_driver.c | 29 +++++++++++++++ >> src/remote/remote_protocol.x | 14 +++++++- >> src/remote_protocol-structs | 6 ++++ >> tools/virsh-domain.c | 20 +++++++++++ >> 9 files changed, 208 insertions(+), 1 deletion(-) >> > > I searched on VIR_DOMAIN_EVENT_ID_DEVICE - what about > examples/object-events/event-test.c ? > This patch already includes an example in tools/virsh :) I have sent a separate patch adding it to event-test as well. (I would have separated this patch too, as suggested by the http://libvirt.org/api_extension.html page, but it failed to compile separately due to either ACL check or unhandled enum values) > Also should 'src/libvirt-domain.c' have a description for the _ADDED > flag in 'virDomainAttachDeviceFlags' like there is for _REMOVED in > 'virDomainDetachDeviceFlags'? No. With DetachDevice, the device may still not be detached even if the API returns and the caller needs to wait for the event to be sure. Attaching a device is synchronous, so waiting for the event only makes sense if the domain was changed from another connection. Generally, we don't seem to document events in the APIs that cause them. > (although even that text is a bit shy of > an 'a' - as in "or add a handler for" rather than "or add handler for" > <sigh> Fixed: http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=682ba8e9 > > ACK in general for what's here and with the new test and change to > AttachDevice description... > I don't think either of those should be included in the patch. Jan