[libvirt] [PATCH 3/8] UndefineFlags: Implement the public API

[Osier Yang]

于 2011年07月14日 01:53, Eric Blake 写道:
> On 07/13/2011 04:19 AM, Osier Yang wrote:
>> ---
>>   src/libvirt.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 files changed, 52 insertions(+), 1 deletions(-)
>>
>>
> per my comments in 2/8,
>
>>   /**
>> + * virDomainUndefineWithFlags:
> s/virDomainUndefineWithFlags/virDomainUndefineFlags/
>
>> + * @domain: pointer to a defined domain
>> + * @flags: bitwise-or of supported virDomainUndefineFlags
>
> s/virDomainUndefineFlags/virDomainUndefineFlagValues/
>
>> + *
>> + * Undefine a domain but does not stop it if it is running
> Copy and paste from the existing virDomainUndefine, but I think it would
> read better (in both places) as:
>
> Undefine a persistent domain.  A running domain is left running but
> converted into a transient domain.
>
>> + *
>> + * If VIR_DOMAIN_UNDEFINE_MANAGED_STATE is specified, the domain
> s/the/any/ - since a managed state file might not exist
>
>> + * managed state file will be removed along with domain undefine
>> + * process, the entire domain undefine process will fail if
> s/process, the/process, and the/
>
>> + * it fails on removing the managed state file.
> We also need to clearly document what happens if a managed state file
> exists but the flag is not present.  My preference would be rewording
> this entire paragraph as:
>
> If the domain has a managed state file (see
> virDomainHasManagedSaveImage()), then including
> VIR_DOMAIN_UNDEFINE_MANAGED_STATE in @flags will also remove that file,
> and omitting the flag will cause the undefine process to fail.
>
> and back at virDomainUndefine, add a paragraph:
>
> If the domain has a managed state file (see
> virDomainHasManagedSaveImage()), then the undefine will fail.  See
> virDomainUndefineFlags() for more control.
>
> The code additions look fine except for the choice of API name, but the
> documentation is just as important, so this needs a v2.  Also, I'd
> probably squash patch 2 and 3 into one patch - that is, it would be nice
> to add both the declaration and the documentation of the API in the same
> patch.
>

This is nice document, but is it good to change behaviour of 
virDomainUndefine?
(fails if it has a managed state file)

Osier