12:30 a.m.
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(a)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
12:30 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: avoid dead store in qemuMonitorTextBlockJob
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(a)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
9:14 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: avoid dead store in qemuMonitorTextBlockJob
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(a)redhat.com>
---
src/qemu/qemu_monitor_text.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
ACK and pushed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
12:30 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: avoid dead store in doPeer2PeerMigrate3
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(a)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
9:27 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: avoid dead store in doPeer2PeerMigrate3
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(a)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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
3:12 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: avoid dead store in doPeer2PeerMigrate3
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(a)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
10:15 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: avoid dead store in doPeer2PeerMigrate3
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(a)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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
9:28 p.m.
New subject: [libvirt] [PATCH 3/3] qemu: avoid dead store in doPeer2PeerMigrate3
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(a)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
9:05 a.m.
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(a)redhat.com>
---
src/qemu/qemu_process.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
ACK and pushed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
4855
days inactive
4859
days old
8 comments
3 participants
participants (3)
-
ajia＠redhat.com -
Alex Jia
-
Eric Blake