11:17 a.m.
* src/libvirt.c (virConnectClose): Mention reference count return.
Reported by Michal Novotny, analyzed by Matthias Bolte.
---
src/libvirt.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index c57e0c3..b9765b1 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1295,7 +1295,12 @@ error:
* especially if there is running domain which need further monitoring by
* the application.
*
- * Returns 0 in case of success or -1 in case of error.
+ * Connections are reference counted; the connection is not actually
+ * closed until all calls to virConnectRef have had a matching
+ * virConnectClose call.
+ *
+ * Returns the number of remaining references (positive if the connection
+ * remains open, 0 if closed) in case of success or -1 in case of error.
*/
int
virConnectClose(virConnectPtr conn)
--
1.7.4.4
11:43 a.m.
2011/6/22 Eric Blake <eblake(a)redhat.com>:
* src/libvirt.c (virConnectClose): Mention reference count return.
Reported by Michal Novotny, analyzed by Matthias Bolte.
---
src/libvirt.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index c57e0c3..b9765b1 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1295,7 +1295,12 @@ error:
* especially if there is running domain which need further monitoring by
* the application.
*
- * Returns 0 in case of success or -1 in case of error.
+ * Connections are reference counted; the connection is not actually
+ * closed until all calls to virConnectRef have had a matching
+ * virConnectClose call.
Actually you need to match the virConnectOpen* call too. Interpreted
strictly it says that you only need to match the virConnectRef calls
which means that you don't have to call it when you didn't call
virConnectRef for a connection, doesn't it? Maybe just adding
appending a "too" to the sentence fixes this.
The reference counting applies to vir(Domain|Network|...)Free too. But
this comment is missing that domain, network, etc objects that depend
on the connection take a reference on it too. So it's more complicated
than the comment implies. I'm not sure if we should explain this here.
virConnectClose returning > 0 even when the virConnectOpen* and
virConnectRef calls are matched with virConnectClose calls might
confuse an application developer and giving him the impression that
libvirt doesn't behave as expected or having a bug. This can happen
when you try to close the connection before freeing the remaining
objects depending on it.
+ * Returns the number of remaining references (positive if the
connection
+ * remains open, 0 if closed) in case of success or -1 in case of error.
*/
int
virConnectClose(virConnectPtr conn)
--
1.7.4.4
--
Matthias Bolte
http://photron.blogspot.com
11:57 a.m.
On 06/22/2011 10:43 AM, Matthias Bolte wrote:
2011/6/22 Eric Blake <eblake(a)redhat.com>:
> * src/libvirt.c (virConnectClose): Mention reference count return.
> Reported by Michal Novotny, analyzed by Matthias Bolte.
> ---
> src/libvirt.c | 7 ++++++-
> 1 files changed, 6 insertions(+), 1 deletions(-)
>
> - * Returns 0 in case of success or -1 in case of error.
> + * Connections are reference counted; the connection is not actually
> + * closed until all calls to virConnectRef have had a matching
> + * virConnectClose call.
Actually you need to match the virConnectOpen* call too. Interpreted
strictly it says that you only need to match the virConnectRef calls
which means that you don't have to call it when you didn't call
virConnectRef for a connection, doesn't it? Maybe just adding
appending a "too" to the sentence fixes this.
The reference counting applies to vir(Domain|Network|...)Free too. But
this comment is missing that domain, network, etc objects that depend
on the connection take a reference on it too. So it's more complicated
than the comment implies. I'm not sure if we should explain this here.
Hmm, good points. How about:
Connections are reference counted; the count is explicitly increased by
virConnectRef and virConnectOpen, as well as temporarily increased by
other API that depend on the connection remaining alive. Every
virConnectRef and virConnectOpen call should have a matching
virConnectClose, and all other references will be released after the
corresponding operation completes.
The return value is the number of remaining references on success
(positive implies that some other call still has a reference open, 0
implies that no references remain and the connection is closed), or -1
on failure. It is possible for the last virConnectClose to return a
positive value if some other object still has a temporary reference to
the connection, but the application should not try to further use a
connection after the virConnectClose that matches the virConnectOpen.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
12:30 p.m.
2011/6/22 Eric Blake <eblake(a)redhat.com>:
On 06/22/2011 10:43 AM, Matthias Bolte wrote:
> 2011/6/22 Eric Blake <eblake(a)redhat.com>:
>> * src/libvirt.c (virConnectClose): Mention reference count return.
>> Reported by Michal Novotny, analyzed by Matthias Bolte.
>> ---
>> src/libvirt.c | 7 ++++++-
>> 1 files changed, 6 insertions(+), 1 deletions(-)
>>
>> - * Returns 0 in case of success or -1 in case of error.
>> + * Connections are reference counted; the connection is not actually
>> + * closed until all calls to virConnectRef have had a matching
>> + * virConnectClose call.
>
> Actually you need to match the virConnectOpen* call too. Interpreted
> strictly it says that you only need to match the virConnectRef calls
> which means that you don't have to call it when you didn't call
> virConnectRef for a connection, doesn't it? Maybe just adding
> appending a "too" to the sentence fixes this.
>
> The reference counting applies to vir(Domain|Network|...)Free too. But
> this comment is missing that domain, network, etc objects that depend
> on the connection take a reference on it too. So it's more complicated
> than the comment implies. I'm not sure if we should explain this here.
Hmm, good points. How about:
Connections are reference counted; the count is explicitly increased by
virConnectRef and virConnectOpen, as well as temporarily increased by
other API that depend on the connection remaining alive. Every
virConnectRef and virConnectOpen call should have a matching
virConnectClose, and all other references will be released after the
corresponding operation completes.
There is virConnectOpenReadOnly and virConnectOpenAuth too.
You're a bit vague about the "other API", but I think its better this
way than being too explicit and creating some kind of list that need
to be maintained here.
The return value is the number of remaining references on success
(positive implies that some other call still has a reference open, 0
implies that no references remain and the connection is closed), or -1
on failure. It is possible for the last virConnectClose to return a
positive value if some other object still has a temporary reference to
the connection, but the application should not try to further use a
connection after the virConnectClose that matches the virConnectOpen.
ACK.
--
Matthias Bolte
http://photron.blogspot.com
2:28 p.m.
On 06/22/2011 11:30 AM, Matthias Bolte wrote:
> Hmm, good points. How about:
>
> Connections are reference counted; the count is explicitly increased by
> virConnectRef and virConnectOpen, as well as temporarily increased by
> other API that depend on the connection remaining alive. Every
> virConnectRef and virConnectOpen call should have a matching
> virConnectClose, and all other references will be released after the
> corresponding operation completes.
There is virConnectOpenReadOnly and virConnectOpenAuth too.
I went with the more general:
... the count is explicitly
* increased by the initial open (virConnectOpen, virConnectOpenAuth,
* and the like) as well as virConnectRef;
You're a bit vague about the "other API", but I think
its better this
way than being too explicit and creating some kind of list that need
to be maintained here.
Agreed.
> The return value is the number of remaining references on success
> (positive implies that some other call still has a reference open, 0
> implies that no references remain and the connection is closed), or -1
> on failure. It is possible for the last virConnectClose to return a
> positive value if some other object still has a temporary reference to
> the connection, but the application should not try to further use a
> connection after the virConnectClose that matches the virConnectOpen.
and here, I did s/virConnectOpen/the initial open/
ACK.
Pushed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
4937
days inactive
4937
days old
4 comments
2 participants
participants (2)
-
Eric Blake -
Matthias Bolte