On Thu, May 24, 2012 at 05:46:51AM -0600, Eric Blake wrote:
> On 05/20/2012 09:56 AM, Peter Krempa wrote:
> > This patch adds a basic implementation of the listing code for
> > virConnectListAllDomains() to qemu driver. The listing code does
> > not support any filtering flags yet, but they may be easily added
> > later.
> > ---
> > src/qemu/qemu_driver.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 files changed, 97 insertions(+), 0 deletions(-)
>
> > +static void
> > +qemuPopulateDomainList(void *payload,
> > + const void *name ATTRIBUTE_UNUSED,
> > + void *opaque)
> > +{
> > + struct virDomainListData *data = opaque;
> > + virDomainObjPtr vm = payload;
> > +
> > + if (data->error ||
> > + (data->limit >= 0 &&
> > + data->ndomains >= data->limit))
> > + return;
> > +
> > + if (!data->populate) {
> > + data->ndomains++;
> > + return;
> > + }
> > +
> > + virDomainObjLock(vm);
>
> I just realized: Since we are executing this under the driver lock, and
> no VM can change state until we let go of the driver lock, it is not
> necessary to lock vms in this loop. That will help things go faster in
> computing the list.
Hmm, this feels slightly dangerous to me. Saying that is in effect saying
you can do reads on a virDomainObjPtr without locking. This would be ok
if it were impossible for the fields we're reading to be changed without
driver lock held.
1. Thread A lock(driver)
2. Thread A lock(vm1)
3. Thread A unlock(driver)
4. Thread B lock(driver)
5. Thread B ...starts getting the list of domains...
Now consider Thread A changes the 'id' feld in a virDomainPtr eg due
to the guest shutting down.
Won't thread B be doing unsafe reads of 'id' now ?
The only way I can see that this is safe, is if we are sure that
every change of the 'name', 'uuid' and 'id' fields is protected
by the driver lock.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list