[libvirt] [PATCH] SNMP: include inactive domains in guest table

[Michal Privoznik]

On 31.05.2012 00:04, Jonathan Daugherty wrote:
> Hi,
> 
> I've written a patch to include inactive domains in the guest table
> available via the libvirt-snmp subagent.  This makes it possible to
> start domains via SNMP (a state transition which AFAICT was previously
> impossible, because inactive domains never appeared in the SNMP
> result).
> 
> The code now collects the IDs of running domains along with the names
> of declared (but not running) domains and normalizes them into a list
> of domain pointers, which is then processed by the preexisting SNMP
> container row allocation code.  The change was fairly straightforward,
> but certainly there might be Good Reasons it wasn't already
> implemented.
> 
> As part of this change, the other (noteworthy) change I made was to
> move calls to virDomainFree() to the end of libvirtSnmpLoadGuests()
> since I have to free all of the used domain pointers there anyway.
> 
> Some other issues which may need to be addressed:
> 
>   - This will be a breaking change for anyone who assumes that the old
>     behavior is still in place (i.e., that the guest table only
>     includes non-shutdown domains).  However, it might not be *that*
>     bad because the domain rows already included a 'state' that one
>     could already filter on.
> 
>   - I don't know if there exists other code in libvirt-snmp that
>     assumes inactive domains can't appear in the table.
> 
>   - I have not tested the inactive -> active state transition (because
>     I don't want to enable writable SNMP OIDs on my server).
> 
> Please let me know if there is anything further I need to do to make
> the patch acceptable.
> 
> Thanks for libvirt-snmp!
> 

Firstly, have you known that you are the first person outside RH to post
patch for libvirt-snmp? It's awesome.

Secondly, as Eric replied, I'd wait for the new API as well, so we can
do an atomic list. Therefore ACK to the idea. That's for sure.
So are you comfortable with this resolution?

Michal