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