[libvirt] [PATCH v2 00/10] Make loading domains with invalid XML possible

[John Ferlan]

On 12/01/2015 12:35 PM, Martin Kletzander wrote:
> 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 at 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