Re: [PATCH 3/4] virsh: add event command, for lifecycle events

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/20/2014 10:33 AM, Daniel P. Berrange wrote:
> On Fri, Feb 14, 2014 at 05:21:40PM -0700, Eric Blake wrote:
>> Add 'virsh event --list' and 'virsh event [dom] --event=name
>> [--loop] [--timeout]'.  Borrows somewhat from event-test.c,
>> but defaults to a one-shot notification, and takes advantage
>> of the event loop integration to allow Ctrl-C to interrupt the
>> wait for an event.  For now, this just does lifecycle events.
>>
>> * tools/virsh.pod (event): Document new command.
>> * tools/virsh-domain.c (vshDomainEventToString)
>> (vshDomainEventDetailToString, vshDomEventData)
>> (vshEventLifecyclePrint, cmdEvent): New struct and functions.
> 
> ACK
> 

>> +    case VIR_DOMAIN_EVENT_DEFINED:
>> +        ret = _("Defined");
>> +        break;
> 

> How about using VIR_ENUM ?
> 
> We avoided it in the event-test.c file since we wanted it to
> be example code people can compile outside libvirt. Using
> enums would be fine for virsh though i think

VIR_ENUM doesn't allow _("") translation.  This output is human legible,
so we want it to appear in the user's locale (see also
vshDomainVcpuStateToString() and friends).

>> +++ b/tools/virsh.pod
>> +By default, tihs command is one-shot, and returns success once an event
> 
> s/tihs/this/ 
> 
> Rare case of me spotting a typo in your code, instead of the
> reverse :-)

Hey - I have to inject errors somewhere, to make the review worthwhile,
right? :)

Fixed, and will push the series shortly as well as crank out patch 5/4
for the remaining events.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]