[libvirt PATCH] qemu: Fix domain ID allocation

[Daniel P. Berrangé]

On Fri, Jan 31, 2020 at 04:33:50PM +0100, Michal Privoznik wrote:
> On 1/31/20 3:43 PM, Ján Tomko wrote:
> > The rewrite to use GLib's atomic ops functions changed the behavior
> > of virAtomicIntInc - before it returned the pre-increment value.
> > 
> > Most of the callers using its value were adjusted, but the one
> > in qemuDriverAllocateID was not. If libvirtd would reconnect to
> > a running domain during startup, the next started domain would get
> > the same ID:
> > 
> > $ virsh list
> >   Id   Name       State
> > --------------------------
> >   1    f28live    running
> >   1    f28live1   running
> > 
> > Use the g_atomic_add function directly (as recommended in viratomic.h)
> > and add 1 to the result.
> > 
> > This also restores the usual numbering from 1 instead of 0.
> > 
> > Signed-off-by: Ján Tomko <jtomko at redhat.com>
> > Fixes: 7b9645a7d127a374b8d1c83fdf9789706dbab2c9
> > ---
> >   src/qemu/qemu_conf.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> > index e5051027fc..0b119cbe78 100644
> > --- a/src/qemu/qemu_conf.c
> > +++ b/src/qemu/qemu_conf.c
> > @@ -1858,7 +1858,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
> >   int qemuDriverAllocateID(virQEMUDriverPtr driver)
> >   {
> > -    return virAtomicIntInc(&driver->lastvmid);
> > +    return g_atomic_int_add(&driver->lastvmid, 1) + 1;
> >   }
> > 
> 
> Does it makes sense to replace all virAtomic with g_atomic or do we still
> pretend that we care about client library and we don't crash on OOM in it?

We explicitly abort-on-OOM *everywhere*, the moment we started using
g_new for VIR_ALLOC, and that includs the client library.


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