We always had to deal with new parsing errors in a weird way. All
of
them needed to go into functions starting the domains. That messes up
the code, it's confusing to newcomers and so on.
I once had an idea that we can handle this stuff. We know what
failed, we have the XML that failed parsing and if the problem is not
in the domain name nor UUID, we can create a domain object out of that
as well. This way we are able to do something we weren't with this
series applied. Example follows.
Assume "dummy" is a domain with invalid XML (I just modified the
file). Now, just for the purpose of this silly example, let's say we
allowed domains with archtecture x86_*, but now we've realized that
x86_64 is the only one we want to allow, but there already is a domain
defined that has <type arch='x86_256' .../>. With the current master,
the domain would be lost, so we would need to modify the funstion
starting the domain (e.g. qemuProcessStart for qemu domain). However,
with this series, it behaves like this:
# virsh list --all --reason
Id Name State Reason
---------------------------------------------------------------
- dummy shut off invalid XML
# virsh domstate --reason dummy
shut off (invalid XML)
# virsh start dummy
error: Failed to start domain dummy
error: XML error: domain 'dummy' was not loaded due to an XML error
(unsupported configuration: Unknown architecture x86_256), please
redefine it
# VISUAL='sed -i s/x86_256/x86_64/' virsh edit dummy
Domain dummy XML configuration edited.
# virsh domstate --reason dummy
shut off (unknown)
# virsh start dummy
Domain dummy started
This is a logical next step for us to clean and separate parsing and
starting, getting rid of some old code without sacrifying compatibility
and maybe generating parser code in the future.
v2:
- rebased on top of current master (with virdomainobjlist.c)
- only disallow starting domains with invalid definitions (change
done in v1), but allow re-defining them
- add support for "virsh list --reason" to better excercise the
feature added in this patchset
v1:
- rebase
- don't allow starting domains with invalid state
-
https://www.redhat.com/archives/libvir-list/2015-November/msg01127.html
RFC:
https://www.redhat.com/archives/libvir-list/2015-September/msg00698.html
Martin Kletzander (10):
conf, virsh: Add new domain shutoff reason
virsh: Refactor the table output of the list command
virsh: Add support for list -reason
qemu: Few whitespace cleanups
conf: Extract name-parsing into its own function
conf: Extract UUID parsing into its own function
conf: Optionally keep domains with invalid XML, but don't allow
starting them
qemu: Don't lookup invalid domains unless specified otherwise
qemu: Prepare basic APIs to handle invalid defs
qemu: Load domains with invalid XML on start
include/libvirt/libvirt-domain.h | 2 +
src/bhyve/bhyve_driver.c | 2 +
src/conf/domain_conf.c | 121 +++++++++++++++++++++++++++++++--------
src/conf/domain_conf.h | 7 +++
src/conf/virdomainobjlist.c | 71 +++++++++++++++++++++--
src/conf/virdomainobjlist.h | 1 +
src/libvirt_private.syms | 1 +
src/libxl/libxl_driver.c | 3 +
src/lxc/lxc_driver.c | 3 +
src/qemu/qemu_driver.c | 64 +++++++++++++++++----
src/uml/uml_driver.c | 2 +
tests/virshtest.c | 30 +++++++---
tools/virsh-domain-monitor.c | 60 ++++++++++++-------
tools/virsh.pod | 5 +-
14 files changed, 303 insertions(+), 69 deletions(-)
--
2.6.3
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Beyond the few noted spots changes look good to me. Implicit ACK for
those not specifically noted.
John