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

lhuang lhuang at redhat.com
Wed Dec 2 08:46:20 UTC 2015



On 12/02/2015 01:35 AM, 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(-)
>
>

I am glad to see these patches you told me before.

And i have test your patches and found some problem until now:

1. if guest xml have private info, libvirt will output it if xml is invalid:

# virsh list --all --reason
  Id    Name                           State      Reason
------------------------------------------------------------

  -     test3                          shut off   invalid XML

# virsh -r dumpxml test3
...
     <input type='keyboard' bus='ps3'/>   <----wrong place
     <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' 
passwd='123'>  <----show passwd

2. vcpucount command output looks strange (small problem :) ):

# virsh vcpucount test3


3. hit crash if the guest xml uuid is invalid (delete a number) during 
stop daemon:


Program terminated with signal 11, Segmentation fault.
#0  0x00007fcbe773af87 in virDomainDefFree (def=0x7fcbc423d980) at 
conf/domain_conf.c:2327
2327            virDomainGraphicsDefFree(def->graphics[i]);

Thread 1 (Thread 0x7fcbe835c8c0 (LWP 21589)):
#0  0x00007fcbe773af87 in virDomainDefFree (def=0x7fcbc423d980) at 
conf/domain_conf.c:2327
#1  0x00007fcbe773b903 in virDomainObjDispose (obj=0x7fcbc4235fe0) at 
conf/domain_conf.c:2487
#2  0x00007fcbe76f182b in virObjectUnref (anyobj=<optimized out>) at 
util/virobject.c:265
#3  0x00007fcbe76d0ac9 in virHashFree (table=0x7fcbc412eda0) at 
util/virhash.c:318
#4  0x00007fcbe76f182b in virObjectUnref (anyobj=<optimized out>) at 
util/virobject.c:265
#5  0x00007fcbce85f8cf in qemuStateCleanup () at qemu/qemu_driver.c:1126
#6  0x00007fcbe779a168 in virStateCleanup () at libvirt.c:815
#7  0x00007fcbe83f02b1 in main (argc=<optimized out>, argv=<optimized 
out>) at libvirtd.c:1619

Thanks,
Luyao




More information about the libvir-list mailing list