于 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list