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

ACK or do I need to re-write ?

>>> 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).