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

Martin Kletzander mkletzan at redhat.com
Tue Sep 22 13:26:26 UTC 2015


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 :|
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150922/999d4145/attachment-0001.sig>


More information about the libvir-list mailing list