[libvirt] [PATCH v2 00/10] Make loading domains with invalid XML possible
lhuang
lhuang at redhat.com
Wed Dec 2 09:48:17 UTC 2015
On 12/02/2015 05:13 PM, Martin Kletzander wrote:
> On Wed, Dec 02, 2015 at 04:46:20PM +0800, lhuang wrote:
>>
>>
>> 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
>
> It should be in the same place as it was saved on the disk.
oh, seems you misunderstood, i mean there is the invalid place which
make xml invalid :)
>
>> <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'
>> passwd='123'> <----show passwd
>>
>
> Oh, that's definitely what's not suppose to happen, but since we cannot
> parse it, I will just disable the dumpxml for read-only connections.
>
okay, sounds good.
>> 2. vcpucount command output looks strange (small problem :) ):
>>
>> # virsh vcpucount test3
>>
>
> Oh, so my guess was that this happens because virsh calls dumpxml, then
> searches for the info. But I checked and it calls an API that should be
> blocked. I'm guessing that happened because of the uuid parsing has
> gone wrong (below). Anyway, I mentioned the first idea because that can
> happen with something else and hence the only way how to properly
> prevent that, is to use different API for invalid domains. I'll try
> working on that, but this is starting to be bigger and bigger overkill,
> unfortunately.
>
I checked the code and found virshCPUCountCollect will return -1:
if (checkState &&
((flags & VIR_DOMAIN_AFFECT_LIVE && virDomainIsActive(dom) < 1) ||
(flags & VIR_DOMAIN_AFFECT_CONFIG &&
virDomainIsPersistent(dom) < 1)))
return -1;
and PRINT_COUNT will skip which var <= 0, so command return success and
no output.
> Anyway, thanks a lot for testing that and letting me know.
>
You're welcome, i will reply this mail if i can find more problem.
Luyao
>>
>> 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
>>
>> --
>> libvir-list mailing list
>> libvir-list at redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
>
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151202/70325048/attachment-0001.htm>
More information about the libvir-list
mailing list