[libvirt] [PATCH 1/3] qemu: avoid dead store in qemuProcessStart
Value stored to 'ret' is never read, in fact, 'cleanup' section will directly return -1 when function is fail, so remove this dead assignment. * src/qemu/qemu_process.c: kill dead assignment. Signed-off-by: Alex Jia <ajia@redhat.com> --- src/qemu/qemu_process.c | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 88cefd5..b0d2149 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2908,13 +2908,11 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Writing early domain status to disk"); if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) { - ret = -1; goto cleanup; } VIR_DEBUG("Waiting for handshake from child"); if (virCommandHandshakeWait(cmd) < 0) { - ret = -1; goto cleanup; } @@ -2943,7 +2941,6 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Labelling done, completing handshake to child"); if (virCommandHandshakeNotify(cmd) < 0) { - ret = -1; goto cleanup; } VIR_DEBUG("Handshake complete, child running"); -- 1.7.1
Value stored to 'ret' is never read, so remove this dead assignment. * src/qemu/qemu_monitor_text.c: kill dead assignment. Signed-off-by: Alex Jia <ajia@redhat.com> --- src/qemu/qemu_monitor_text.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 84941f9..7bf733d 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2981,7 +2981,6 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, return -1; } - ret = 0; if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot run monitor command")); -- 1.7.1
On 08/03/2011 11:30 PM, Alex Jia wrote:
Value stored to 'ret' is never read, so remove this dead assignment.
* src/qemu/qemu_monitor_text.c: kill dead assignment.
Signed-off-by: Alex Jia<ajia@redhat.com> --- src/qemu/qemu_monitor_text.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
ACK and pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Value stored to 'ret' is never read. If Confirm3 returns -1, there's nothing more we can do, here should remove this dead assignment. * src/qemu/qemu_migration.c: kill dead assignment. Signed-off-by: Alex Jia <ajia@redhat.com> --- src/qemu/qemu_migration.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 7aeea69..cde1bb6 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2018,9 +2018,9 @@ finish: cookieinlen = cookieoutlen; cookieout = NULL; cookieoutlen = 0; - ret = qemuMigrationConfirm(driver, sconn, vm, - cookiein, cookieinlen, - flags, cancelled); + qemuMigrationConfirm(driver, sconn, vm, + cookiein, cookieinlen, + flags, cancelled); /* If Confirm3 returns -1, there's nothing more we can * do, but fortunately worst case is that there is a * domain left in 'paused' state on source. -- 1.7.1
On 08/03/2011 11:30 PM, Alex Jia wrote:
Value stored to 'ret' is never read. If Confirm3 returns -1, there's nothing more we can do, here should remove this dead assignment.
* src/qemu/qemu_migration.c: kill dead assignment.
Signed-off-by: Alex Jia<ajia@redhat.com> --- src/qemu/qemu_migration.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
I'm thinking NACK as written. We should probably emit a log message if confirm failed, at which point, ret would be used in the conditional deciding whether to do a log message. Furthermore, libvirt.c virDomainMigrateVersion3 has the same bug with a dead assignment to ret; we should solve it in the same way for both implementations. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 08/04/2011 10:27 PM, Eric Blake wrote:
On 08/03/2011 11:30 PM, Alex Jia wrote:
Value stored to 'ret' is never read. If Confirm3 returns -1, there's nothing more we can do, here should remove this dead assignment.
* src/qemu/qemu_migration.c: kill dead assignment.
Signed-off-by: Alex Jia<ajia@redhat.com> --- src/qemu/qemu_migration.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
I'm thinking NACK as written. We should probably emit a log message if confirm failed, at which point, ret would be used in the conditional deciding whether to do a log message. Furthermore, libvirt.c virDomainMigrateVersion3 has the same bug with a dead assignment to ret; we should solve it in the same way for both implementations.
Eric, I haven't figured out a apposite error number in virterror.h, whether we need to define a new error number like this: VIR_ERR_MIGRATE_CONFIRM_FAILED = 75, /* a migration finished, but confirmation failed, there is a domain probably left in 'paused' state on source.*/ Thanks, Alex
On 08/05/2011 02:12 AM, Alex Jia wrote:
On 08/04/2011 10:27 PM, Eric Blake wrote:
On 08/03/2011 11:30 PM, Alex Jia wrote:
Value stored to 'ret' is never read. If Confirm3 returns -1, there's nothing more we can do, here should remove this dead assignment.
* src/qemu/qemu_migration.c: kill dead assignment.
Signed-off-by: Alex Jia<ajia@redhat.com> --- src/qemu/qemu_migration.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
I'm thinking NACK as written. We should probably emit a log message if confirm failed, at which point, ret would be used in the conditional deciding whether to do a log message. Furthermore, libvirt.c virDomainMigrateVersion3 has the same bug with a dead assignment to ret; we should solve it in the same way for both implementations.
Eric, I haven't figured out a apposite error number in virterror.h, whether we need to define a new error number like this:
VIR_ERR_MIGRATE_CONFIRM_FAILED = 75, /* a migration finished, but confirmation failed, there is a domain probably left in 'paused' state on source.*/
That's if we want to report the error back to the caller and fail the API. But the migration happened - it is running on the destination, so I think we have to make the API return success. Which means that we don't need a new VIR_ERR_* value, rather, we just need a new VIR_WARN() call to guarantee a log entry but nothing further. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 08/05/2011 11:15 PM, Eric Blake wrote:
On 08/05/2011 02:12 AM, Alex Jia wrote:
On 08/04/2011 10:27 PM, Eric Blake wrote:
On 08/03/2011 11:30 PM, Alex Jia wrote:
Value stored to 'ret' is never read. If Confirm3 returns -1, there's nothing more we can do, here should remove this dead assignment.
* src/qemu/qemu_migration.c: kill dead assignment.
Signed-off-by: Alex Jia<ajia@redhat.com> --- src/qemu/qemu_migration.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
I'm thinking NACK as written. We should probably emit a log message if confirm failed, at which point, ret would be used in the conditional deciding whether to do a log message. Furthermore, libvirt.c virDomainMigrateVersion3 has the same bug with a dead assignment to ret; we should solve it in the same way for both implementations.
Eric, I haven't figured out a apposite error number in virterror.h, whether we need to define a new error number like this:
VIR_ERR_MIGRATE_CONFIRM_FAILED = 75, /* a migration finished, but confirmation failed, there is a domain probably left in 'paused' state on source.*/
That's if we want to report the error back to the caller and fail the API. But the migration happened - it is running on the destination, so I think we have to make the API return success. Which means that we don't need a new VIR_ERR_* value, rather, we just need a new VIR_WARN() call to guarantee a log entry but nothing further.
Yeah, only need to record warning information into log. Thanks, Alex
On 08/03/2011 11:30 PM, Alex Jia wrote:
Value stored to 'ret' is never read, in fact, 'cleanup' section will directly return -1 when function is fail, so remove this dead assignment.
* src/qemu/qemu_process.c: kill dead assignment.
Signed-off-by: Alex Jia<ajia@redhat.com> --- src/qemu/qemu_process.c | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-)
ACK and pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (3)
-
ajia＠redhat.com -
Alex Jia
-
Eric Blake