于 2011年08月19日 22:03, Osier Yang 写道: Pushed the whole series, thanks for the quick reviewing, Eric. Osier

--- src/openvz/openvz_driver.c | 15 +++++++-------- 1 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index b9dc712..69ff444 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1132,19 +1132,18 @@ openvzDomainUndefineFlags(virDomainPtr dom, if (openvzGetVEStatus(vm, &status, NULL) == -1) goto cleanup; - if (status != VIR_DOMAIN_SHUTOFF) { - openvzError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot delete active domain")); - goto cleanup; - } - openvzSetProgramSentinal(prog, vm->def->name); if (virRun(prog, NULL) < 0) { goto cleanup; } - virDomainRemoveInactive(&driver->domains, vm); - vm = NULL; + if (virDomainObjIsActive(vm)) { + vm->persistent = 0; + } else { + virDomainRemoveInactive(&driver->domains, vm); + vm = NULL; + } + ret = 0; cleanup: -- 1.7.6

--- src/test/test_driver.c | 17 ++++++++--------- 1 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index fb14b10..422486e 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2601,18 +2601,17 @@ static int testDomainUndefineFlags(virDomainPtr domain, goto cleanup; } - if (virDomainObjGetState(privdom, NULL) != VIR_DOMAIN_SHUTOFF) { - testError(VIR_ERR_INTERNAL_ERROR, - _("Domain '%s' is still running"), domain->name); - goto cleanup; - } - event = virDomainEventNewFromObj(privdom, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_REMOVED); - virDomainRemoveInactive(&privconn->domains, - privdom); - privdom = NULL; + if (virDomainObjIsActive(vm)) { + vm->persistent = 0; + } else { + virDomainRemoveInactive(&privconn->domains, + privdom); + privdom = NULL; + } + ret = 0; cleanup: -- 1.7.6

On 08/19/2011 08:03 AM, Osier Yang wrote: Spoke too soon. This doesn't compile. I'll be pushing this. diff --git i/src/test/test_driver.c w/src/test/test_driver.c index 422486e..ec2bd75 100644 --- i/src/test/test_driver.c +++ w/src/test/test_driver.c @@ -2604,8 +2604,8 @@ static int testDomainUndefineFlags(virDomainPtr domain, event = virDomainEventNewFromObj(privdom, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_REMOVED); - if (virDomainObjIsActive(vm)) { - vm->persistent = 0; + if (virDomainObjIsActive(privdom)) { + privdom->persistent = 0; } else { virDomainRemoveInactive(&privconn->domains, privdom); -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Test failure exposed in commit 7d3390f. * tests/undefine: Fix to match updated test driver semantics. --- Pushing under the build-breaker rule. tests/undefine | 42 ++++++++++++++++++++++++++++-------------- 1 files changed, 28 insertions(+), 14 deletions(-) diff --git a/tests/undefine b/tests/undefine index 5c39e27..6c821ad 100755 --- a/tests/undefine +++ b/tests/undefine @@ -29,32 +29,46 @@ fi fail=0 -# Attempt to undefine a running domain, by domain name. -# Although the API allows this, the test hypervisor does not. -$abs_top_builddir/tools/virsh -q -c test:///default undefine test > out 2>&1 -test $? = 1 || fail=1 +# Attempt to undefine a running domain, by domain name. Every time a new +# connection is opened to the test driver, it starts life with a new +# persistent running domain named 'test' with a different uuid, so +# testing this command requires batch mode use of virsh. +$abs_top_builddir/tools/virsh -q -c test:///default \ + 'dominfo test; undefine test; dominfo test' > out1 2>&1 +test $? = 0 || fail=1 +sed '/^Persistent/n; /:/d' < out1 > out cat <<\EOF > exp || fail=1 -error: Failed to undefine domain test -error: internal error Domain 'test' is still running +Persistent: yes +Domain test has been undefined +Persistent: no EOF compare exp out || fail=1 # A similar diagnostic when specifying a domain ID -$abs_top_builddir/tools/virsh -q -c test:///default undefine 1 > out 2>&1 -test $? = 1 || fail=1 +$abs_top_builddir/tools/virsh -q -c test:///default \ + 'dominfo 1; undefine 1; dominfo 1' > out1 2>&1 +test $? = 0 || fail=1 +sed '/^Persistent/n; /:/d' < out1 > out cat <<\EOF > exp || fail=1 -error: Failed to undefine domain 1 -error: internal error Domain 'test' is still running +Persistent: yes +Domain 1 has been undefined +Persistent: no EOF compare exp out || fail=1 # Succeed, now: first shut down, then undefine, both via name. -$abs_top_builddir/tools/virsh -q -c test:///default 'shutdown test; undefine test' > out 2>&1 -test $? = 0 || fail=1 -cat <<\EOF > exp || fail=1 +$abs_top_builddir/tools/virsh -q -c test:///default \ + 'shutdown test; undefine test; dominfo test' > out 2> err +test $? = 1 || fail=1 +cat <<\EOF > expout || fail=1 Domain test is being shutdown Domain test has been undefined EOF -compare exp out || fail=1 +cat <<\EOF > experr || fail=1 +error: failed to get domain 'test' +error: Domain not found +EOF +compare expout out || fail=1 +compare experr err || fail=1 (exit $fail); exit $fail -- 1.7.4.4

--- src/uml/uml_driver.c | 16 +++++++--------- 1 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index a9cb7ef..19b6c55 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1806,12 +1806,6 @@ static int umlDomainUndefineFlags(virDomainPtr dom, goto cleanup; } - if (virDomainObjIsActive(vm)) { - umlReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot delete active domain")); - goto cleanup; - } - if (!vm->persistent) { umlReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot undefine transient domain")); @@ -1821,9 +1815,13 @@ static int umlDomainUndefineFlags(virDomainPtr dom, if (virDomainDeleteConfig(driver->configDir, driver->autostartDir, vm) < 0) goto cleanup; - virDomainRemoveInactive(&driver->domains, - vm); - vm = NULL; + if (virDomainObjIsActive(vm)) { + vm->persistent = 0; + } else { + virDomainRemoveInactive(&driver->domains, vm); + vm = NULL; + } + ret = 0; cleanup: -- 1.7.6

--- src/vmware/vmware_driver.c | 15 +++++++-------- 1 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 5c63239..b2cfdce 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -639,20 +639,19 @@ vmwareDomainUndefineFlags(virDomainPtr dom, goto cleanup; } - if (virDomainObjIsActive(vm)) { - vmwareError(VIR_ERR_OPERATION_INVALID, - "%s", _("cannot undefine active domain")); - goto cleanup; - } - if (!vm->persistent) { vmwareError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot undefine transient domain")); goto cleanup; } - virDomainRemoveInactive(&driver->domains, vm); - vm = NULL; + if (virDomainObjIsActive(vm)) { + vm->persistent = 0; + } else { + virDomainRemoveInactive(&driver->domains, vm); + vm = NULL; + } + ret = 0; cleanup: -- 1.7.6

--- src/xen/xm_internal.c | 18 ++++++++++-------- 1 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 95387c9..5c8a017 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -1218,8 +1218,6 @@ int xenXMDomainUndefine(virDomainPtr domain) { return (-1); } - if (domain->id != -1) - return (-1); if (domain->conn->flags & VIR_CONNECT_RO) return (-1); @@ -1235,13 +1233,17 @@ int xenXMDomainUndefine(virDomainPtr domain) { if (unlink(entry->filename) < 0) goto cleanup; - /* Remove the name -> filename mapping */ - if (virHashRemoveEntry(priv->nameConfigMap, domain->name) < 0) - goto cleanup; + if (virDomainObjIsActive(vm)) { + vm->persistent = 0; + } else { + /* Remove the name -> filename mapping */ + if (virHashRemoveEntry(priv->nameConfigMap, domain->name) < 0) + goto cleanup; - /* Remove the config record itself */ - if (virHashRemoveEntry(priv->configCache, entry->filename) < 0) - goto cleanup; + /* Remove the config record itself */ + if (virHashRemoveEntry(priv->configCache, entry->filename) < 0) + goto cleanup; + } ret = 0; -- 1.7.6

On 08/19/2011 08:03 AM, Osier Yang wrote: Aargh, again I spoke too soon. There is no vm in scope. Furthermore, xm_internal is used solely for management of inactive domains - see this code in xen_driver.c for xenUnifiedDomainIsPersistent: if (priv->opened[XEN_UNIFIED_XM_OFFSET]) { /* Old Xen, pre-inactive domain management. * If the XM driver can see the guest, it is definitely persistent */ currdom = xenXMDomainLookupByUUID(dom->conn, dom->uuid); if (currdom) ret = 1; else ret = 0; } else { /* New Xen with inactive domain management */ if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) { currdom = xenDaemonLookupByUUID(dom->conn, dom->uuid); if (currdom) { if (currdom->id == -1) { /* If its inactive, then trivially, it must be persistent */ ret = 1; } else { char *path; char uuidstr[VIR_UUID_STRING_BUFLEN]; /* If its running there's no official way to tell, so we * go behind xend's back & look at the config dir */ ... I think we need to revert this patch, and instead fix things so that xm_internal fails on running domains (that is, if the domain is running, the xm driver has nothing to do with it, but by returning failure, the unified driver then knows to fall back to the xend driver), and it is the xend driver that should be managing a transition from running persistent to transient. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org