[libvirt] [PATCH 08/10] qemu: Handle genid processing during startup/launch

[Daniel P. Berrangé]

On Thu, Apr 12, 2018 at 07:54:57AM -0400, John Ferlan wrote:
> 
> 
> On 04/12/2018 07:13 AM, Daniel P. Berrangé wrote:
> > On Wed, Apr 11, 2018 at 04:10:03PM -0400, John Ferlan wrote:
> >> Before we generate the command line for qemu, if the domain about to
> >> be launched desires to utilize the VM Generation ID functionality, then
> >> handle both the regenerating the GUID value for backup recovery (restore
> >> operation) and the startup after snapshot as well as checking that the
> >> genid value that's about to be placed on the command line doesn't
> >> duplicate some other already running domain.
> >>
> >> Signed-off-by: John Ferlan <jferlan at redhat.com>
> >> ---
> >>  src/qemu/qemu_driver.c  |  22 +++++++---
> >>  src/qemu/qemu_process.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++-
> >>  src/qemu/qemu_process.h |   1 +
> >>  3 files changed, 126 insertions(+), 7 deletions(-)
> > 
> > 
> >> +    /* Now let's make sure the genid this domain has is not duplicitous
> >> +     * with something else running. */
> >> +    if (virDomainObjListForEach(driver->domains, qemuProcessCheckGenID, vm) < 0)
> >> +        return -1;
> > 
> > Is this really required  ?   AFAIK, the gen ID merely has to be unique within
> > the scope of the installed guest disk image, not amongst all VMs that exist.
> > If it was the latter, the check we're doing is pretty weak because it only
> > considers one host, not VMs on every other host.   IMHO this check is not
> > desirable because it adds mutex contention against every guest, into the
> > start up path and other code paths too.
> > 
> 
> Well it wasn't 100% clear from what I read whether it would be required.
> I may have misread something along the way too with how/why the value
> was being used with Active Directory and perhaps a cloned domain using
> (or needing) some mechanism to distinguish between the two.
> 
> Still, in rereading the MS GenID paper again uniqueness between two
> domains doesn't seem to be required (which is fine by me - I didn't like
> the code either) since the AD issues seem to be time based to rolling
> back using a snapshot.

In practice, unless someone uses tragically bad  UUID generator that
produces clashing strings, they're all going to be unique anyway. So
I think we're safe to drop the check and blame any mistakes on the mgmt
app using libvirt, if they occurr.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|