[libvirt] Allow to undefine a running domain
Per API virDomainUndefineFlags' doc says, if a domain is running, it will be converted to transient, but keep running. However, the drivers prohibit one undefining a running domain. This patch series modify the internal domainUndefineFlags function of all drivers (except some driver don't need to check if the domain is running, it's handled by the underlying hypervisor, such as ESX and XEND). The principle is: 1) Set vm->persistent = 0 for a running domain 2) domainDestroy and domainShutdown will take care of remove the domain obj from the hash table. [PATCH 1/8] libxl: Allow to undefine a running domain [PATCH 2/8] lxc: Allow to undefine a running domain [PATCH 3/8] openvz: Allow to undefine a running domain [PATCH 4/8] qemu: Allow to undefine a running domain [PATCH 5/8] test: Allow to undefine a running domain [PATCH 6/8] uml: Allow to undefine a running domain [PATCH 7/8] vmware: Allow to undefine a running domain [PATCH 8/8] xen: Allow to undefine a running domain (xm_internal) Split the patches for easy reviewing, will merge when committing if need. Regards Osier
On 08/19/2011 08:03 AM, Osier Yang wrote:
Per API virDomainUndefineFlags' doc says, if a domain is running, it will be converted to transient, but keep running. However, the drivers prohibit one undefining a running domain.
This patch series modify the internal domainUndefineFlags function of all drivers (except some driver don't need to check if the domain is running, it's handled by the underlying hypervisor, such as ESX and XEND).
The principle is:
1) Set vm->persistent = 0 for a running domain 2) domainDestroy and domainShutdown will take care of remove the domain obj from the hash table.
For hypervisors like qemu that already support transient domains, this is good. But I worry that for hypervisors like esx, which currently have no notion of a transient domain, this is wrong (that is, esx domains are always persistent, because the domain state is maintained by esx and libvirt merely queries esx which domains exist, running or otherwise; if esx doesn't support transient domains natively, then libvirt can't fake it). Thus, for domains where virDomainIsPersistent always returns true (such as esx), I think you need to keep the restriction that attempts to undefine a running domain are rejected, because the hypervisor lacks transient domain support.
[PATCH 1/8] libxl: Allow to undefine a running domain [PATCH 2/8] lxc: Allow to undefine a running domain [PATCH 3/8] openvz: Allow to undefine a running domain [PATCH 4/8] qemu: Allow to undefine a running domain [PATCH 5/8] test: Allow to undefine a running domain [PATCH 6/8] uml: Allow to undefine a running domain [PATCH 7/8] vmware: Allow to undefine a running domain [PATCH 8/8] xen: Allow to undefine a running domain (xm_internal)
Split the patches for easy reviewing, will merge when committing if need.
Nope, keep it separate; that way we can apply just the hypervisors that need it. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
于 2011年08月19日 21:39, Eric Blake 写道:
On 08/19/2011 08:03 AM, Osier Yang wrote:
Per API virDomainUndefineFlags' doc says, if a domain is running, it will be converted to transient, but keep running. However, the drivers prohibit one undefining a running domain.
This patch series modify the internal domainUndefineFlags function of all drivers (except some driver don't need to check if the domain is running, it's handled by the underlying hypervisor, such as ESX and XEND).
The principle is:
1) Set vm->persistent = 0 for a running domain 2) domainDestroy and domainShutdown will take care of remove the domain obj from the hash table.
For hypervisors like qemu that already support transient domains, this is good. But I worry that for hypervisors like esx, which currently have no notion of a transient domain, this is wrong (that is, esx domains are always persistent, because the domain state is maintained by esx and libvirt merely queries esx which domains exist, running or otherwise; if esx doesn't support transient domains natively, then libvirt can't fake it). Thus, for domains where virDomainIsPersistent always returns true (such as esx), I think you need to keep the restriction that attempts to undefine a running domain are rejected, because the hypervisor lacks transient domain support.
I don't modify ESX driver, it don't check if the domain is running or not.
[PATCH 1/8] libxl: Allow to undefine a running domain [PATCH 2/8] lxc: Allow to undefine a running domain [PATCH 3/8] openvz: Allow to undefine a running domain [PATCH 4/8] qemu: Allow to undefine a running domain [PATCH 5/8] test: Allow to undefine a running domain [PATCH 6/8] uml: Allow to undefine a running domain [PATCH 7/8] vmware: Allow to undefine a running domain [PATCH 8/8] xen: Allow to undefine a running domain (xm_internal)
Split the patches for easy reviewing, will merge when committing if need.
Nope, keep it separate; that way we can apply just the hypervisors that need it.
--- src/libxl/libxl_driver.c | 15 +++++++-------- 1 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 516148f..d6e0c28 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2802,12 +2802,6 @@ libxlDomainUndefineFlags(virDomainPtr dom, goto cleanup; } - if (virDomainObjIsActive(vm)) { - libxlError(VIR_ERR_OPERATION_INVALID, - "%s", _("cannot undefine active domain")); - goto cleanup; - } - if (!vm->persistent) { libxlError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot undefine transient domain")); @@ -2841,8 +2835,13 @@ libxlDomainUndefineFlags(virDomainPtr dom, event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_REMOVED); - 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
On 08/19/2011 08:03 AM, Osier Yang wrote:
--- src/libxl/libxl_driver.c | 15 +++++++-------- 1 files changed, 7 insertions(+), 8 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
于 2011年08月19日 22:03, Osier Yang 写道:
--- src/libxl/libxl_driver.c | 15 +++++++-------- 1 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 516148f..d6e0c28 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2802,12 +2802,6 @@ libxlDomainUndefineFlags(virDomainPtr dom, goto cleanup; }
- if (virDomainObjIsActive(vm)) { - libxlError(VIR_ERR_OPERATION_INVALID, - "%s", _("cannot undefine active domain")); - goto cleanup; - } - if (!vm->persistent) { libxlError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot undefine transient domain")); @@ -2841,8 +2835,13 @@ libxlDomainUndefineFlags(virDomainPtr dom, event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_REMOVED);
- virDomainRemoveInactive(&driver->domains, vm); - vm = NULL; + if (virDomainObjIsActive(vm)) { + vm->persistent = 0; + } else { + virDomainRemoveInactive(&driver->domains, vm); + vm = NULL; + } + ret = 0;
cleanup: Pushed the whole series, thanks for the quick reviewing, Eric.
Osier
--- src/lxc/lxc_driver.c | 15 +++++++-------- 1 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index bb560b6..a596945 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -479,12 +479,6 @@ static int lxcDomainUndefineFlags(virDomainPtr dom, goto cleanup; } - if (virDomainObjIsActive(vm)) { - lxcError(VIR_ERR_OPERATION_INVALID, - "%s", _("Cannot delete active domain")); - goto cleanup; - } - if (!vm->persistent) { lxcError(VIR_ERR_OPERATION_INVALID, "%s", _("Cannot undefine transient domain")); @@ -500,8 +494,13 @@ static int lxcDomainUndefineFlags(virDomainPtr dom, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_REMOVED); - 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
On 08/19/2011 08:03 AM, Osier Yang wrote:
--- src/lxc/lxc_driver.c | 15 +++++++-------- 1 files changed, 7 insertions(+), 8 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
--- 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
On 08/19/2011 08:03 AM, Osier Yang wrote:
--- src/openvz/openvz_driver.c | 15 +++++++-------- 1 files changed, 7 insertions(+), 8 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
--- src/qemu/qemu_driver.c | 22 +++++++++++++--------- 1 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 421a98e..81be950 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4720,12 +4720,6 @@ qemuDomainUndefineFlags(virDomainPtr dom, goto cleanup; } - if (virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cannot delete active domain")); - goto cleanup; - } - if (!vm->persistent) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot undefine transient domain")); @@ -4760,9 +4754,19 @@ qemuDomainUndefineFlags(virDomainPtr dom, VIR_DOMAIN_EVENT_UNDEFINED_REMOVED); VIR_INFO("Undefining domain '%s'", vm->def->name); - virDomainRemoveInactive(&driver->domains, - vm); - vm = NULL; + + /* If the domain is active, keep it running but set it as transient. + * domainDestroy and domainShutdown will take care of remove the + * domain obj from the hash table. + */ + if (virDomainObjIsActive(vm)) { + vm->persistent = 0; + } else { + virDomainRemoveInactive(&driver->domains, + vm); + vm = NULL; + } + ret = 0; cleanup: -- 1.7.6
On 08/19/2011 08:03 AM, Osier Yang wrote:
--- src/qemu/qemu_driver.c | 22 +++++++++++++--------- 1 files changed, 13 insertions(+), 9 deletions(-)
ACK with nit.
VIR_INFO("Undefining domain '%s'", vm->def->name); - virDomainRemoveInactive(&driver->domains, - vm); - vm = NULL; + + /* If the domain is active, keep it running but set it as transient. + * domainDestroy and domainShutdown will take care of remove the
s/remove/removing/ -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
于 2011年08月19日 21:48, Eric Blake 写道:
On 08/19/2011 08:03 AM, Osier Yang wrote:
--- src/qemu/qemu_driver.c | 22 +++++++++++++--------- 1 files changed, 13 insertions(+), 9 deletions(-)
ACK with nit.
VIR_INFO("Undefining domain '%s'", vm->def->name); - virDomainRemoveInactive(&driver->domains, - vm); - vm = NULL; + + /* If the domain is active, keep it running but set it as transient. + * domainDestroy and domainShutdown will take care of remove the
s/remove/removing/
Pushed with the nix fixed.
--- 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:
--- src/test/test_driver.c | 17 ++++++++--------- 1 files changed, 8 insertions(+), 9 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 08/19/2011 08:03 AM, Osier Yang wrote:
--- 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;
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 08/19/2011 08:12 AM, Eric Blake wrote:
- virDomainRemoveInactive(&privconn->domains, - privdom); - privdom = NULL; + if (virDomainObjIsActive(vm)) { + vm->persistent = 0;
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;
And even with that fixed, it now introduces a 'make check' failure, due to tests/undefine being hard-wired to the old semantics. I'll be fixing that test, and pushing something soon under the build-breaker rule. -- Eric Blake eblake@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
On 08/19/2011 08:58 AM, Eric Blake wrote:
# 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
Aargh - this points out a bug in virsh - we aren't calling fflush() in between writes to stdout and stderr. If those two FILE* are mapped to the same underlying file description (as is the case when you do 2>&1 redirection from the shell), then it is essential to flush in between every transition between the two FILE* if you want the output to appear in linear order. Without patching virsh, I had to split this into capturing output in two files, with no sense of relationship between timings, because my test run happened to flush stderr prior to stdout and reverse the output from the expected linear ordering of when it was produced. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
--- 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
On 08/19/2011 08:03 AM, Osier Yang wrote:
--- src/uml/uml_driver.c | 16 +++++++--------- 1 files changed, 7 insertions(+), 9 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
--- 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
On 08/19/2011 08:03 AM, Osier Yang wrote:
--- src/vmware/vmware_driver.c | 15 +++++++-------- 1 files changed, 7 insertions(+), 8 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
--- 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:
--- src/xen/xm_internal.c | 18 ++++++++++-------- 1 files changed, 10 insertions(+), 8 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 08/19/2011 08:03 AM, Osier Yang wrote:
--- src/xen/xm_internal.c | 18 ++++++++++-------- 1 files changed, 10 insertions(+), 8 deletions(-)
Aargh, again I spoke too soon.
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;
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Osier Yang