Het Gala <het.g...@nutanix.com> wrote:
> Socket transport backend for 'migrate'/'migrate-incoming' QAPIs accept
> new wire protocol of MigrateAddress struct.
>
> It is achived by parsing 'uri' string and storing migration parameters
> required for socket connection into well defined SocketAddress struct.
>
> Suggested-by: Aravind Retnakaran <aravind.retnaka...@nutanix.com>
> Signed-off-by: Het Gala <het.g...@nutanix.com>
> ---
> migration/exec.c | 4 ++--
> migration/exec.h | 4 ++++
> migration/migration.c | 44 +++++++++++++++++++++++++++++++------------
> migration/socket.c | 34 +++++----------------------------
> migration/socket.h | 7 ++++---
> 5 files changed, 47 insertions(+), 46 deletions(-)
>
> diff --git a/migration/exec.c b/migration/exec.c
> index 2bf882bbe1..c4a3293246 100644
> --- a/migration/exec.c
> +++ b/migration/exec.c
> @@ -27,7 +27,6 @@
> #include "qemu/cutils.h"
>
> #ifdef WIN32
> -const char *exec_get_cmd_path(void);
> const char *exec_get_cmd_path(void)
> {
> g_autofree char *detected_path = g_new(char, MAX_PATH);
Make this and related chunks it its own patch.
> @@ -40,7 +39,8 @@ const char *exec_get_cmd_path(void)
> }
> #endif
>
> -void exec_start_outgoing_migration(MigrationState *s, const char *command,
> Error **errp)
> +void exec_start_outgoing_migration(MigrationState *s, const char *command,
> + Error **errp)
> {
> QIOChannel *ioc;
>
Drop this bit. If you want to cleanup longer that 80 chars lines, do it
in a sperate patch.
> + if (local_err) {
> + qapi_free_SocketAddress(saddr);
> + error_propagate(errp, local_err);
> + return;
> + }
Aha, I see what you want to do here, but I
>From include/qapi/error.h
* - Whenever practical, also return a value that indicates success /
* failure. This can make the error checking more concise, and can
* avoid useless error object creation and destruction. Note that
* we still have many functions returning void. We recommend
* • bool-valued functions return true on success / false on failure,
* • pointer-valued functions return non-null / null pointer, and
* • integer-valued functions return non-negative / negative.
*
I think that what you have to do here is drop local_err and just change
socket_start_incoming_migration() and fd_start_incoming_migration() to
return a bool.
> -void socket_start_outgoing_migration(MigrationState *s,
> - const char *str,
> - Error **errp)
> -{
> - Error *err = NULL;
> - SocketAddress *saddr = socket_parse(str, &err);
> - if (!err) {
> - socket_start_outgoing_migration_internal(s, saddr, &err);
> - }
> - error_propagate(errp, err);
> -}
And here we are. This function got it wrong, and you copied from here.
My understanding is that this function should have been written as:
void socket_start_outgoing_migration(MigrationState *s,
const char *str,
Error **errp)
{
SocketAddress *saddr = socket_parse(str, &err);
if (!saddr) {
socket_start_outgoing_migration_internal(s, saddr, &err);
}
}
> -void socket_start_incoming_migration(const char *str, Error **errp)
> -{
> - Error *err = NULL;
> - SocketAddress *saddr = socket_parse(str, &err);
> - if (!err) {
> - socket_start_incoming_migration_internal(saddr, &err);
> - }
> - qapi_free_SocketAddress(saddr);
> - error_propagate(errp, err);
> -}
Exactly the same here.
Not your fault, but can you do the changes, please?
Later, Juan.
