[Qemu-devel] Re: [PATCH 3/3] monitor: Convert do_migrate() to QError
Luiz Capitulino lcapitul...@redhat.com writes:
On Thu, 25 Mar 2010 20:30:33 +0100
Markus Armbruster arm...@redhat.com wrote:
Luiz Capitulino lcapitul...@redhat.com writes:
On Thu, 25 Mar 2010 18:37:25 +0100
Markus Armbruster arm...@redhat.com wrote:
[...]
@@ -86,12 +86,13 @@ int do_migrate(Monitor *mon, const QDict *qdict,
QObject **ret_data)
(int)qdict_get_int(qdict,
inc));
#endif
} else {
-monitor_printf(mon, unknown migration protocol: %s\n, uri);
+qerror_report(QERR_INVALID_PARAMETER, uri);
return -1;
}
if (s == NULL) {
-monitor_printf(mon, migration failed\n);
+/* TODO push error reporting into the
FOO_start_outgoing_migration() */
+qerror_report(QERR_MIGRATION_FAILED);
return -1;
}
I think this one is no better than the automatic UndefinedError
which is going to be triggered. I would only touch this when/if
we get the migration functions converted.
I feel it is a bit better, because:
* It doesn't dilute the nice this is a bug, and I should report it
property of UndefinedError.
* It avoids the returned failure but did not pass an error message.
Minor, because it's under CONFIG_DEBUG_MONITOR.
But this is exactly what we want because having QERR_MIGRATION_FAILED
there doesn't fix the problems those warnings are making us aware of.
Except we are already aware of the problem, and additional warnings are
just noise.
Our memory is not the best place for it, not only because we can forget,
but also because it's limited on you and me.
Anyone who enables QMP's debugging support should be able to know what's
there to be done.
That's what TODO comments are for.
[Qemu-devel] Re: [PATCH 3/3] monitor: Convert do_migrate() to QError
On Thu, 25 Mar 2010 20:30:33 +0100
Markus Armbruster arm...@redhat.com wrote:
Luiz Capitulino lcapitul...@redhat.com writes:
On Thu, 25 Mar 2010 18:37:25 +0100
Markus Armbruster arm...@redhat.com wrote:
[...]
@@ -86,12 +86,13 @@ int do_migrate(Monitor *mon, const QDict *qdict,
QObject **ret_data)
(int)qdict_get_int(qdict,
inc));
#endif
} else {
-monitor_printf(mon, unknown migration protocol: %s\n, uri);
+qerror_report(QERR_INVALID_PARAMETER, uri);
return -1;
}
if (s == NULL) {
-monitor_printf(mon, migration failed\n);
+/* TODO push error reporting into the
FOO_start_outgoing_migration() */
+qerror_report(QERR_MIGRATION_FAILED);
return -1;
}
I think this one is no better than the automatic UndefinedError
which is going to be triggered. I would only touch this when/if
we get the migration functions converted.
I feel it is a bit better, because:
* It doesn't dilute the nice this is a bug, and I should report it
property of UndefinedError.
* It avoids the returned failure but did not pass an error message.
Minor, because it's under CONFIG_DEBUG_MONITOR.
But this is exactly what we want because having QERR_MIGRATION_FAILED
there doesn't fix the problems those warnings are making us aware of.
Except we are already aware of the problem, and additional warnings are
just noise.
Our memory is not the best place for it, not only because we can forget,
but also because it's limited on you and me.
Anyone who enables QMP's debugging support should be able to know what's
there to be done.
[Qemu-devel] Re: [PATCH 3/3] monitor: Convert do_migrate() to QError
Luiz Capitulino lcapitul...@redhat.com writes:
On Tue, 23 Mar 2010 19:07:21 +0100
Markus Armbruster arm...@redhat.com wrote:
Human monitor error message changes from unknown migration protocol:
FOO to Invalid parameter uri.
The conversion is shallow: the FOO_start_outgoing_migration() aren't
converted. Converting them is a big job for relatively little
practical benefit, so leave it for later.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
migration.c |9 +
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/migration.c b/migration.c
index 05f6cc5..47d2ab5 100644
--- a/migration.c
+++ b/migration.c
@@ -56,14 +56,14 @@ void qemu_start_incoming_migration(const char *uri)
int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
-MigrationState *s = NULL;
+MigrationState *s;
const char *p;
int detach = qdict_get_int(qdict, detach);
const char *uri = qdict_get_str(qdict, uri);
if (current_migration
current_migration-get_status(current_migration) ==
MIG_STATE_ACTIVE) {
-monitor_printf(mon, migration already in progress\n);
+qerror_report(QERR_MIGRATION_IN_PROGRESS);
return -1;
}
What about QERR_OPERATION_IN_PROGRESS? So that we have:
Operation already in progress: migration.
We'd get an error argument activity: migration. Fine with me.
@@ -86,12 +86,13 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject
**ret_data)
(int)qdict_get_int(qdict, inc));
#endif
} else {
-monitor_printf(mon, unknown migration protocol: %s\n, uri);
+qerror_report(QERR_INVALID_PARAMETER, uri);
return -1;
}
if (s == NULL) {
-monitor_printf(mon, migration failed\n);
+/* TODO push error reporting into the
FOO_start_outgoing_migration() */
+qerror_report(QERR_MIGRATION_FAILED);
return -1;
}
I think this one is no better than the automatic UndefinedError
which is going to be triggered. I would only touch this when/if
we get the migration functions converted.
I feel it is a bit better, because:
* It doesn't dilute the nice this is a bug, and I should report it
property of UndefinedError.
* It avoids the returned failure but did not pass an error message.
Minor, because it's under CONFIG_DEBUG_MONITOR.
But I'm not particular about it.
[Qemu-devel] Re: [PATCH 3/3] monitor: Convert do_migrate() to QError
On Thu, 25 Mar 2010 18:37:25 +0100
Markus Armbruster arm...@redhat.com wrote:
[...]
@@ -86,12 +86,13 @@ int do_migrate(Monitor *mon, const QDict *qdict,
QObject **ret_data)
(int)qdict_get_int(qdict, inc));
#endif
} else {
-monitor_printf(mon, unknown migration protocol: %s\n, uri);
+qerror_report(QERR_INVALID_PARAMETER, uri);
return -1;
}
if (s == NULL) {
-monitor_printf(mon, migration failed\n);
+/* TODO push error reporting into the
FOO_start_outgoing_migration() */
+qerror_report(QERR_MIGRATION_FAILED);
return -1;
}
I think this one is no better than the automatic UndefinedError
which is going to be triggered. I would only touch this when/if
we get the migration functions converted.
I feel it is a bit better, because:
* It doesn't dilute the nice this is a bug, and I should report it
property of UndefinedError.
* It avoids the returned failure but did not pass an error message.
Minor, because it's under CONFIG_DEBUG_MONITOR.
But this is exactly what we want because having QERR_MIGRATION_FAILED
there doesn't fix the problems those warnings are making us aware of.
[Qemu-devel] Re: [PATCH 3/3] monitor: Convert do_migrate() to QError
Luiz Capitulino lcapitul...@redhat.com writes:
On Thu, 25 Mar 2010 18:37:25 +0100
Markus Armbruster arm...@redhat.com wrote:
[...]
@@ -86,12 +86,13 @@ int do_migrate(Monitor *mon, const QDict *qdict,
QObject **ret_data)
(int)qdict_get_int(qdict,
inc));
#endif
} else {
-monitor_printf(mon, unknown migration protocol: %s\n, uri);
+qerror_report(QERR_INVALID_PARAMETER, uri);
return -1;
}
if (s == NULL) {
-monitor_printf(mon, migration failed\n);
+/* TODO push error reporting into the
FOO_start_outgoing_migration() */
+qerror_report(QERR_MIGRATION_FAILED);
return -1;
}
I think this one is no better than the automatic UndefinedError
which is going to be triggered. I would only touch this when/if
we get the migration functions converted.
I feel it is a bit better, because:
* It doesn't dilute the nice this is a bug, and I should report it
property of UndefinedError.
* It avoids the returned failure but did not pass an error message.
Minor, because it's under CONFIG_DEBUG_MONITOR.
But this is exactly what we want because having QERR_MIGRATION_FAILED
there doesn't fix the problems those warnings are making us aware of.
Except we are already aware of the problem, and additional warnings are
just noise.
[Qemu-devel] Re: [PATCH 3/3] monitor: Convert do_migrate() to QError
On Tue, 23 Mar 2010 19:07:21 +0100
Markus Armbruster arm...@redhat.com wrote:
Human monitor error message changes from unknown migration protocol:
FOO to Invalid parameter uri.
The conversion is shallow: the FOO_start_outgoing_migration() aren't
converted. Converting them is a big job for relatively little
practical benefit, so leave it for later.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
migration.c |9 +
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/migration.c b/migration.c
index 05f6cc5..47d2ab5 100644
--- a/migration.c
+++ b/migration.c
@@ -56,14 +56,14 @@ void qemu_start_incoming_migration(const char *uri)
int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
-MigrationState *s = NULL;
+MigrationState *s;
const char *p;
int detach = qdict_get_int(qdict, detach);
const char *uri = qdict_get_str(qdict, uri);
if (current_migration
current_migration-get_status(current_migration) ==
MIG_STATE_ACTIVE) {
-monitor_printf(mon, migration already in progress\n);
+qerror_report(QERR_MIGRATION_IN_PROGRESS);
return -1;
}
What about QERR_OPERATION_IN_PROGRESS? So that we have:
Operation already in progress: migration.
@@ -86,12 +86,13 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject
**ret_data)
(int)qdict_get_int(qdict, inc));
#endif
} else {
-monitor_printf(mon, unknown migration protocol: %s\n, uri);
+qerror_report(QERR_INVALID_PARAMETER, uri);
return -1;
}
if (s == NULL) {
-monitor_printf(mon, migration failed\n);
+/* TODO push error reporting into the FOO_start_outgoing_migration()
*/
+qerror_report(QERR_MIGRATION_FAILED);
return -1;
}
I think this one is no better than the automatic UndefinedError
which is going to be triggered. I would only touch this when/if
we get the migration functions converted.
