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

[John Ferlan]

On 04/14/2015 09:35 AM, Ján Tomko wrote:
> 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
> 

OK - explanation is fine with me.

ACK -

John