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

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

 



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@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
> >> Reviewed-by: Bjoern Walk <bwalk@xxxxxxxxxxxxxxxxxx>
> >> Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx>
> >> ---
> >>  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 ?

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/ :|

--
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]
  Powered by Linux