Re: [RFC PATCH 7/8] qemu: Prepare basic APIs to handle invalid defs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 22, 2015 at 02:17:14PM +0100, Daniel P. Berrange wrote:
On Tue, Sep 22, 2015 at 03:12:14PM +0200, Martin Kletzander wrote:
On Tue, Sep 22, 2015 at 01:48:01PM +0100, Daniel P. Berrange wrote:
>On Tue, Sep 22, 2015 at 02:15:53PM +0200, Martin Kletzander wrote:
>>In order for the user to be able to fix broken domains function
>>qemuDomainGetXMLDesc() needs to be able to lookup invalid domain
>>definitions and handle them properly.  When redefined, function
>>qemuDomainDefineXMLFlags() must clear the 'invalid XML' reason.  As a
>>nice addition, qemuDomainGetState() can lookup such domains without any
>>other change and that allows virsh not only to get their status, but
>>also to list them.
>
>Hmm, that's an interesting approach to the problem. I wonder though
>if we could do things slightly differently such that we don't need
>to change so many APIs.
>
>eg, just have a 'bool error' field in virDomainDefPtr. When loading
>the XML fails, populate a virDomainObjPtr/DefPtr as normal, but set
>the error field. Now we merely need to change the qemuDomainStart
>method, so it refuses to launch a VM with the 'error' flag set. All
>the other APIs could be essentially unchanged. Sure it would not
>be useful to allow things like virDomainAttachDevice, etc on such
>broken domains, but for sake of simplicity we can avoid touching
>all the methods except start.
>

To be honest, I'm a afraid that we might forget some API that needs to
be blocked as well.  And we would have to go through all the APIs just
to see whether it accesses something that might be missing.  Moreover,
how would you decide what to skip at each error during parsing?  If,
for example, the <numatune> has some faulty attribute in one
subelement should we skip all the elements or just the one or just
skip that one particular attribute?  We would also not format the
faulty attribute to the XML being dumped, so the user wouldn't see
what's missing, which is even worse when there are two problems and
they fix only one, so we skip the other one.

Oh, I wasn't suggesting changing the parser. I just mean that we would
create a virDomainDefPtr instance which only contains the name and
UUID, and ID field (if the guest is running from domain status XML).

We'd leave all the rest of the config blank - our code ought to be
able to deal with that, since essentially all config is optional
aside from the name/uuid anyway.


Then probably I misunderstood because that's exactly what this series
is doing.

I thought about your approach as well, but that would require complete
rewrite of the whole parsing code (maybe changing it to generated one
based on RNG schema and some minor additional info) and it would touch
way more APIs.  The approach I'm suggesting here keeps almost all APIs
untouched and it doesn't have an effect on anything, unless explicitly
specified.

Check last two patches to see how much qemu driver needs to be
changed, for others it will be similar, probably a little bit less..
The patches are 17+/9- when combined.  I don't think you can do much
better than that.  And that doesn't show the improvements that can be
made in the starting and parsing code after this series is applied.

Regards,
Daniel
--
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]