[libvirt] [PATCH 0/7] Resolve Coverity issues
The recent changes to change 'int i' to 'size_t i' seem to have uncovered a few new Coverity issues - resolve those. Also add a [dead_error_code] tag to a Coverity issue that's been there for a bit. After these patches the result is just back to 1 Coverity error in 'virLXCProcessReboot' regarding the usage of 'conn' which still needs to be eventually resolved, see: https://www.redhat.com/archives/libvir-list/2013-May/msg01218.html and subsequent followups John Ferlan (7): hellolibvirt: Resolve Coverity issues testutils: Resolve Coverity issues virsh-domain-monitor: Resolve Coverity issues virsh-interface: Resolve Coverity issues virsh-nodedev: Resolve Coverity issues storage_backend: Resolve Coverity issue qemu_hostdev: Resolve Coverity issue examples/hellolibvirt/hellolibvirt.c | 7 +++++++ src/qemu/qemu_hostdev.c | 12 +++++++----- src/storage/storage_backend.c | 1 + tests/testutils.c | 3 +++ tools/virsh-domain-monitor.c | 10 ++++++---- tools/virsh-interface.c | 24 +++++++++++++----------- tools/virsh-nodedev.c | 12 +++++++----- 7 files changed, 44 insertions(+), 25 deletions(-) -- 1.8.1.4
Recent changes uncovered a NEGATIVE_RETURNS when processing 'numNames' in 'showDomains' in the for loop after a possible -1 return. --- examples/hellolibvirt/hellolibvirt.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellolibvirt.c index 83045b1..2a48681 100644 --- a/examples/hellolibvirt/hellolibvirt.c +++ b/examples/hellolibvirt/hellolibvirt.c @@ -90,6 +90,13 @@ showDomains(virConnectPtr conn) numNames = virConnectListAllDomains(conn, &nameList, flags); + if (numNames == -1) { + ret = 1; + printf ("Failed to get a list of all domains: %s\n", + virGetLastErrorMessage()); + goto out; + } + for (i = 0; i < numNames; i++) { int active = virDomainIsActive(nameList[i]); printf(" %8s (%s)\n", -- 1.8.1.4
On Thu, Jul 11, 2013 at 08:34:01AM -0400, John Ferlan wrote:
Recent changes uncovered a NEGATIVE_RETURNS when processing 'numNames' in 'showDomains' in the for loop after a possible -1 return. --- examples/hellolibvirt/hellolibvirt.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellolibvirt.c index 83045b1..2a48681 100644 --- a/examples/hellolibvirt/hellolibvirt.c +++ b/examples/hellolibvirt/hellolibvirt.c @@ -90,6 +90,13 @@ showDomains(virConnectPtr conn) numNames = virConnectListAllDomains(conn, &nameList, flags); + if (numNames == -1) { + ret = 1; + printf ("Failed to get a list of all domains: %s\n", + virGetLastErrorMessage()); + goto out; + } + for (i = 0; i < numNames; i++) { int active = virDomainIsActive(nameList[i]); printf(" %8s (%s)\n",
ACK 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 :|
On 07/11/2013 06:34 AM, John Ferlan wrote:
Recent changes uncovered a NEGATIVE_RETURNS when processing 'numNames' in 'showDomains' in the for loop after a possible -1 return. --- examples/hellolibvirt/hellolibvirt.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellolibvirt.c index 83045b1..2a48681 100644 --- a/examples/hellolibvirt/hellolibvirt.c +++ b/examples/hellolibvirt/hellolibvirt.c @@ -90,6 +90,13 @@ showDomains(virConnectPtr conn) numNames = virConnectListAllDomains(conn, &nameList, flags); + if (numNames == -1) { + ret = 1;
Should we (as an independent patch) tweak this entire example file to use EXIT_FAILURE instead of '1', to make it more obvious why we deviate from our normal practice of negative return on failure? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Recent changes uncovered a NEGATIVE_RETURNS in the return from sysconf() when processing a for loop in virtTestCaptureProgramExecChild() --- tests/testutils.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/testutils.c b/tests/testutils.c index ec0fe52..2fdf7b8 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -281,6 +281,9 @@ void virtTestCaptureProgramExecChild(const char *const argv[], goto cleanup; open_max = sysconf(_SC_OPEN_MAX); + if (open_max < 0) + goto cleanup; + for (i = 0; i < open_max; i++) { if (i != stdinfd && i != pipefd) { -- 1.8.1.4
On Thu, Jul 11, 2013 at 08:34:02AM -0400, John Ferlan wrote:
Recent changes uncovered a NEGATIVE_RETURNS in the return from sysconf() when processing a for loop in virtTestCaptureProgramExecChild() --- tests/testutils.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tests/testutils.c b/tests/testutils.c index ec0fe52..2fdf7b8 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -281,6 +281,9 @@ void virtTestCaptureProgramExecChild(const char *const argv[], goto cleanup;
open_max = sysconf(_SC_OPEN_MAX); + if (open_max < 0) + goto cleanup; + for (i = 0; i < open_max; i++) { if (i != stdinfd && i != pipefd) {
ACK, there are several more cases of sysconf(_SC_OPEN_MAX) in other libvirt code which are not checked too, but happen to not trigger coverity. We ought to fix those too. 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 :|
Recent changes uncovered a pair of NEGATIVE_RETURNS when processing the 'nnames' in 'vshDomainListCollect' in the for loop due to possible -1 value. --- tools/virsh-domain-monitor.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 7af765e..bd853ba 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1399,6 +1399,7 @@ vshDomainListCollect(vshControl *ctl, unsigned int flags) vshDomainListPtr list = vshMalloc(ctl, sizeof(*list)); size_t i; int ret; + int rc; int *ids = NULL; int nids = 0; char **names = NULL; @@ -1467,16 +1468,17 @@ fallback: if (!VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE) || VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_INACTIVE)) { - if ((nnames = virConnectNumOfDefinedDomains(ctl->conn)) < 0) { + if ((rc = virConnectNumOfDefinedDomains(ctl->conn)) < 0) { vshError(ctl, "%s", _("Failed to list inactive domains")); goto cleanup; } - if (nnames) { + if (rc) { + nnames = rc; names = vshMalloc(ctl, sizeof(char *) * nnames); - if ((nnames = virConnectListDefinedDomains(ctl->conn, names, - nnames)) < 0) { + if ((rc = virConnectListDefinedDomains(ctl->conn, names, + nnames)) < 0) { vshError(ctl, "%s", _("Failed to list inactive domains")); goto cleanup; } -- 1.8.1.4
On Thu, Jul 11, 2013 at 08:34:03AM -0400, John Ferlan wrote:
Recent changes uncovered a pair of NEGATIVE_RETURNS when processing the 'nnames' in 'vshDomainListCollect' in the for loop due to possible -1 value. --- tools/virsh-domain-monitor.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 7af765e..bd853ba 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1399,6 +1399,7 @@ vshDomainListCollect(vshControl *ctl, unsigned int flags) vshDomainListPtr list = vshMalloc(ctl, sizeof(*list)); size_t i; int ret; + int rc; int *ids = NULL; int nids = 0; char **names = NULL; @@ -1467,16 +1468,17 @@ fallback:
if (!VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE) || VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_INACTIVE)) { - if ((nnames = virConnectNumOfDefinedDomains(ctl->conn)) < 0) { + if ((rc = virConnectNumOfDefinedDomains(ctl->conn)) < 0) { vshError(ctl, "%s", _("Failed to list inactive domains")); goto cleanup; }
- if (nnames) { + if (rc) { + nnames = rc; names = vshMalloc(ctl, sizeof(char *) * nnames);
- if ((nnames = virConnectListDefinedDomains(ctl->conn, names, - nnames)) < 0) { + if ((rc = virConnectListDefinedDomains(ctl->conn, names, + nnames)) < 0) { vshError(ctl, "%s", _("Failed to list inactive domains")); goto cleanup; }
I think is is nicer to change the code in the 'cleanup' label to do cleanup: for (i = 0; nnames != -1 && i < nnames; i++) VIR_FREE(names[i]); 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 :|
Recent changes uncovered FORWARD_NULL and NEGATIVE_RETURNS problems with the processing of the 'nActiveIfaces' and 'nInactiveIfaces' and their associated allocated arrays in 'vshInterfaceListCollect' due to the possibility of returning -1 in a call and using the return value as a for loop index end condition. --- tools/virsh-interface.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index 9fdd36e..5ee16a3 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -199,6 +199,7 @@ vshInterfaceListCollect(vshControl *ctl, int nActiveIfaces = 0; int nInactiveIfaces = 0; int nAllIfaces = 0; + int rc; /* try the list with flags support (0.10.2 and later) */ if ((ret = virConnectListAllInterfaces(ctl->conn, @@ -222,16 +223,17 @@ fallback: vshResetLibvirtError(); if (flags & VIR_CONNECT_LIST_INTERFACES_ACTIVE) { - nActiveIfaces = virConnectNumOfInterfaces(ctl->conn); - if (nActiveIfaces < 0) { + rc = virConnectNumOfInterfaces(ctl->conn); + if (rc < 0) { vshError(ctl, "%s", _("Failed to list active interfaces")); goto cleanup; } - if (nActiveIfaces) { + if (rc) { + nActiveIfaces = rc; activeNames = vshMalloc(ctl, sizeof(char *) * nActiveIfaces); - if ((nActiveIfaces = virConnectListInterfaces(ctl->conn, activeNames, - nActiveIfaces)) < 0) { + if ((rc = virConnectListInterfaces(ctl->conn, activeNames, + nActiveIfaces)) < 0) { vshError(ctl, "%s", _("Failed to list active interfaces")); goto cleanup; } @@ -239,17 +241,17 @@ fallback: } if (flags & VIR_CONNECT_LIST_INTERFACES_INACTIVE) { - nInactiveIfaces = virConnectNumOfDefinedInterfaces(ctl->conn); - if (nInactiveIfaces < 0) { + rc = virConnectNumOfDefinedInterfaces(ctl->conn); + if (rc < 0) { vshError(ctl, "%s", _("Failed to list inactive interfaces")); goto cleanup; } - if (nInactiveIfaces) { + if (rc) { + nInactiveIfaces = rc; inactiveNames = vshMalloc(ctl, sizeof(char *) * nInactiveIfaces); - if ((nInactiveIfaces = - virConnectListDefinedInterfaces(ctl->conn, inactiveNames, - nInactiveIfaces)) < 0) { + if ((rc = virConnectListDefinedInterfaces(ctl->conn, inactiveNames, + nInactiveIfaces)) < 0) { vshError(ctl, "%s", _("Failed to list inactive interfaces")); goto cleanup; } -- 1.8.1.4
On Thu, Jul 11, 2013 at 08:34:04AM -0400, John Ferlan wrote:
Recent changes uncovered FORWARD_NULL and NEGATIVE_RETURNS problems with the processing of the 'nActiveIfaces' and 'nInactiveIfaces' and their associated allocated arrays in 'vshInterfaceListCollect' due to the possibility of returning -1 in a call and using the return value as a for loop index end condition.
Again, I think it is preferrable to change the for() condition in the cleanup: label instead. I did that in several other places in virsh code, but clearly missed this one & a few others. 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 :|
Recent changes uncovered FORWARD_NULL and NEGATIVE_RETURNS problems with the processing of the 'ndevices' and its associated allocated arrays in 'vshNodeDeviceListCollect' due to the possibility of returning -1 in a call and using the returned value as a for loop index end condition. --- tools/virsh-nodedev.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 2eb1979..d22e1c6 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -234,6 +234,7 @@ vshNodeDeviceListCollect(vshControl *ctl, size_t deleted = 0; int ndevices = 0; char **names = NULL; + int rc; /* try the list with flags support (0.10.2 and later) */ if ((ret = virConnectListAllNodeDevices(ctl->conn, @@ -256,19 +257,20 @@ fallback: /* fall back to old method (0.10.1 and older) */ vshResetLibvirtError(); - ndevices = virNodeNumOfDevices(ctl->conn, NULL, 0); - if (ndevices < 0) { + rc = virNodeNumOfDevices(ctl->conn, NULL, 0); + if (rc < 0) { vshError(ctl, "%s", _("Failed to count node devices")); goto cleanup; } - if (ndevices == 0) + if (rc == 0) return list; + ndevices = rc; names = vshMalloc(ctl, sizeof(char *) * ndevices); - ndevices = virNodeListDevices(ctl->conn, NULL, names, ndevices, 0); - if (ndevices < 0) { + rc = virNodeListDevices(ctl->conn, NULL, names, ndevices, 0); + if (rc < 0) { vshError(ctl, "%s", _("Failed to list node devices")); goto cleanup; } -- 1.8.1.4
On Thu, Jul 11, 2013 at 08:34:05AM -0400, John Ferlan wrote:
Recent changes uncovered FORWARD_NULL and NEGATIVE_RETURNS problems with the processing of the 'ndevices' and its associated allocated arrays in 'vshNodeDeviceListCollect' due to the possibility of returning -1 in a call and using the returned value as a for loop index end condition.
Same comment as prev patch. 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 :|
The switch statement in 'virStorageBackendCreateQemuImgOpts' used the for loop end condition 'VIR_STORAGE_FILE_FEATURE_LAST' as a possible value, but since that cannot happen Coverity spits out a DEADCODE message. Adding the Coverity tag just removes the Coverity message --- src/storage/storage_backend.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 5d06eb6..8d5880e 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -663,6 +663,7 @@ virStorageBackendCreateQemuImgOpts(char **opts, } break; + /* coverity[dead_error_begin] */ case VIR_STORAGE_FILE_FEATURE_LAST: ; } -- 1.8.1.4
On 07/11/2013 02:34 PM, John Ferlan wrote:
The switch statement in 'virStorageBackendCreateQemuImgOpts' used the for loop end condition 'VIR_STORAGE_FILE_FEATURE_LAST' as a possible value, but since that cannot happen Coverity spits out a DEADCODE message. Adding the Coverity tag just removes the Coverity message --- src/storage/storage_backend.c | 1 + 1 file changed, 1 insertion(+)
ACK Jan
On Thu, Jul 11, 2013 at 08:34:06AM -0400, John Ferlan wrote:
The switch statement in 'virStorageBackendCreateQemuImgOpts' used the for loop end condition 'VIR_STORAGE_FILE_FEATURE_LAST' as a possible value, but since that cannot happen Coverity spits out a DEADCODE message. Adding the Coverity tag just removes the Coverity message --- src/storage/storage_backend.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 5d06eb6..8d5880e 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -663,6 +663,7 @@ virStorageBackendCreateQemuImgOpts(char **opts, } break;
+ /* coverity[dead_error_begin] */ case VIR_STORAGE_FILE_FEATURE_LAST: ; }
Wow, I tend to think that this code is terribly over engineered. We really don't need to have a for loop with a nested case. The entire thing can be replaced with something much simpler. 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 :|
On 07/11/2013 09:56 AM, Daniel P. Berrange wrote:
On Thu, Jul 11, 2013 at 08:34:06AM -0400, John Ferlan wrote:
The switch statement in 'virStorageBackendCreateQemuImgOpts' used the for loop end condition 'VIR_STORAGE_FILE_FEATURE_LAST' as a possible value, but since that cannot happen Coverity spits out a DEADCODE message. Adding the Coverity tag just removes the Coverity message --- src/storage/storage_backend.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 5d06eb6..8d5880e 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -663,6 +663,7 @@ virStorageBackendCreateQemuImgOpts(char **opts, } break;
+ /* coverity[dead_error_begin] */ case VIR_STORAGE_FILE_FEATURE_LAST: ; }
Wow, I tend to think that this code is terribly over engineered. We really don't need to have a for loop with a nested case. The entire thing can be replaced with something much simpler.
Daniel
It's not the only code that does something similar, but I was following how it was handled in other code. See: src/lxc/lxc_driver.c (search on dead_error_begin or LXC_NB_BLKIO_PARAM) Should that code change as well? For this code I wasn't sure if the future held more VIR_STORAGE_FILE_FEATURE options... John
On Thu, Jul 11, 2013 at 10:48:50AM -0400, John Ferlan wrote:
On 07/11/2013 09:56 AM, Daniel P. Berrange wrote:
On Thu, Jul 11, 2013 at 08:34:06AM -0400, John Ferlan wrote:
The switch statement in 'virStorageBackendCreateQemuImgOpts' used the for loop end condition 'VIR_STORAGE_FILE_FEATURE_LAST' as a possible value, but since that cannot happen Coverity spits out a DEADCODE message. Adding the Coverity tag just removes the Coverity message --- src/storage/storage_backend.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 5d06eb6..8d5880e 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -663,6 +663,7 @@ virStorageBackendCreateQemuImgOpts(char **opts, } break;
+ /* coverity[dead_error_begin] */ case VIR_STORAGE_FILE_FEATURE_LAST: ; }
Wow, I tend to think that this code is terribly over engineered. We really don't need to have a for loop with a nested case. The entire thing can be replaced with something much simpler.
Daniel
It's not the only code that does something similar, but I was following how it was handled in other code. See:
src/lxc/lxc_driver.c (search on dead_error_begin or LXC_NB_BLKIO_PARAM)
Should that code change as well?
For this code I wasn't sure if the future held more VIR_STORAGE_FILE_FEATURE options...
Well, ok, lets go with your first patch for now. ACK 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 :|
Recent changes uncovered a possibility that 'last_processed_hostdev_vf' was set to -1 in 'qemuPrepareHostdevPCIDevices' and would cause problems in for loop end condition in the 'resetvfnetconfig' label if the variable was never set to 'i' due to 'qemuDomainHostdevNetConfigReplace' failure. --- src/qemu/qemu_hostdev.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 9bdace1..970d099 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -644,11 +644,13 @@ inactivedevs: } resetvfnetconfig: - for (i = 0; i < last_processed_hostdev_vf; i++) { - virDomainHostdevDefPtr hostdev = hostdevs[i]; - if (hostdev->parent.type == VIR_DOMAIN_DEVICE_NET && - hostdev->parent.data.net) { - qemuDomainHostdevNetConfigRestore(hostdev, cfg->stateDir); + if (last_processed_hostdev_vf >= 0) { + for (i = 0; i < last_processed_hostdev_vf; i++) { + virDomainHostdevDefPtr hostdev = hostdevs[i]; + if (hostdev->parent.type == VIR_DOMAIN_DEVICE_NET && + hostdev->parent.data.net) { + qemuDomainHostdevNetConfigRestore(hostdev, cfg->stateDir); + } } } -- 1.8.1.4
On Thu, Jul 11, 2013 at 08:34:07AM -0400, John Ferlan wrote:
Recent changes uncovered a possibility that 'last_processed_hostdev_vf' was set to -1 in 'qemuPrepareHostdevPCIDevices' and would cause problems in for loop end condition in the 'resetvfnetconfig' label if the variable was never set to 'i' due to 'qemuDomainHostdevNetConfigReplace' failure. --- src/qemu/qemu_hostdev.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 9bdace1..970d099 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -644,11 +644,13 @@ inactivedevs: }
resetvfnetconfig: - for (i = 0; i < last_processed_hostdev_vf; i++) { - virDomainHostdevDefPtr hostdev = hostdevs[i]; - if (hostdev->parent.type == VIR_DOMAIN_DEVICE_NET && - hostdev->parent.data.net) { - qemuDomainHostdevNetConfigRestore(hostdev, cfg->stateDir); + if (last_processed_hostdev_vf >= 0) { + for (i = 0; i < last_processed_hostdev_vf; i++) { + virDomainHostdevDefPtr hostdev = hostdevs[i]; + if (hostdev->parent.type == VIR_DOMAIN_DEVICE_NET && + hostdev->parent.data.net) { + qemuDomainHostdevNetConfigRestore(hostdev, cfg->stateDir); + } } }
Slight preference for just changing the termination condition in the for() loop to include "last_processed_hostdev_vf != -1" 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 :|
participants (4)
-
Daniel P. Berrange -
Eric Blake -
John Ferlan -
Ján Tomko