[libvirt] [PATCH 2/2] qemu: Validate the domain after marking the current domain as transient

[Daniel P. Berrange]

On Mon, Feb 27, 2017 at 10:59:39AM +0100, Marc Hartmayer wrote:
> On Thu, Feb 23, 2017 at 05:36 PM +0100, "Daniel P. Berrange" <berrange at redhat.com> wrote:
> > On Thu, Feb 23, 2017 at 05:22:44PM +0100, Marc Hartmayer wrote:
> >> On Thu, Feb 23, 2017 at 03:33 PM +0100, Michal Privoznik <mprivozn at redhat.com> wrote:
> >> > On 02/23/2017 10:44 AM, Marc Hartmayer wrote:
> >> >> Validate the domain that actually will be started. It's semantically
> >> >> more clear and also it can detect failures that may have happened in
> >> >> virDomainObjSetDefTransient().
> >> >>
> >> >> Signed-off-by: Marc Hartmayer <mhartmay at linux.vnet.ibm.com>
> >> >> Reviewed-by: Bjoern Walk <bwalk at linux.vnet.ibm.com>
> >> >> Reviewed-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
> >> >> ---
> >> >>  src/qemu/qemu_process.c | 6 +++---
> >> >>  1 file changed, 3 insertions(+), 3 deletions(-)
> >> >>
> >> >> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> >> >> index a57d136..bd3a8b8 100644
> >> >> --- a/src/qemu/qemu_process.c
> >> >> +++ b/src/qemu/qemu_process.c
> >> >> @@ -4746,9 +4746,6 @@ qemuProcessInit(virQEMUDriverPtr driver,
> >> >>                                                        vm->def->os.machine)))
> >> >>          goto cleanup;
> >> >>
> >> >> -    if (qemuProcessStartValidate(driver, vm, priv->qemuCaps, caps, flags) < 0)
> >> >> -        goto cleanup;
> >> >> -
> >> >>      /* Do this upfront, so any part of the startup process can add
> >> >>       * runtime state to vm->def that won't be persisted. This let's us
> >> >>       * report implicit runtime defaults in the XML, like vnc listen/socket
> >> >> @@ -4757,6 +4754,9 @@ qemuProcessInit(virQEMUDriverPtr driver,
> >> >>      if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm) < 0)
> >> >>          goto cleanup;
> >> >>
> >> >> +    if (qemuProcessStartValidate(driver, vm, priv->qemuCaps, caps, flags) < 0)
> >> >> +        goto cleanup;
> >> >> +
> >> >
> >> > This needs to be goto stop for the reasons described in the previous e-mail.
> >> >
> >> >>      if (flags & VIR_QEMU_PROCESS_START_PRETEND) {
> >> >>          if (qemuDomainSetPrivatePaths(driver, vm) < 0)
> >> >>              goto cleanup;
> >> >>
> >> >
> >> > Honestly, I like what we have now better. I mean, SetDefTransient() is
> >> > very unlikely to fail. It's just doing a copy of domain definition (in a
> >> > very stupid way, but lets save that for a different discussion).
> >> > Basically, it will fail on OOM only (which you will not get on a Linux
> >> > system, unless you really try).
> >> 
> >> It's semantically more clear (at least for me) and for example it
> >> enables us to change some parts of the transient domain before
> >> validation (affect the transient domain only, not the persistent).
> >
> > What are you planning to change in the config before validation ?
> >
> 
> I'm implementing a feature which allows to select the boot device at
> domain start time (something like 'virsh start --with-bootdevice sda
> domain1'). The main reason why we want this is because the s390
> architecture boots only from the first device specified in the boot
> order.

There's no need to changes in the QEMU driver todo this. You can just
query the current XML config, change the boot device in it, and then
run virDomainCreateXML to launch the guest with the changed config,
or virDomainDefineXML again to make the changed boot device permanent.


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