[libvirt] [PATCH] qemu_agent: support qemu agent general command

[Eric Blake]

On 08/05/2012 04:49 PM, MATSUDA, Daiki wrote:
> ACK or do I need to re-write ?

Given this earlier review...

>>>> Rather than just 2 flags, can we take an integer timeout parameter?  If
>>>> the parameter is -1, then we don't wait; if the parameter is 0, then we
>>>> wait forever, otherwise, we wait for the user-specified timeout.
>>>
>>> Okay, but I think the interpretation of values should be reversed:
>>>   -1 for wait forever
>>>    0 not to wait at all
>>>
>>> I think it's more mnemonic since -1 is all bits set thus huge number
>>> hence evokes infinity. Wait for zero time units means no wait at all.
>>
>> Definitely argues that -1 and 0 should have named aliases.
>>
>> Or even use the NULL-ness of result to determine whether to wait:
>>
>> /**
>>   * virDomainQemuAgentCommand:
>>   *
>>   * Issue @cmd to the guest agent running in @domain.
>>   * If @result is NULL, then don't wait for a result (and @timeout
>>   * must be 0).  Otherwise, wait for @timeout seconds for a
>>   * successful result to be populated into *@result, with
>>   * VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK (-1) meaning to block
>>   * forever waiting for a result.
>>   */
>> int virDomainQemuAgentCommand(virDomainPtr domain, const char *cmd,
>>                                char **result, unsigned int timeout,
>>                                unsigned int flags);
>>
>> And I still stand by my earlier review comment that this needs to be
>> split into multiple patches (introduce public API separate from the
>> qemu-specific implementation of that API, even if the API is only usable
>> by qemu).

I think that the state of this patch right now is waiting for you to do
a rewrite that splits it into several patches and takes into account the
review comments on altering the API.  It's still a good idea, but I'm
personally a bit swamped to take over the series myself without first
seeing a more up-to-date posting.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120806/61a7aff5/attachment-0001.sig>