Re: [libvirt] [PATCH 02/12] Introduce new domain create API virDomainCreateWithParams

On Wed, May 16, 2018 at 06:21:40PM +0200, Christian Borntraeger wrote: > > > On 05/16/2018 05:35 PM, Daniel P. Berrangé wrote: > > On Wed, May 16, 2018 at 05:30:33PM +0200, Marc Hartmayer wrote: > >> On Wed, May 09, 2018 at 05:40 PM +0200, "Daniel P. Berrangé" <berrange(a)redhat.com&gt; wrote: > >>> On Wed, May 09, 2018 at 04:56:12PM +0200, Marc Hartmayer wrote: > >>>> Introduce new libvirt API virDomainCreateWithParams that allows to > >>>> temporarily boot from another boot device, to use another kernel, > >>>> initrd, and cmdline than defined in the persistent domain > >>>> definition. All typed parameters are optional. > >>>> > >>>> The design of the API was chosen to ease future extensions. > >>> > >>> I don't really see the point in doing this. We already have the ability > >>> to temporary boot with a different configuration than is stored in > >>> the persistent XML. Just call virDomainCreateXML() passing in the > >>> alternative XML doc. This allows changing *any* aspect of the guest > >>> configuration, so we're not restricted to just bot device, kernel > >>> initrd and cmdline, and thus won't need to write more code anytime > >>> someone asks to be able to override something else too. > >> > >> I know there is the API virDomainCreateXML for creating a transient > >> domain and that it’s possible to temporarily replace parts of the > >> persistent XML with it. But my idea is _not_ to add a functionality to > >> override parts of the persistent XML. My idea is to provide support > >> allowing an easy one-time switch of the boot device in a persistently > >> defined domain. For s390 it’s essential to have an easy way to change > >> the boot configuration since it “knows” only one boot device at a time > >> and it has no support for interactively changing the boot device during > >> the boot/IPL process. > > > > virDomainCreateXML does not change the persistent XML. If you have > > an existing persistent guest, and use virDomainCreateXML it lets you > > start that guest with that customized XML just once, without affecting > > the persistent XML at all. > > > >> I started out with a fixed API (as it was done for example with > >> virDomanCreateWithFiles) but than I liked the approach of providing an > >> more extensible and flexible API better. This is also the reason why I > >> used typed parameters for passing the arguments. This type of API is > >> also not restricted to boot order changes since it could be freely > >> expanded (e.g. passing file descriptors). I can certainly revert back to > >> the static API. > > > > Using virDomainCreateXML is the most flexible API because it lets you > > changed anything in the XML for just that one boot up attempt. > > To clarify this: > would it be ok for you to implement this feature (the command line parameter > to "virsh start") in virsh by doing the xml mangling in the virsh command line > tool itself? I'm not really a fan of that, as the modifications to virsh are just as non-extensible as the modifications to the API. Each time someone wants another field customizable, we have to add new virsh CLI args and XML munging for them. I could suggest a general arg 'virsh start --edit $GUESTNAME' though, which takes the current persistent XML, launches it in an editor, allowing the user to change the boot order and then starts the guest from this temporary editted XML using virDomainCreateXML. This is a conceptual fit with the 'virsh edit $GUESTNAME' command we already have