[libvirt] [PATCH] docs: fix docs to match behavior of virConnectClose
* 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
2011/6/22 Eric Blake <eblake@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
On 06/22/2011 10:43 AM, Matthias Bolte wrote:
2011/6/22 Eric Blake <eblake@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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2011/6/22 Eric Blake <eblake@redhat.com>:
On 06/22/2011 10:43 AM, Matthias Bolte wrote:
2011/6/22 Eric Blake <eblake@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
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Matthias Bolte