On 07/15/2011 03:06 AM, Osier Yang wrote:

This introduce a new API virDomainUndefineFlags to control the

s/introduce/introduces/

domain undefine process, as the existed API virDomainUndefine

s/existed/existing/

doesn't support flags.

Currently only flag VIR_DOMAIN_UNDEFINE_MANAGED_STATE is supported. If the domain has a managed state file, including VIR_DOMAIN_UNDEFINE_MANAGED_STATE in @flags will also remove that file, and omitting the flag will cause undefine process to fail.

This patch also changes the behavior of virDomainUndefine, if the domain has a managed state file, the undefine will be refused.

I also just realized that this is the right change, because by default, libvirt should never allow data loss without explicit action. If a domain has managed state and libvirt were to silently delete the file, then you have lost the data that was in memory at the time the managed state file was created. And undefining a domain while leaving the state file behind opens the door for some other domain to be created and clobber the state file, which is also data loss. So even though it is a semantic change to refuse to delete a domain with managed save data, it fixes a data loss bug so it is the right thing to do.

+++ b/src/libvirt.c @@ -6376,7 +6376,11 @@ error: * virDomainUndefine: * @domain: pointer to a defined domain * - * Undefine a domain but does not stop it if it is running + * Undefine the configuration for an inactive domain.

Per the discussion in related threads, we want this to read more like: Undefine a domain. If the domain is running, this converts it to a transient domain, without halting the guest. If the domain is inactive, this removes all traces of the domain.

+ * + * If the domain has a managed state file (see + * virDomainHasManagedSaveImage()), then the undefine will fail. See + * virDomainUndefineFlags() for more control. * * Returns 0 in case of success, -1 in case of error */ @@ -6415,6 +6419,56 @@ error: }

/** + * virDomainUndefineFlags: + * @domain: pointer to a defined domain + * @flags: bitwise-or of supported virDomainUndefineFlagsValues + * + * Undefine the configuration for an inactive domain.

Same paragraph change as for virDomainUndefine. ACK with those paragraphs changed (and we'll have to fix qemu behavior in a later patch). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 07/15/2011 03:06 AM, Osier Yang wrote:

--- src/remote/remote_driver.c | 3 ++- src/remote/remote_protocol.x | 9 ++++++++- src/remote_protocol-structs | 4 ++++ 3 files changed, 14 insertions(+), 2 deletions(-)

+++ b/src/remote_protocol-structs @@ -533,6 +533,10 @@ struct remote_domain_define_xml_ret { struct remote_domain_undefine_args { remote_nonnull_domain dom; }; +struct remote_domain_undefine_flags_args { + remote_nonnull_domain dom; + u_int flags; +}; struct remote_domain_inject_nmi_args { remote_nonnull_domain dom; u_int flags;

ACK, but be sure to run 'make check' before committing, because you'll have to add a line to remote_protocol-structs if I get my enum-checking patch pushed first. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 07/15/2011 03:06 AM, Osier Yang wrote:

* src/qemu/qemu_driver.c: New call back for qemu_driver, New function qemudDomainUndefineFlags, and changes on qemudDomainUndefine. --- src/qemu/qemu_driver.c | 35 ++++++++++++++++++++++++++++++++++- 1 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3d4207e..0bf135d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4303,12 +4303,18 @@ cleanup: return dom; }

-static int qemudDomainUndefine(virDomainPtr dom) { +static int +qemudDomainUndefineFlags(virDomainPtr dom, + unsigned int flags)

As long as we're touching this, s/qemudDomain/qemuDomain/

+ name = qemuDomainManagedSavePath(driver, vm); + if (name == NULL) + goto cleanup; + + if (virFileExists(name)) { + if ((flags & VIR_DOMAIN_UNDEFINE_MANAGED_STATE) + && (unlink(name) < 0)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed on removing domain managed "

Grammar.

+ "state file '%s'"), name);

No need to expose our internal file name details to the user.

+ goto cleanup; + } else { + qemuReportError(VIR_ERR_INTERNAL_ERROR,

Not an internal error.

+ _("Refusing to undefine with managed state " + "file '%s' exists"), name);

Grammar.

+ goto cleanup; + }

Logic bug. This fails with "Refusing to undefine" if the unlink() succeeds. Rather, you want: if (virFileExists(name)) { if (flags & VIR_DOMAIN_UNDEFINE_MANAGED_STATE) { if (unlink(name) < 0) { virReportSystemError(errno, "%s", _("Failed to remove managed state for domain")); goto cleanup; } } else { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Refusing to undefine domain with managed state")); goto cleanup; } } Needs a v3. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

* src/qemu/qemu_driver.c: New call back for qemu_driver, New function qemudDomainUndefineFlags, and changes on qemudDomainUndefine. --- src/qemu/qemu_driver.c | 36 +++++++++++++++++++++++++++++++++++- 1 files changed, 35 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8d54e58..0310b45 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4311,12 +4311,18 @@ cleanup: return dom; } -static int qemudDomainUndefine(virDomainPtr dom) { +static int +qemuDomainUndefineFlags(virDomainPtr dom, + unsigned int flags) +{ struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; virDomainEventPtr event = NULL; + char *name = NULL; int ret = -1; + virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_STATE, -1); + qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -4340,6 +4346,26 @@ static int qemudDomainUndefine(virDomainPtr dom) { goto cleanup; } + name = qemuDomainManagedSavePath(driver, vm); + if (name == NULL) + goto cleanup; + + if (virFileExists(name)) { + if (flags & VIR_DOMAIN_UNDEFINE_MANAGED_STATE) { + if (unlink(name) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to remove domain managed " + "save image")); + goto cleanup; + } + } else { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Refusing to undefine while managed save " + "image exists")); + goto cleanup; + } + } + if (virDomainDeleteConfig(driver->configDir, driver->autostartDir, vm) < 0) goto cleanup; @@ -4354,6 +4380,7 @@ static int qemudDomainUndefine(virDomainPtr dom) { ret = 0; cleanup: + VIR_FREE(name); if (vm) virDomainObjUnlock(vm); if (event) @@ -4363,6 +4390,12 @@ cleanup: } static int +qemudDomainUndefine(virDomainPtr dom) +{ + return qemuDomainUndefineFlags(dom, 0); +} + +static int qemuDomainAttachDeviceDiskLive(struct qemud_driver *driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev) @@ -8550,6 +8583,7 @@ static virDriver qemuDriver = { .domainCreateWithFlags = qemudDomainStartWithFlags, /* 0.8.2 */ .domainDefineXML = qemudDomainDefine, /* 0.2.0 */ .domainUndefine = qemudDomainUndefine, /* 0.2.0 */ + .domainUndefineFlags = qemuDomainUndefineFlags, /* 0.9.4 */ .domainAttachDevice = qemuDomainAttachDevice, /* 0.4.1 */ .domainAttachDeviceFlags = qemuDomainAttachDeviceFlags, /* 0.7.7 */ .domainDetachDevice = qemuDomainDetachDevice, /* 0.5.0 */ -- 1.7.6

This is just similiar as changes on qemu driver. * src/libxl/libxl_driver.c: New callback for libxl_driver, new function libxlDomainUndefineFlags, and changes libxlDomainUndefine as a wrapper of libxlDomainUndefineFlags. --- src/libxl/libxl_driver.c | 34 +++++++++++++++++++++++++++++++++- 1 files changed, 33 insertions(+), 1 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index cc37d05..0bbf113 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2724,13 +2724,17 @@ cleanup: } static int -libxlDomainUndefine(virDomainPtr dom) +libxlDomainUndefineFlags(virDomainPtr dom, + unsigned int flags) { libxlDriverPrivatePtr driver = dom->conn->privateData; virDomainObjPtr vm; virDomainEventPtr event = NULL; + char *name = NULL; int ret = -1; + virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE, -1); + libxlDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -2755,6 +2759,26 @@ libxlDomainUndefine(virDomainPtr dom) goto cleanup; } + name = libxlDomainManagedSavePath(driver, vm); + if (name == NULL) + goto cleanup; + + if (virFileExists(name)) { + if (flags & VIR_DOMAIN_UNDEFINE_MANAGED_SAVE) { + if (unlink(name) < 0) { + libxlError(VIR_ERR_INTERNAL_ERR, "%s", + _("Failed to remove domain managed save " + "image")); + goto cleanup; + } + } else { + libxlError(VIR_ERR_OPERATION_INVALID, "%s", + _("Refuse to undefine while managed save " + "image exists")); + goto cleanup; + } + } + if (virDomainDeleteConfig(driver->configDir, driver->autostartDir, vm) < 0) @@ -2768,6 +2792,7 @@ libxlDomainUndefine(virDomainPtr dom) ret = 0; cleanup: + VIR_FREE(name); if (vm) virDomainObjUnlock(vm); if (event) @@ -2777,6 +2802,12 @@ libxlDomainUndefine(virDomainPtr dom) } static int +libxlDomainUndefine(virDomainPtr dom) +{ + return libxlDomainUndefineFlags(dom, 0); +} + +static int libxlDomainChangeEjectableMedia(libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, virDomainDiskDefPtr disk) { @@ -3836,6 +3867,7 @@ static virDriver libxlDriver = { .domainCreateWithFlags = libxlDomainCreateWithFlags, /* 0.9.0 */ .domainDefineXML = libxlDomainDefineXML, /* 0.9.0 */ .domainUndefine = libxlDomainUndefine, /* 0.9.0 */ + .domainUndefineFlags = libxlDomainUndefineFlags, /* 0.9.4 */ .domainAttachDevice = libxlDomainAttachDevice, /* 0.9.2 */ .domainAttachDeviceFlags = libxlDomainAttachDeviceFlags, /* 0.9.2 */ .domainDetachDevice = libxlDomainDetachDevice, /* 0.9.2 */ -- 1.7.6

On 07/15/2011 03:06 AM, Osier Yang wrote:

Add domainUndefineFlags support for these drivers (except qemu and libxl driver), and changes domainUndefine as a wrapper of domainUndefineFlags, but they actually perform same. --- src/esx/esx_driver.c | 11 ++++++++++- src/lxc/lxc_driver.c | 11 ++++++++++- src/openvz/openvz_driver.c | 11 ++++++++++- src/test/test_driver.c | 12 +++++++++++- src/uml/uml_driver.c | 12 +++++++++++- src/vmware/vmware_driver.c | 12 +++++++++++- src/xen/xen_driver.c | 9 ++++++++- 7 files changed, 71 insertions(+), 7 deletions(-)

@@ -3329,6 +3332,11 @@ esxDomainUndefine(virDomainPtr domain) }

+static int +esxDomainUndefine(virDomainPtr domain) +{ + esxDomainUndefineFlags(domain, 0);

Missing a return keyword. I'm guessing you aren't set up to compile-test ESX. ACK with that fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 07/15/2011 03:06 AM, Osier Yang wrote:

If the domain has managed state file, and --managed-state is not specified, then it fails with error prompt to tell user there is managed state file exists.

Grammar suggestion: then it fails with an error telling the user that a managed save still exists. Hmm, I'm now having second thoughts about the names "VIR_DOMAIN_UNDEFINE_MANAGED_STATE" and "--managed-state", since the name of the API is virDomainManagedSave and the virsh command is managedsave. Would it be better to go with: VIR_DOMAIN_UNDEFINE_MANAGED_SAVE and --managed-save? This would mean tweaking the earlier patches in this series.

And the domain has managed state file, and --managed-state is

s/And/If/

specified, it invokes virDomainUndefineFlags, if virDomainUndefineFlag fails, then it trys to remove the managed

s/trys/tries/

state file using virDomainManagedSaveRemove first, with invoking virDomainUndefine following. (For compatibility between new virsh with this patch and older libvirt without this fix)

Simliar if the domain has no managed state. See the codes for

s/Simliar/Similarly/

detail. --- tools/virsh.c | 44 +++++++++++++++++++++++++++++++++++++++++++- tools/virsh.pod | 6 +++++- 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c index 4af8fea..8a62612 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1409,6 +1409,7 @@ static const vshCmdInfo info_undefine[] = {

static const vshCmdOptDef opts_undefine[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name or uuid")}, + {"managed-state", VSH_OT_BOOL, 0, N_("remove domain managed state file")},

s/state/save/g, given my above thoughts.

{NULL, 0, 0, NULL} };

@@ -1419,6 +1420,16 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) bool ret = true; const char *name = NULL; int id; + int flags = 0; + int managed_state = vshCommandOptBool(cmd, "managed-state"); + int has_managed_state = 0; + int rc = -1; + + if (managed_state) + flags |= VIR_DOMAIN_UNDEFINE_MANAGED_STATE; + + if (!managed_state) + flags = -1;

I'm not sure if you need this line. Instead...

if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -1440,7 +1451,37 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) VSH_BYNAME|VSH_BYUUID))) return false;

- if (virDomainUndefine(dom) == 0) { + has_managed_state = virDomainHasManagedSaveImage(dom, 0); + if (has_managed_state < 0) + return false; + + if (flags == -1) {

...this conditional can just be if (!managed_state)

+ if (has_managed_state == 1) { + vshError(ctl, + _("Refusing to undefine with managed state " + "file exists"));

Grammar: Refusing to undefine while managed save image exists

+ return false; + } + + rc = virDomainUndefine(dom); + } else { + rc = virDomainUndefineFlags(dom, flags); + + /* It might fail for virDomainUndefineFlags is not + * supported on older libvirt, try to undefine the + * domain with combo virDomainManagedSaveRemove and + * virDomainUndefine. + */ + if (rc < 0) {

Here, we should optimize by checking the error. If the error is VIR_ERR_NO_SUPPORT, then we go with the fallback; but if it is anything else, then virDomainUndefineFlags exists but failed, and the fallback would also fail, so give up now.

+ if (has_managed_state && + virDomainManagedSaveRemove(dom, 0) < 0) + return false;

This early return...

+ + rc = virDomainUndefine(dom); + } + } + + if (rc == 0) { vshPrint(ctl, _("Domain %s has been undefined\n"), name); } else { vshError(ctl, _("Failed to undefine domain %s"), name);

...will lose out on this error message.

-=item B<undefine> I<domain-id> +=item B<undefine> I<domain-id> I<--managed-state>

managed-state (or managed-save, whatever we name it) is optional, so this should be: =item B<undefine> I<domain-id> [I<--managed-state>]

Undefine the configuration for an inactive domain. Since it's not running the domain name or UUID must be used as the I<domain-id>.

+The I<--managed-save> flag guarantees that any managed state (see the +B<managesave> command) is also cleaned up. Without the flag, attempts

s/managesave/managedsave/

+to undefine a domain with managed state will fail. + =item B<vcpucount> I<domain-id> optional I<--maximum> I<--current> I<--config> I<--live>

Probably needs a v3. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

If the domain has managed state file, and --managed-state is not specified, then it fails with an error telling the user that a managed save image still exists. If the domain has managed state file, and --managed-state is specified, it invokes virDomainUndefineFlags. If virDomainUndefineFlags fails, then it tries to remove the managed save image using virDomainManagedSaveRemove first, with invoking virDomainUndefine following. (For compatibility between new virsh with this patch and older libvirt without this patch). Similarly if the domain has no managed state. See the codes for detail. NOTE: Have not removing the codes checking if the domain is running in function "cmdUndefine", it will go along with qemu driver's fix (allow to undefine a running domain). --- tools/virsh.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++- tools/virsh.pod | 14 +++++++++++--- 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index e75a249..06eff0e 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1420,6 +1420,7 @@ static const vshCmdInfo info_undefine[] = { static const vshCmdOptDef opts_undefine[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name or uuid")}, + {"managed-save", VSH_OT_BOOL, 0, N_("remove domain managed state file")}, {NULL, 0, 0, NULL} }; @@ -1430,6 +1431,16 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) bool ret = true; const char *name = NULL; int id; + int flags = 0; + int managed_save = vshCommandOptBool(cmd, "managed-save"); + int has_managed_save = 0; + int rc = -1; + + if (managed_save) + flags |= VIR_DOMAIN_UNDEFINE_MANAGED_SAVE; + + if (!managed_save) + flags = -1; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -1447,11 +1458,50 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) virDomainFree(dom); return false; } + if (!(dom = vshCommandOptDomainBy(ctl, cmd, &name, VSH_BYNAME|VSH_BYUUID))) return false; - if (virDomainUndefine(dom) == 0) { + has_managed_save = virDomainHasManagedSaveImage(dom, 0); + if (has_managed_save < 0) { + virDomainFree(dom); + return false; + } + + if (flags == -1) { + if (has_managed_save == 1) { + vshError(ctl, + _("Refusing to undefine while domain managed save " + "image exists")); + virDomainFree(dom); + return false; + } + + rc = virDomainUndefine(dom); + } else { + rc = virDomainUndefineFlags(dom, flags); + + /* It might fail for virDomainUndefineFlags is not + * supported on older libvirt, try to undefine the + * domain with combo virDomainManagedSaveRemove and + * virDomainUndefine. + */ + if (rc < 0) { + virErrorPtr err = virGetLastError(); + if (err && err->code != VIR_ERR_NO_SUPPORT) + goto end; + + if (has_managed_save && + virDomainManagedSaveRemove(dom, 0) < 0) + goto end; + + rc = virDomainUndefine(dom); + } + } + +end: + if (rc == 0) { vshPrint(ctl, _("Domain %s has been undefined\n"), name); } else { vshError(ctl, _("Failed to undefine domain %s"), name); @@ -2436,6 +2486,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) VIR_FREE(seclabel); } } + virDomainFree(dom); return ret; } diff --git a/tools/virsh.pod b/tools/virsh.pod index 2d65cf9..377fac0 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -807,10 +807,18 @@ hypervisor. Output the device used for the TTY console of the domain. If the information is not available the processes will provide an exit code of 1. -=item B<undefine> I<domain-id> +=item B<undefine> I<domain-id> optional I<--managed-save> -Undefine the configuration for an inactive domain. Since it's not running -the domain name or UUID must be used as the I<domain-id>. +Undefine a domain. If the domain is running, this converts it to a +transient domain, without stopping it. If the domain is inactive, +the domain configuration is removed. + +The I<--managed-save> flag guarantees that any managed save image(see +the B<managedsave> command) is also cleaned up. Without the flag, attempts +to undefine a domain with managed save image will fail. + +NOTE: For an inactive domain, the domain name or UUID must be used as the +I<domain-id>. =item B<vcpucount> I<domain-id> optional I<--maximum> I<--current> I<--config> I<--live> -- 1.7.6

于 2011年07月16日 05:45, Eric Blake 写道:

On 07/15/2011 03:06 AM, Osier Yang wrote:

...

--- tools/virsh.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c index 8a62612..120f3c8 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2366,6 +2366,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) int autostart; unsigned int id; char *str, uuid[VIR_UUID_STRING_BUFLEN]; + int has_managed_state = 0;

if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -2430,6 +2431,13 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) autostart ? _("enable") : _("disable") ); }

+ has_managed_state = virDomainHasManagedSaveImage(dom, 0); + if (has_managed_state< 0) + vshPrint(ctl, "%-15s %s\n", _("Has managed state:"), _("unknown")); + else + vshPrint(ctl, "%-15s %s\n", _("Has managed state:"), + has_managed_state ? _("yes") : _("no")); This changes default output of an existing command, but it's not the first time we've done that (for example, commit aa2c9726 added the "Security model" string). For this particular command, that's fine, since it remains machine parsable (each line is a name-value pair, and you search for the line with the name you care about); still, I'd feel better if your new string came last rather than in the middle in case existing parsers count a certain number of lines and expect a particular name on that line.

Agree it's better as a last string, but it's need changes on codes to print "Security *" strings, to prevent the early return.

On 07/15/2011 03:06 AM, Osier Yang wrote:

--- tools/virsh.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)

Your rebuttals to my arguments about positioning are sound, so I'm okay with leaving the positioning of this output prior to "Security" strings. However, on re-reading it, I see one more issue:

diff --git a/tools/virsh.c b/tools/virsh.c index 8a62612..120f3c8 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2366,6 +2366,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) int autostart; unsigned int id; char *str, uuid[VIR_UUID_STRING_BUFLEN]; + int has_managed_state = 0;

if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -2430,6 +2431,13 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) autostart ? _("enable") : _("disable") ); }

+ has_managed_state = virDomainHasManagedSaveImage(dom, 0); + if (has_managed_state< 0) + vshPrint(ctl, "%-15s %s\n", _("Has managed state:"), _("unknown"));

Here, we print "managed state", but the command that creates that state is called "managedsave", and the API we call is virDomainHasManagedSaveImage. Not to mention that it surpasses 15 columns, which makes the output not lined up with all the other rows. Also, the "Has" is a bit redundant. I'd prefer to see this output as: Managed save: yes|no|unknown ACK with that nit fixed, and this can be pushed independently of the rest of the series. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

于 2011年07月19日 01:37, Eric Blake 写道:

On 07/18/2011 11:34 AM, Eric Blake wrote:

...

Here, we print "managed state", but the command that creates that state is called "managedsave", and the API we call is virDomainHasManagedSaveImage. Not to mention that it surpasses 15 columns, which makes the output not lined up with all the other rows. Also, the "Has" is a bit redundant. I'd prefer to see this output as:

Managed save: yes|no|unknown

ACK with that nit fixed, and this can be pushed independently of the rest of the series.

Also, squash in some documentation:

diff --git i/tools/virsh.pod w/tools/virsh.pod index 5afa1f2..312ddf0 100644 --- i/tools/virsh.pod +++ w/tools/virsh.pod @@ -514,6 +514,9 @@ Save and destroy (stop) a running domain, so it can be restarted from the same state at a later time. When the virsh B<start> command is next run for the domain, it will automatically be started from this saved state.

+The B<dominfo> command can be used to query whether a domain currently +has any managed save state. + =item B<managedsave-remove> I<domain-id>

Remove the B<managedsave> state file for a domain, if it exists. This

Pushed with the document, and the nits you mentioned in previous mail fixed. Thanks Osier