[libvirt] [PATCH] libvirt.c: fix virStateActive return value; document ...
The s/1/-1/ fix induces no semantic change, since the sole use of virStateActive tests solely for nonzero.
From 4bc9713207a2ed6b101e2067f7bba82d1df45987 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Mon, 24 Nov 2008 15:52:55 +0100 Subject: [PATCH] libvirt.c: fix virStateActive return value; document some new functions
* src/libvirt.c (virStateActive): Return -1 upon error, not 1, to be consistent with the other virState* functions. (virStateActive, virStateCleanup, virStateReload, virStateActive): Add per-function comments. --- src/libvirt.c | 30 +++++++++++++++++++++++++++++- 1 files changed, 29 insertions(+), 1 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index f445333..10835b9 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -636,6 +636,13 @@ virRegisterStateDriver(virStateDriverPtr driver) return virStateDriverTabCount++; } +/** + * virStateInitialize: + * + * Initialize all virtualization drivers. + * + * Return 0 if successful, -1 upon error. + */ int virStateInitialize(void) { int i, ret = 0; @@ -650,6 +657,13 @@ int virStateInitialize(void) { return ret; } +/** + * virStateCleanup: + * + * Run each virtualization driver's cleanup method. + * + * Return 0 if successful, -1 upon error. + */ int virStateCleanup(void) { int i, ret = 0; @@ -661,6 +675,13 @@ int virStateCleanup(void) { return ret; } +/** + * virStateReload + * + * Run each virtualization driver's reload method. + * + * Return 0 if successful, -1 upon error. + */ int virStateReload(void) { int i, ret = 0; @@ -672,13 +693,20 @@ int virStateReload(void) { return ret; } +/** + * virStateActive + * + * Run each virtualization driver's "active" method. + * + * Return 0 if successful, -1 upon error. + */ int virStateActive(void) { int i, ret = 0; for (i = 0 ; i < virStateDriverTabCount ; i++) { if (virStateDriverTab[i]->active && virStateDriverTab[i]->active()) - ret = 1; + ret = -1; } return ret; } -- 1.6.0.4.1021.g4320
On Mon, Nov 24, 2008 at 03:54:30PM +0100, Jim Meyering wrote:
The s/1/-1/ fix induces no semantic change, since the sole use of virStateActive tests solely for nonzero.
From 4bc9713207a2ed6b101e2067f7bba82d1df45987 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Mon, 24 Nov 2008 15:52:55 +0100 Subject: [PATCH] libvirt.c: fix virStateActive return value; document some new functions
* src/libvirt.c (virStateActive): Return -1 upon error, not 1, to be consistent with the other virState* functions. (virStateActive, virStateCleanup, virStateReload, virStateActive): Add per-function comments.
Looks fine to me ! +1 Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Mon, Nov 24, 2008 at 03:54:30PM +0100, Jim Meyering wrote:
The s/1/-1/ fix induces no semantic change, since the sole use of virStateActive tests solely for nonzero.
From 4bc9713207a2ed6b101e2067f7bba82d1df45987 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Mon, 24 Nov 2008 15:52:55 +0100 Subject: [PATCH] libvirt.c: fix virStateActive return value; document some new functions
* src/libvirt.c (virStateActive): Return -1 upon error, not 1, to be consistent with the other virState* functions. (virStateActive, virStateCleanup, virStateReload, virStateActive): Add per-function comments.
NACK.
+/** + * virStateActive + * + * Run each virtualization driver's "active" method. + * + * Return 0 if successful, -1 upon error. + */ int virStateActive(void) { int i, ret = 0;
for (i = 0 ; i < virStateDriverTabCount ; i++) { if (virStateDriverTab[i]->active && virStateDriverTab[i]->active()) - ret = 1; + ret = -1;
This is *not* an error condition. This method is basically asking whether the driver is 'active' - eg, does it have any domains running. It returns 0 if it isn't active, or 1 if it is active. There is no error scenario - it can never fail. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Mon, Nov 24, 2008 at 04:31:28PM +0000, Daniel P. Berrange wrote:
On Mon, Nov 24, 2008 at 03:54:30PM +0100, Jim Meyering wrote:
The s/1/-1/ fix induces no semantic change, since the sole use of virStateActive tests solely for nonzero.
From 4bc9713207a2ed6b101e2067f7bba82d1df45987 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Mon, 24 Nov 2008 15:52:55 +0100 Subject: [PATCH] libvirt.c: fix virStateActive return value; document some new functions
* src/libvirt.c (virStateActive): Return -1 upon error, not 1, to be consistent with the other virState* functions. (virStateActive, virStateCleanup, virStateReload, virStateActive): Add per-function comments.
NACK.
+/** + * virStateActive + * + * Run each virtualization driver's "active" method. + * + * Return 0 if successful, -1 upon error. + */ int virStateActive(void) { int i, ret = 0;
for (i = 0 ; i < virStateDriverTabCount ; i++) { if (virStateDriverTab[i]->active && virStateDriverTab[i]->active()) - ret = 1; + ret = -1;
This is *not* an error condition. This method is basically asking whether the driver is 'active' - eg, does it have any domains running. It returns 0 if it isn't active, or 1 if it is active.
There is no error scenario - it can never fail.
Then that should be explained in the (missing) comment! Still +1 for other parts of that patch including the comments I assume, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
"Daniel P. Berrange" <berrange@redhat.com> wrote: ...
+ * Run each virtualization driver's "active" method. + * + * Return 0 if successful, -1 upon error. + */ int virStateActive(void) { int i, ret = 0;
for (i = 0 ; i < virStateDriverTabCount ; i++) { if (virStateDriverTab[i]->active && virStateDriverTab[i]->active()) - ret = 1; + ret = -1;
This is *not* an error condition. This method is basically asking whether the driver is 'active' - eg, does it have any domains running. It returns 0 if it isn't active, or 1 if it is active.
There is no error scenario - it can never fail.
Ah. I see what you mean. This is better:
From a85968eed75cdbcf8848ea62b2739469fd6a9d60 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Mon, 24 Nov 2008 15:52:55 +0100 Subject: [PATCH 1/3] libvirt.c: document the virState* functions
* src/libvirt.c (virStateActive, virStateCleanup, virStateReload): (virStateActive): Add per-function comments. --- src/libvirt.c | 28 ++++++++++++++++++++++++++++ 1 files changed, 28 insertions(+), 0 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index f445333..6195a4e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -636,6 +636,13 @@ virRegisterStateDriver(virStateDriverPtr driver) return virStateDriverTabCount++; } +/** + * virStateInitialize: + * + * Initialize all virtualization drivers. + * + * Return 0 if all succeed, -1 upon any failure. + */ int virStateInitialize(void) { int i, ret = 0; @@ -650,6 +657,13 @@ int virStateInitialize(void) { return ret; } +/** + * virStateCleanup: + * + * Run each virtualization driver's cleanup method. + * + * Return 0 if all succeed, -1 upon any failure. + */ int virStateCleanup(void) { int i, ret = 0; @@ -661,6 +675,13 @@ int virStateCleanup(void) { return ret; } +/** + * virStateReload: + * + * Run each virtualization driver's reload method. + * + * Return 0 if all succeed, -1 upon any failure. + */ int virStateReload(void) { int i, ret = 0; @@ -672,6 +693,13 @@ int virStateReload(void) { return ret; } +/** + * virStateActive: + * + * Run each virtualization driver's "active" method. + * + * Return 0 if none are active, 1 if at least one is. + */ int virStateActive(void) { int i, ret = 0; -- 1.6.0.4.1044.g77718
On Mon, Nov 24, 2008 at 06:05:37PM +0100, Jim Meyering wrote:
"Daniel P. Berrange" <berrange@redhat.com> wrote: ...
+ * Run each virtualization driver's "active" method. + * + * Return 0 if successful, -1 upon error. + */ int virStateActive(void) { int i, ret = 0;
for (i = 0 ; i < virStateDriverTabCount ; i++) { if (virStateDriverTab[i]->active && virStateDriverTab[i]->active()) - ret = 1; + ret = -1;
This is *not* an error condition. This method is basically asking whether the driver is 'active' - eg, does it have any domains running. It returns 0 if it isn't active, or 1 if it is active.
There is no error scenario - it can never fail.
Ah. I see what you mean. This is better:
From a85968eed75cdbcf8848ea62b2739469fd6a9d60 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Mon, 24 Nov 2008 15:52:55 +0100 Subject: [PATCH 1/3] libvirt.c: document the virState* functions
ACK. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Jim Meyering