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

[Marc Hartmayer]

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.

> 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/ :|
>
-- 
Beste Grüße / Kind regards
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294