[Qemu-devel] [PATCH v4 00/36] Migration errors & cleanup (the integrated version)

[Qemu-devel] [PATCH v4 00/36] Migration errors & cleanup (the integrated version)

[Juan Quintela]

Hi

v4:

- rebase on top of new qemu and new migration-errors series
- integrate back erros & cleanup series
- s/MIG_STATE_NONE/MIG_STATE_SETUP/ (Orit suggestion)
- s/migrate_create_state/migrate_new/ (Anthony suggestion)
- Add migrate_get_current() accessor.
- make has_error contain the errno instead of a bool
- rename qemu_file_has_error() -> qemu_file_get_error()
- rename has_error field into last_error
- migration_state_notifiers now pass MigrationState pointer


v3:
	this patch applies on top of my previous "migration error"
	patches.  All error handling has been moved to that series,
	except for "propagate error correctly", without this
	refactoring, it is quite complicated to apply it.

Please, review.

Later, Juan.

v3:
- more checkpatch.pl happines
- split error handling in a previous series
- make Anthony happy.  current_migration is still a pointer, but points to
  a static variable.  We can change current_migration when we integrate
  kemari.

v2:
- make Jan^Wcheckpatch.pl happy
- Yoshiaki Tamura suggestions:
  - include its two patches to clean things
  - MAX_THROTTLE define
  - migration_state enum
- I removed spurious differences between migration-{tcp,unix}
- better error propagation, after this patch:
   migrate -d "tcp:name_don_exist:port"
   migrate -d "tcp:name:port_dont_exist"
   migrate -d "exec: prog_dont_exist"
   migrate -d "exec: gzip > /path/dont/exist"
 fail as expected.  Last two used to enter an infinite loop.

The fixes part should be backported to 0.14, waiting for the review to do that.

Later, Juan.

v1:
This series:
- Fold MigrationState into FdMigrationState (and then rename)
- Factorize migration statec creation in a single place
- Make use of MIG_STATE_*, setup through helpers and make them local
- remove relase & cancel callbacks (where used only one in same
  file than defined)
- get_status() is no more, just access directly to .state
- current_migration use cleanup, and make variable static
- max_throotle is gone, now inside current_migration
- change get_migration_status() to migration_has_finished()
  and actualize single user.

Please review.

Later, Juan.


Juan Quintela (35):
  ds1225y: Use stdio instead of QEMUFile
  migration: simplify state assignmente
  migration: Check that migration is active before cancel it
  migration: return real error code
  migration: If there is one error, it makes no sense to continue
  buffered_file: Use right "opaque"
  buffered_file: reuse QEMUFile has_error field
  migration: don't "write" when migration is not active
  migration: set error if select return one error
  migration: change has_error to contain errno values
  migration: rename qemu_file_has_error to qemu_file_get_error
  savevm: Rename has_error to last_error field
  migration: use qemu_file_get_error() return value when possible
  migration: Make *start_outgoing_migration return FdMigrationState
  migration: Use FdMigrationState instead of MigrationState when
    possible
  migration: Fold MigrationState into FdMigrationState
  migration: Rename FdMigrationState MigrationState
  migration: Refactor MigrationState creation
  migration: Make all posible migration functions static
  migration: move migrate_new to do_migrate
  migration: Introduce MIG_STATE_SETUP
  migration: Refactor and simplify error checking in
    migrate_fd_put_ready
  migration: Introduce migrate_fd_completed() for consistency
  migration: Our release callback was just free
  migration: Remove get_status() accessor
  migration: Remove migration cancel() callback
  migration: Move exported functions to the end of the file
  migration: create accessor for current_migration
  migration: Use bandwidth_limit directly
  migration: Pass MigrationState in migration notifiers
  migration: Export a function that tells if the migration has finished
    correctly
  migration: Make state definitions local
  migration: Don't use callback on file defining it
  migration: propagate error correctly
  migration: make migration-{tcp,unix} consistent

Yoshiaki Tamura (1):
  migration: add error handling to migrate_fd_put_notify().

 arch_init.c       |    8 +-
 block-migration.c |   19 ++-
 buffered_file.c   |   34 +++--
 hw/ds1225y.c      |   28 ++--
 hw/hw.h           |    4 +-
 migration-exec.c  |   39 +----
 migration-fd.c    |   42 +----
 migration-tcp.c   |   76 ++++------
 migration-unix.c  |  113 ++++++---------
 migration.c       |  439 +++++++++++++++++++++++++++--------------------------
 migration.h       |   85 ++---------
 savevm.c          |   58 ++++----
 ui/spice-core.c   |    4 +-
 13 files changed, 412 insertions(+), 537 deletions(-)

-- 
1.7.6.4

[Qemu-devel] [PATCH 01/36] ds1225y: Use stdio instead of QEMUFile

[Juan Quintela]

QEMUFile * is only intended for migration nowadays.  Using it for
anything else just adds pain and a layer of buffers for no good
reason.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/ds1225y.c |   28 ++++++++++++++++------------
 1 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/hw/ds1225y.c b/hw/ds1225y.c
index 9875c44..6852a61 100644
--- a/hw/ds1225y.c
+++ b/hw/ds1225y.c
@@ -29,7 +29,7 @@ typedef struct {
     DeviceState qdev;
     uint32_t chip_size;
     char *filename;
-    QEMUFile *file;
+    FILE *file;
     uint8_t *contents;
 } NvRamState;

@@ -70,9 +70,9 @@ static void nvram_writeb (void *opaque, target_phys_addr_t addr, uint32_t val)

     s->contents[addr] = val;
     if (s->file) {
-        qemu_fseek(s->file, addr, SEEK_SET);
-        qemu_put_byte(s->file, (int)val);
-        qemu_fflush(s->file);
+        fseek(s->file, addr, SEEK_SET);
+        fputc(val, s->file);
+        fflush(s->file);
     }
 }

@@ -108,15 +108,17 @@ static int nvram_post_load(void *opaque, int version_id)

     /* Close file, as filename may has changed in load/store process */
     if (s->file) {
-        qemu_fclose(s->file);
+        fclose(s->file);
     }

     /* Write back nvram contents */
-    s->file = qemu_fopen(s->filename, "wb");
+    s->file = fopen(s->filename, "wb");
     if (s->file) {
         /* Write back contents, as 'wb' mode cleaned the file */
-        qemu_put_buffer(s->file, s->contents, s->chip_size);
-        qemu_fflush(s->file);
+        if (fwrite(s->contents, s->chip_size, 1, s->file) != 1) {
+            printf("nvram_post_load: short write\n");
+        }
+        fflush(s->file);
     }

     return 0;
@@ -143,7 +145,7 @@ typedef struct {
 static int nvram_sysbus_initfn(SysBusDevice *dev)
 {
     NvRamState *s = &FROM_SYSBUS(SysBusNvRamState, dev)->nvram;
-    QEMUFile *file;
+    FILE *file;
     int s_io;

     s->contents = g_malloc0(s->chip_size);
@@ -153,11 +155,13 @@ static int nvram_sysbus_initfn(SysBusDevice *dev)
     sysbus_init_mmio(dev, s->chip_size, s_io);

     /* Read current file */
-    file = qemu_fopen(s->filename, "rb");
+    file = fopen(s->filename, "rb");
     if (file) {
         /* Read nvram contents */
-        qemu_get_buffer(file, s->contents, s->chip_size);
-        qemu_fclose(file);
+        if (fread(s->contents, s->chip_size, 1, file) != 1) {
+            printf("nvram_sysbus_initfn: short read\n");
+        }
+        fclose(file);
     }
     nvram_post_load(s, 0);

-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 01/36] ds1225y: Use stdio instead of QEMUFile

[Zhi Hui Li]

On 10/11/2011 06:00 PM, Juan Quintela wrote:
> QEMUFile * is only intended for migration nowadays.  Using it for
> anything else just adds pain and a layer of buffers for no good
> reason.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>
> ---
>   hw/ds1225y.c |   28 ++++++++++++++++------------
>   1 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/hw/ds1225y.c b/hw/ds1225y.c
> index 9875c44..6852a61 100644
> --- a/hw/ds1225y.c
> +++ b/hw/ds1225y.c
> @@ -29,7 +29,7 @@ typedef struct {
>       DeviceState qdev;
>       uint32_t chip_size;
>       char *filename;
> -    QEMUFile *file;
> +    FILE *file;
>       uint8_t *contents;
>   } NvRamState;
>
> @@ -70,9 +70,9 @@ static void nvram_writeb (void *opaque, target_phys_addr_t addr, uint32_t val)
>
>       s->contents[addr] = val;
>       if (s->file) {
> -        qemu_fseek(s->file, addr, SEEK_SET);
> -        qemu_put_byte(s->file, (int)val);
> -        qemu_fflush(s->file);
> +        fseek(s->file, addr, SEEK_SET);
> +        fputc(val, s->file);
> +        fflush(s->file);
>       }
>   }
>
> @@ -108,15 +108,17 @@ static int nvram_post_load(void *opaque, int version_id)
>
>       /* Close file, as filename may has changed in load/store process */
>       if (s->file) {
> -        qemu_fclose(s->file);
> +        fclose(s->file);
>       }
>
>       /* Write back nvram contents */
> -    s->file = qemu_fopen(s->filename, "wb");
> +    s->file = fopen(s->filename, "wb");
>       if (s->file) {
>           /* Write back contents, as 'wb' mode cleaned the file */
> -        qemu_put_buffer(s->file, s->contents, s->chip_size);
> -        qemu_fflush(s->file);
> +        if (fwrite(s->contents, s->chip_size, 1, s->file) != 1) {
> +            printf("nvram_post_load: short write\n");
> +        }
> +        fflush(s->file);
>       }
>
>       return 0;
> @@ -143,7 +145,7 @@ typedef struct {
>   static int nvram_sysbus_initfn(SysBusDevice *dev)
>   {
>       NvRamState *s =&FROM_SYSBUS(SysBusNvRamState, dev)->nvram;
> -    QEMUFile *file;
> +    FILE *file;
>       int s_io;
>
>       s->contents = g_malloc0(s->chip_size);
> @@ -153,11 +155,13 @@ static int nvram_sysbus_initfn(SysBusDevice *dev)
>       sysbus_init_mmio(dev, s->chip_size, s_io);
>
>       /* Read current file */
> -    file = qemu_fopen(s->filename, "rb");
> +    file = fopen(s->filename, "rb");
>       if (file) {
>           /* Read nvram contents */
> -        qemu_get_buffer(file, s->contents, s->chip_size);
> -        qemu_fclose(file);
> +        if (fread(s->contents, s->chip_size, 1, file) != 1) {
> +            printf("nvram_sysbus_initfn: short read\n");
> +        }
> +        fclose(file);
>       }
>       nvram_post_load(s, 0);
>

Tested-by: Zhi Hui Li<zhihuili@linux.vnet.ibm.com>

Re: [Qemu-devel] [PATCH 01/36] ds1225y: Use stdio instead of QEMUFile

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> QEMUFile * is only intended for migration nowadays.  Using it for
> anything else just adds pain and a layer of buffers for no good
> reason.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   hw/ds1225y.c |   28 ++++++++++++++++------------
>   1 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/hw/ds1225y.c b/hw/ds1225y.c
> index 9875c44..6852a61 100644
> --- a/hw/ds1225y.c
> +++ b/hw/ds1225y.c
> @@ -29,7 +29,7 @@ typedef struct {
>       DeviceState qdev;
>       uint32_t chip_size;
>       char *filename;
> -    QEMUFile *file;
> +    FILE *file;
>       uint8_t *contents;
>   } NvRamState;
>
> @@ -70,9 +70,9 @@ static void nvram_writeb (void *opaque, target_phys_addr_t addr, uint32_t val)
>
>       s->contents[addr] = val;
>       if (s->file) {
> -        qemu_fseek(s->file, addr, SEEK_SET);
> -        qemu_put_byte(s->file, (int)val);
> -        qemu_fflush(s->file);
> +        fseek(s->file, addr, SEEK_SET);
> +        fputc(val, s->file);
> +        fflush(s->file);
>       }
>   }
>
> @@ -108,15 +108,17 @@ static int nvram_post_load(void *opaque, int version_id)
>
>       /* Close file, as filename may has changed in load/store process */
>       if (s->file) {
> -        qemu_fclose(s->file);
> +        fclose(s->file);
>       }
>
>       /* Write back nvram contents */
> -    s->file = qemu_fopen(s->filename, "wb");
> +    s->file = fopen(s->filename, "wb");
>       if (s->file) {
>           /* Write back contents, as 'wb' mode cleaned the file */
> -        qemu_put_buffer(s->file, s->contents, s->chip_size);
> -        qemu_fflush(s->file);
> +        if (fwrite(s->contents, s->chip_size, 1, s->file) != 1) {
> +            printf("nvram_post_load: short write\n");
> +        }
> +        fflush(s->file);
>       }
>
>       return 0;
> @@ -143,7 +145,7 @@ typedef struct {
>   static int nvram_sysbus_initfn(SysBusDevice *dev)
>   {
>       NvRamState *s =&FROM_SYSBUS(SysBusNvRamState, dev)->nvram;
> -    QEMUFile *file;
> +    FILE *file;
>       int s_io;
>
>       s->contents = g_malloc0(s->chip_size);
> @@ -153,11 +155,13 @@ static int nvram_sysbus_initfn(SysBusDevice *dev)
>       sysbus_init_mmio(dev, s->chip_size, s_io);
>
>       /* Read current file */
> -    file = qemu_fopen(s->filename, "rb");
> +    file = fopen(s->filename, "rb");
>       if (file) {
>           /* Read nvram contents */
> -        qemu_get_buffer(file, s->contents, s->chip_size);
> -        qemu_fclose(file);
> +        if (fread(s->contents, s->chip_size, 1, file) != 1) {
> +            printf("nvram_sysbus_initfn: short read\n");
> +        }
> +        fclose(file);
>       }
>       nvram_post_load(s, 0);
>

[Qemu-devel] [PATCH 02/36] migration: simplify state assignmente

[Juan Quintela]

Once there, make sure that if we already know that there is one error,
just call migration_fd_cleanup() with the ERROR state.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/migration.c b/migration.c
index 77a51ad..7fd6c23 100644
--- a/migration.c
+++ b/migration.c
@@ -371,7 +371,6 @@ void migrate_fd_put_ready(void *opaque)

     DPRINTF("iterate\n");
     if (qemu_savevm_state_iterate(s->mon, s->file) == 1) {
-        int state;
         int old_vm_running = runstate_is_running();

         DPRINTF("done iterating\n");
@@ -381,20 +380,18 @@ void migrate_fd_put_ready(void *opaque)
             if (old_vm_running) {
                 vm_start();
             }
-            state = MIG_STATE_ERROR;
-        } else {
-            state = MIG_STATE_COMPLETED;
+            s->state = MIG_STATE_ERROR;
         }
         if (migrate_fd_cleanup(s) < 0) {
             if (old_vm_running) {
                 vm_start();
             }
-            state = MIG_STATE_ERROR;
+            s->state = MIG_STATE_ERROR;
         }
-        if (state == MIG_STATE_COMPLETED) {
+        if (s->state == MIG_STATE_ACTIVE) {
+            s->state = MIG_STATE_COMPLETED;
             runstate_set(RUN_STATE_POSTMIGRATE);
         }
-        s->state = state;
         notifier_list_notify(&migration_state_notifiers, NULL);
     }
 }
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 02/36] migration: simplify state assignmente

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> Once there, make sure that if we already know that there is one error,
> just call migration_fd_cleanup() with the ERROR state.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   migration.c |   11 ++++-------
>   1 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 77a51ad..7fd6c23 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -371,7 +371,6 @@ void migrate_fd_put_ready(void *opaque)
>
>       DPRINTF("iterate\n");
>       if (qemu_savevm_state_iterate(s->mon, s->file) == 1) {
> -        int state;
>           int old_vm_running = runstate_is_running();
>
>           DPRINTF("done iterating\n");
> @@ -381,20 +380,18 @@ void migrate_fd_put_ready(void *opaque)
>               if (old_vm_running) {
>                   vm_start();
>               }
> -            state = MIG_STATE_ERROR;
> -        } else {
> -            state = MIG_STATE_COMPLETED;
> +            s->state = MIG_STATE_ERROR;
>           }
>           if (migrate_fd_cleanup(s)<  0) {
>               if (old_vm_running) {
>                   vm_start();
>               }
> -            state = MIG_STATE_ERROR;
> +            s->state = MIG_STATE_ERROR;
>           }
> -        if (state == MIG_STATE_COMPLETED) {
> +        if (s->state == MIG_STATE_ACTIVE) {
> +            s->state = MIG_STATE_COMPLETED;
>               runstate_set(RUN_STATE_POSTMIGRATE);
>           }
> -        s->state = state;
>           notifier_list_notify(&migration_state_notifiers, NULL);
>       }
>   }

[Qemu-devel] [PATCH 03/36] migration: Check that migration is active before cancel it

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration.c b/migration.c
index 7fd6c23..71b8aad 100644
--- a/migration.c
+++ b/migration.c
@@ -133,9 +133,9 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     MigrationState *s = current_migration;

-    if (s)
+    if (s && s->get_status(s) == MIG_STATE_ACTIVE) {
         s->cancel(s);
-
+    }
     return 0;
 }

-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 03/36] migration: Check that migration is active before cancel it

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   migration.c |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 7fd6c23..71b8aad 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -133,9 +133,9 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
>   {
>       MigrationState *s = current_migration;
>
> -    if (s)
> +    if (s&&  s->get_status(s) == MIG_STATE_ACTIVE) {
>           s->cancel(s);
> -
> +    }
>       return 0;
>   }
>

[Qemu-devel] [PATCH 04/36] migration: return real error code

[Juan Quintela]

make functions propagate errno, instead of just using -EIO.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |    6 +++++-
 savevm.c    |   33 +++++++++++++++------------------
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/migration.c b/migration.c
index 71b8aad..2d3a55b 100644
--- a/migration.c
+++ b/migration.c
@@ -363,6 +363,7 @@ void migrate_fd_connect(FdMigrationState *s)
 void migrate_fd_put_ready(void *opaque)
 {
     FdMigrationState *s = opaque;
+    int ret;

     if (s->state != MIG_STATE_ACTIVE) {
         DPRINTF("put_ready returning because of non-active state\n");
@@ -370,7 +371,10 @@ void migrate_fd_put_ready(void *opaque)
     }

     DPRINTF("iterate\n");
-    if (qemu_savevm_state_iterate(s->mon, s->file) == 1) {
+    ret = qemu_savevm_state_iterate(s->mon, s->file);
+    if (ret < 0) {
+        migrate_fd_error(s);
+    } else if (ret == 1) {
         int old_vm_running = runstate_is_running();

         DPRINTF("done iterating\n");
diff --git a/savevm.c b/savevm.c
index bf4d0e7..15c9c52 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1466,6 +1466,7 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable,
                             int shared)
 {
     SaveStateEntry *se;
+    int ret;

     QTAILQ_FOREACH(se, &savevm_handlers, entry) {
         if(se->set_params == NULL) {
@@ -1497,13 +1498,13 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable,

         se->save_live_state(mon, f, QEMU_VM_SECTION_START, se->opaque);
     }
-
-    if (qemu_file_has_error(f)) {
+    ret = qemu_file_has_error(f);
+    if (ret != 0) {
         qemu_savevm_state_cancel(mon, f);
-        return -EIO;
     }

-    return 0;
+    return ret;
+
 }

 int qemu_savevm_state_iterate(Monitor *mon, QEMUFile *f)
@@ -1528,16 +1529,14 @@ int qemu_savevm_state_iterate(Monitor *mon, QEMUFile *f)
             break;
         }
     }
-
-    if (ret)
-        return 1;
-
-    if (qemu_file_has_error(f)) {
+    if (ret != 0) {
+        return ret;
+    }
+    ret = qemu_file_has_error(f);
+    if (ret != 0) {
         qemu_savevm_state_cancel(mon, f);
-        return -EIO;
     }
-
-    return 0;
+    return ret;
 }

 int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f)
@@ -1580,10 +1579,7 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f)

     qemu_put_byte(f, QEMU_VM_EOF);

-    if (qemu_file_has_error(f))
-        return -EIO;
-
-    return 0;
+    return qemu_file_has_error(f);
 }

 void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f)
@@ -1623,8 +1619,9 @@ static int qemu_savevm_state(Monitor *mon, QEMUFile *f)
     ret = qemu_savevm_state_complete(mon, f);

 out:
-    if (qemu_file_has_error(f))
-        ret = -EIO;
+    if (ret == 0) {
+        ret = qemu_file_has_error(f);
+    }

     if (!ret && saved_vm_running)
         vm_start();
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 04/36] migration: return real error code

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> make functions propagate errno, instead of just using -EIO.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   migration.c |    6 +++++-
>   savevm.c    |   33 +++++++++++++++------------------
>   2 files changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 71b8aad..2d3a55b 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -363,6 +363,7 @@ void migrate_fd_connect(FdMigrationState *s)
>   void migrate_fd_put_ready(void *opaque)
>   {
>       FdMigrationState *s = opaque;
> +    int ret;
>
>       if (s->state != MIG_STATE_ACTIVE) {
>           DPRINTF("put_ready returning because of non-active state\n");
> @@ -370,7 +371,10 @@ void migrate_fd_put_ready(void *opaque)
>       }
>
>       DPRINTF("iterate\n");
> -    if (qemu_savevm_state_iterate(s->mon, s->file) == 1) {
> +    ret = qemu_savevm_state_iterate(s->mon, s->file);
> +    if (ret<  0) {
> +        migrate_fd_error(s);
> +    } else if (ret == 1) {
>           int old_vm_running = runstate_is_running();
>
>           DPRINTF("done iterating\n");
> diff --git a/savevm.c b/savevm.c
> index bf4d0e7..15c9c52 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1466,6 +1466,7 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable,
>                               int shared)
>   {
>       SaveStateEntry *se;
> +    int ret;
>
>       QTAILQ_FOREACH(se,&savevm_handlers, entry) {
>           if(se->set_params == NULL) {
> @@ -1497,13 +1498,13 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable,
>
>           se->save_live_state(mon, f, QEMU_VM_SECTION_START, se->opaque);
>       }
> -
> -    if (qemu_file_has_error(f)) {
> +    ret = qemu_file_has_error(f);
> +    if (ret != 0) {
>           qemu_savevm_state_cancel(mon, f);
> -        return -EIO;
>       }
>
> -    return 0;
> +    return ret;
> +
>   }
>
>   int qemu_savevm_state_iterate(Monitor *mon, QEMUFile *f)
> @@ -1528,16 +1529,14 @@ int qemu_savevm_state_iterate(Monitor *mon, QEMUFile *f)
>               break;
>           }
>       }
> -
> -    if (ret)
> -        return 1;
> -
> -    if (qemu_file_has_error(f)) {
> +    if (ret != 0) {
> +        return ret;
> +    }
> +    ret = qemu_file_has_error(f);
> +    if (ret != 0) {
>           qemu_savevm_state_cancel(mon, f);
> -        return -EIO;
>       }
> -
> -    return 0;
> +    return ret;
>   }
>
>   int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f)
> @@ -1580,10 +1579,7 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f)
>
>       qemu_put_byte(f, QEMU_VM_EOF);
>
> -    if (qemu_file_has_error(f))
> -        return -EIO;
> -
> -    return 0;
> +    return qemu_file_has_error(f);
>   }
>
>   void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f)
> @@ -1623,8 +1619,9 @@ static int qemu_savevm_state(Monitor *mon, QEMUFile *f)
>       ret = qemu_savevm_state_complete(mon, f);
>
>   out:
> -    if (qemu_file_has_error(f))
> -        ret = -EIO;
> +    if (ret == 0) {
> +        ret = qemu_file_has_error(f);
> +    }
>
>       if (!ret&&  saved_vm_running)
>           vm_start();

[Qemu-devel] [PATCH 05/36] migration: add error handling to migrate_fd_put_notify().

[Juan Quintela]

From: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>

Although migrate_fd_put_buffer() sets MIG_STATE_ERROR if it failed,
since migrate_fd_put_notify() isn't checking error of underlying
QEMUFile, those resources are kept open.  This patch checks it and
calls migrate_fd_error() in case of error.

Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/migration.c b/migration.c
index 2d3a55b..7ac1fc2 100644
--- a/migration.c
+++ b/migration.c
@@ -313,6 +313,9 @@ void migrate_fd_put_notify(void *opaque)

     qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
     qemu_file_put_notify(s->file);
+    if (qemu_file_has_error(s->file)) {
+        migrate_fd_error(s);
+    }
 }

 ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
@@ -329,9 +332,6 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)

     if (ret == -EAGAIN) {
         qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
-    } else if (ret < 0) {
-        s->state = MIG_STATE_ERROR;
-        notifier_list_notify(&migration_state_notifiers, NULL);
     }

     return ret;
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 05/36] migration: add error handling to migrate_fd_put_notify().

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> From: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp>
>
> Although migrate_fd_put_buffer() sets MIG_STATE_ERROR if it failed,
> since migrate_fd_put_notify() isn't checking error of underlying
> QEMUFile, those resources are kept open.  This patch checks it and
> calls migrate_fd_error() in case of error.
>
> Signed-off-by: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp>
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   migration.c |    6 +++---
>   1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 2d3a55b..7ac1fc2 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -313,6 +313,9 @@ void migrate_fd_put_notify(void *opaque)
>
>       qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
>       qemu_file_put_notify(s->file);
> +    if (qemu_file_has_error(s->file)) {
> +        migrate_fd_error(s);
> +    }
>   }
>
>   ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
> @@ -329,9 +332,6 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
>
>       if (ret == -EAGAIN) {
>           qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
> -    } else if (ret<  0) {
> -        s->state = MIG_STATE_ERROR;
> -        notifier_list_notify(&migration_state_notifiers, NULL);
>       }
>
>       return ret;

[Qemu-devel] [PATCH 06/36] migration: If there is one error, it makes no sense to continue

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 buffered_file.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index 486af57..bcdf04f 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -193,9 +193,9 @@ static int buffered_rate_limit(void *opaque)
 {
     QEMUFileBuffered *s = opaque;

-    if (s->has_error)
-        return 0;
-
+    if (s->has_error) {
+        return 1;
+    }
     if (s->freeze_output)
         return 1;

-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 06/36] migration: If there is one error, it makes no sense to continue

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela<quintela@redhat.com>

The original intention of returning zero was to force a quick finish of the 
migration.

I think this code makes things more brittle because now if you're not doing 
error checking in the throttling path, you'll just pause the migration forever 
instead of fast forwarding to a point where you're actually checking for errors.

Regards,

Anthony Liguori

> ---
>   buffered_file.c |    6 +++---
>   1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/buffered_file.c b/buffered_file.c
> index 486af57..bcdf04f 100644
> --- a/buffered_file.c
> +++ b/buffered_file.c
> @@ -193,9 +193,9 @@ static int buffered_rate_limit(void *opaque)
>   {
>       QEMUFileBuffered *s = opaque;
>
> -    if (s->has_error)
> -        return 0;
> -
> +    if (s->has_error) {
> +        return 1;
> +    }
>       if (s->freeze_output)
>           return 1;
>

Re: [Qemu-devel] [PATCH 06/36] migration: If there is one error, it makes no sense to continue

[Juan Quintela]

Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 10/11/2011 05:00 AM, Juan Quintela wrote:
>> Signed-off-by: Juan Quintela<quintela@redhat.com>
>
> The original intention of returning zero was to force a quick finish
> of the migration.
>
> I think this code makes things more brittle because now if you're not
> doing error checking in the throttling path, you'll just pause the
> migration forever instead of fast forwarding to a point where you're
> actually checking for errors.


This is the only caller:

    while (!qemu_file_rate_limit(f)) {
        int bytes_sent;

        bytes_sent = ram_save_block(f);
        bytes_transferred += bytes_sent;
        if (bytes_sent == 0) { /* no more blocks */
            break;
        }
    }

The error that I was finding is that whe get one error, we stay on that
loop a lot of time.  This change just made the error be discovered
instantanously.

Later, Juan.


>
> Regards,
>
> Anthony Liguori
>
>> ---
>>   buffered_file.c |    6 +++---
>>   1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/buffered_file.c b/buffered_file.c
>> index 486af57..bcdf04f 100644
>> --- a/buffered_file.c
>> +++ b/buffered_file.c
>> @@ -193,9 +193,9 @@ static int buffered_rate_limit(void *opaque)
>>   {
>>       QEMUFileBuffered *s = opaque;
>>
>> -    if (s->has_error)
>> -        return 0;
>> -
>> +    if (s->has_error) {
>> +        return 1;
>> +    }
>>       if (s->freeze_output)
>>           return 1;
>>

[Qemu-devel] [PATCH 07/36] buffered_file: Use right "opaque"

[Juan Quintela]

buffered_close 's' variable is of type QEMUFileBuffered, and
wait_for_unfreeze() expect to receive a MigrationState, that
'coincidentaly' is s->opaque.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 buffered_file.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index bcdf04f..701b440 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -176,7 +176,7 @@ static int buffered_close(void *opaque)
     while (!s->has_error && s->buffer_size) {
         buffered_flush(s);
         if (s->freeze_output)
-            s->wait_for_unfreeze(s);
+            s->wait_for_unfreeze(s->opaque);
     }

     ret = s->close(s->opaque);
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 07/36] buffered_file: Use right "opaque"

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> buffered_close 's' variable is of type QEMUFileBuffered, and
> wait_for_unfreeze() expect to receive a MigrationState, that
> 'coincidentaly' is s->opaque.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   buffered_file.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/buffered_file.c b/buffered_file.c
> index bcdf04f..701b440 100644
> --- a/buffered_file.c
> +++ b/buffered_file.c
> @@ -176,7 +176,7 @@ static int buffered_close(void *opaque)
>       while (!s->has_error&&  s->buffer_size) {
>           buffered_flush(s);
>           if (s->freeze_output)
> -            s->wait_for_unfreeze(s);
> +            s->wait_for_unfreeze(s->opaque);
>       }
>
>       ret = s->close(s->opaque);

[Qemu-devel] [PATCH 08/36] buffered_file: reuse QEMUFile has_error field

[Juan Quintela]

Instead of having two has_error fields in QEMUFile & QEMUBufferedFile,
reuse the 1st one.  Notice that the one in buffered_file is only set
after a file operation.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 buffered_file.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index 701b440..3dadec0 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -27,7 +27,6 @@ typedef struct QEMUFileBuffered
     BufferedCloseFunc *close;
     void *opaque;
     QEMUFile *file;
-    int has_error;
     int freeze_output;
     size_t bytes_xfer;
     size_t xfer_limit;
@@ -73,7 +72,7 @@ static void buffered_flush(QEMUFileBuffered *s)
 {
     size_t offset = 0;

-    if (s->has_error) {
+    if (qemu_file_has_error(s->file)) {
         DPRINTF("flush when error, bailing\n");
         return;
     }
@@ -93,7 +92,7 @@ static void buffered_flush(QEMUFileBuffered *s)

         if (ret <= 0) {
             DPRINTF("error flushing data, %zd\n", ret);
-            s->has_error = 1;
+            qemu_file_set_error(s->file);
             break;
         } else {
             DPRINTF("flushed %zd byte(s)\n", ret);
@@ -114,7 +113,7 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in

     DPRINTF("putting %d bytes at %" PRId64 "\n", size, pos);

-    if (s->has_error) {
+    if (qemu_file_has_error(s->file)) {
         DPRINTF("flush when error, bailing\n");
         return -EINVAL;
     }
@@ -139,7 +138,7 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in

         if (ret <= 0) {
             DPRINTF("error putting\n");
-            s->has_error = 1;
+            qemu_file_set_error(s->file);
             offset = -EINVAL;
             break;
         }
@@ -173,7 +172,7 @@ static int buffered_close(void *opaque)

     DPRINTF("closing\n");

-    while (!s->has_error && s->buffer_size) {
+    while (!qemu_file_has_error(s->file) && s->buffer_size) {
         buffered_flush(s);
         if (s->freeze_output)
             s->wait_for_unfreeze(s->opaque);
@@ -193,7 +192,7 @@ static int buffered_rate_limit(void *opaque)
 {
     QEMUFileBuffered *s = opaque;

-    if (s->has_error) {
+    if (qemu_file_has_error(s->file)) {
         return 1;
     }
     if (s->freeze_output)
@@ -208,7 +207,7 @@ static int buffered_rate_limit(void *opaque)
 static int64_t buffered_set_rate_limit(void *opaque, int64_t new_rate)
 {
     QEMUFileBuffered *s = opaque;
-    if (s->has_error)
+    if (qemu_file_has_error(s->file))
         goto out;

     if (new_rate > SIZE_MAX) {
@@ -232,7 +231,7 @@ static void buffered_rate_tick(void *opaque)
 {
     QEMUFileBuffered *s = opaque;

-    if (s->has_error) {
+    if (qemu_file_has_error(s->file)) {
         buffered_close(s);
         return;
     }
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 08/36] buffered_file: reuse QEMUFile has_error field

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> Instead of having two has_error fields in QEMUFile&  QEMUBufferedFile,
> reuse the 1st one.  Notice that the one in buffered_file is only set
> after a file operation.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   buffered_file.c |   17 ++++++++---------
>   1 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/buffered_file.c b/buffered_file.c
> index 701b440..3dadec0 100644
> --- a/buffered_file.c
> +++ b/buffered_file.c
> @@ -27,7 +27,6 @@ typedef struct QEMUFileBuffered
>       BufferedCloseFunc *close;
>       void *opaque;
>       QEMUFile *file;
> -    int has_error;
>       int freeze_output;
>       size_t bytes_xfer;
>       size_t xfer_limit;
> @@ -73,7 +72,7 @@ static void buffered_flush(QEMUFileBuffered *s)
>   {
>       size_t offset = 0;
>
> -    if (s->has_error) {
> +    if (qemu_file_has_error(s->file)) {
>           DPRINTF("flush when error, bailing\n");
>           return;
>       }
> @@ -93,7 +92,7 @@ static void buffered_flush(QEMUFileBuffered *s)
>
>           if (ret<= 0) {
>               DPRINTF("error flushing data, %zd\n", ret);
> -            s->has_error = 1;
> +            qemu_file_set_error(s->file);
>               break;
>           } else {
>               DPRINTF("flushed %zd byte(s)\n", ret);
> @@ -114,7 +113,7 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
>
>       DPRINTF("putting %d bytes at %" PRId64 "\n", size, pos);
>
> -    if (s->has_error) {
> +    if (qemu_file_has_error(s->file)) {
>           DPRINTF("flush when error, bailing\n");
>           return -EINVAL;
>       }
> @@ -139,7 +138,7 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
>
>           if (ret<= 0) {
>               DPRINTF("error putting\n");
> -            s->has_error = 1;
> +            qemu_file_set_error(s->file);
>               offset = -EINVAL;
>               break;
>           }
> @@ -173,7 +172,7 @@ static int buffered_close(void *opaque)
>
>       DPRINTF("closing\n");
>
> -    while (!s->has_error&&  s->buffer_size) {
> +    while (!qemu_file_has_error(s->file)&&  s->buffer_size) {
>           buffered_flush(s);
>           if (s->freeze_output)
>               s->wait_for_unfreeze(s->opaque);
> @@ -193,7 +192,7 @@ static int buffered_rate_limit(void *opaque)
>   {
>       QEMUFileBuffered *s = opaque;
>
> -    if (s->has_error) {
> +    if (qemu_file_has_error(s->file)) {
>           return 1;
>       }
>       if (s->freeze_output)
> @@ -208,7 +207,7 @@ static int buffered_rate_limit(void *opaque)
>   static int64_t buffered_set_rate_limit(void *opaque, int64_t new_rate)
>   {
>       QEMUFileBuffered *s = opaque;
> -    if (s->has_error)
> +    if (qemu_file_has_error(s->file))
>           goto out;
>
>       if (new_rate>  SIZE_MAX) {
> @@ -232,7 +231,7 @@ static void buffered_rate_tick(void *opaque)
>   {
>       QEMUFileBuffered *s = opaque;
>
> -    if (s->has_error) {
> +    if (qemu_file_has_error(s->file)) {
>           buffered_close(s);
>           return;
>       }

[Qemu-devel] [PATCH 09/36] migration: don't "write" when migration is not active

[Juan Quintela]

If migration is not active, just ignore writes.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/migration.c b/migration.c
index 7ac1fc2..090c925 100644
--- a/migration.c
+++ b/migration.c
@@ -323,6 +323,10 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
     FdMigrationState *s = opaque;
     ssize_t ret;

+    if (s->state != MIG_STATE_ACTIVE) {
+        return -EIO;
+    }
+
     do {
         ret = s->write(s, data, size);
     } while (ret == -1 && ((s->get_error(s)) == EINTR));
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 09/36] migration: don't "write" when migration is not active

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> If migration is not active, just ignore writes.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>
> ---
>   migration.c |    4 ++++
>   1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 7ac1fc2..090c925 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -323,6 +323,10 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
>       FdMigrationState *s = opaque;
>       ssize_t ret;
>
> +    if (s->state != MIG_STATE_ACTIVE) {
> +        return -EIO;
> +    }
> +

Buffered file is buffered.  The migration may complete before the buffer is 
completely drained.  That means additional put_buffer calls may come after the 
migration state has moved to complete.

Regards,

Anthony Liguori

>       do {
>           ret = s->write(s, data, size);
>       } while (ret == -1&&  ((s->get_error(s)) == EINTR));

Re: [Qemu-devel] [PATCH 09/36] migration: don't "write" when migration is not active

[Juan Quintela]

Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 10/11/2011 05:00 AM, Juan Quintela wrote:
>> If migration is not active, just ignore writes.
>>
>> Signed-off-by: Juan Quintela<quintela@redhat.com>
>> ---
>>   migration.c |    4 ++++
>>   1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/migration.c b/migration.c
>> index 7ac1fc2..090c925 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -323,6 +323,10 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
>>       FdMigrationState *s = opaque;
>>       ssize_t ret;
>>
>> +    if (s->state != MIG_STATE_ACTIVE) {
>> +        return -EIO;
>> +    }
>> +
>
> Buffered file is buffered.  The migration may complete before the
> buffer is completely drained.  That means additional put_buffer calls
> may come after the migration state has moved to complete.

static void migrate_fd_completed(MigrationState *s)
{
    DPRINTF("setting completed state\n");
    if (migrate_fd_cleanup(s) < 0) {
        s->state = MIG_STATE_ERROR;
    } else {
        s->state = MIG_STATE_COMPLETED;
        runstate_set(RUN_STATE_POSTMIGRATE);
    }
    notifier_list_notify(&migration_state_notifiers, s);
}

After all the changes in this thread, that is the only place that can
change s->state to "MIG_STATE_COMPLETED", as you can see, we do that
after doing a migrate_fd_cleanup(), and that just flush + close the fd.

So, I still think that everything is done as expected. (testing confirms
that both migrate_cancel after one error don't hang and migration
without errors ends as expected.

Later, Juan.

> Regards,
> Anthony Liguori
>
>>       do {
>>           ret = s->write(s, data, size);
>>       } while (ret == -1&&  ((s->get_error(s)) == EINTR));

[Qemu-devel] [PATCH 10/36] migration: set error if select return one error

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/migration.c b/migration.c
index 090c925..56c2b1c 100644
--- a/migration.c
+++ b/migration.c
@@ -457,6 +457,10 @@ void migrate_fd_wait_for_unfreeze(void *opaque)

         ret = select(s->fd + 1, NULL, &wfds, NULL, NULL);
     } while (ret == -1 && (s->get_error(s)) == EINTR);
+
+    if (ret == -1) {
+        qemu_file_set_error(s->file);
+    }
 }

 int migrate_fd_close(void *opaque)
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 10/36] migration: set error if select return one error

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   migration.c |    4 ++++
>   1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 090c925..56c2b1c 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -457,6 +457,10 @@ void migrate_fd_wait_for_unfreeze(void *opaque)
>
>           ret = select(s->fd + 1, NULL,&wfds, NULL, NULL);
>       } while (ret == -1&&  (s->get_error(s)) == EINTR);
> +
> +    if (ret == -1) {
> +        qemu_file_set_error(s->file);
> +    }
>   }
>
>   int migrate_fd_close(void *opaque)

[Qemu-devel] [PATCH 11/36] migration: change has_error to contain errno values

[Juan Quintela]

We normally already have an errno value.  When not, abuse EINVAL.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c       |    2 +-
 block-migration.c |    6 +++---
 buffered_file.c   |    4 ++--
 hw/hw.h           |    2 +-
 migration.c       |    2 +-
 savevm.c          |    8 ++++----
 6 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index a6c69c7..941d585 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -263,7 +263,7 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
     }

     if (cpu_physical_sync_dirty_bitmap(0, TARGET_PHYS_ADDR_MAX) != 0) {
-        qemu_file_set_error(f);
+        qemu_file_set_error(f, -EINVAL);
         return 0;
     }

diff --git a/block-migration.c b/block-migration.c
index e2775ee..2638f51 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -263,7 +263,7 @@ static int mig_save_device_bulk(Monitor *mon, QEMUFile *f,

 error:
     monitor_printf(mon, "Error reading sector %" PRId64 "\n", cur_sector);
-    qemu_file_set_error(f);
+    qemu_file_set_error(f, -EINVAL);
     g_free(blk->buf);
     g_free(blk);
     return 0;
@@ -439,7 +439,7 @@ static int mig_save_device_dirty(Monitor *mon, QEMUFile *f,

 error:
     monitor_printf(mon, "Error reading sector %" PRId64 "\n", sector);
-    qemu_file_set_error(f);
+    qemu_file_set_error(f, -EINVAL);
     g_free(blk->buf);
     g_free(blk);
     return 0;
@@ -473,7 +473,7 @@ static void flush_blks(QEMUFile* f)
             break;
         }
         if (blk->ret < 0) {
-            qemu_file_set_error(f);
+            qemu_file_set_error(f, -EINVAL);
             break;
         }
         blk_send(f, blk);
diff --git a/buffered_file.c b/buffered_file.c
index 3dadec0..3e5333c 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -92,7 +92,7 @@ static void buffered_flush(QEMUFileBuffered *s)

         if (ret <= 0) {
             DPRINTF("error flushing data, %zd\n", ret);
-            qemu_file_set_error(s->file);
+            qemu_file_set_error(s->file, ret);
             break;
         } else {
             DPRINTF("flushed %zd byte(s)\n", ret);
@@ -138,7 +138,7 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in

         if (ret <= 0) {
             DPRINTF("error putting\n");
-            qemu_file_set_error(s->file);
+            qemu_file_set_error(s->file, ret);
             offset = -EINVAL;
             break;
         }
diff --git a/hw/hw.h b/hw/hw.h
index a124da9..6cf8cd2 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -86,7 +86,7 @@ int qemu_file_rate_limit(QEMUFile *f);
 int64_t qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
 int64_t qemu_file_get_rate_limit(QEMUFile *f);
 int qemu_file_has_error(QEMUFile *f);
-void qemu_file_set_error(QEMUFile *f);
+void qemu_file_set_error(QEMUFile *f, int error);

 /* Try to send any outstanding data.  This function is useful when output is
  * halted due to rate limiting or EAGAIN errors occur as it can be used to
diff --git a/migration.c b/migration.c
index 56c2b1c..541da98 100644
--- a/migration.c
+++ b/migration.c
@@ -459,7 +459,7 @@ void migrate_fd_wait_for_unfreeze(void *opaque)
     } while (ret == -1 && (s->get_error(s)) == EINTR);

     if (ret == -1) {
-        qemu_file_set_error(s->file);
+        qemu_file_set_error(s->file, s->get_error(s));
     }
 }

diff --git a/savevm.c b/savevm.c
index 15c9c52..8bc7272 100644
--- a/savevm.c
+++ b/savevm.c
@@ -430,9 +430,9 @@ int qemu_file_has_error(QEMUFile *f)
     return f->has_error;
 }

-void qemu_file_set_error(QEMUFile *f)
+void qemu_file_set_error(QEMUFile *f, int ret)
 {
-    f->has_error = 1;
+    f->has_error = ret;
 }

 void qemu_fflush(QEMUFile *f)
@@ -447,7 +447,7 @@ void qemu_fflush(QEMUFile *f)
         if (len > 0)
             f->buf_offset += f->buf_index;
         else
-            f->has_error = 1;
+            f->has_error = -EINVAL;
         f->buf_index = 0;
     }
 }
@@ -468,7 +468,7 @@ static void qemu_fill_buffer(QEMUFile *f)
         f->buf_size = len;
         f->buf_offset += len;
     } else if (len != -EAGAIN)
-        f->has_error = 1;
+        f->has_error = len;
 }

 int qemu_fclose(QEMUFile *f)
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 11/36] migration: change has_error to contain errno values

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> We normally already have an errno value.  When not, abuse EINVAL.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   arch_init.c       |    2 +-
>   block-migration.c |    6 +++---
>   buffered_file.c   |    4 ++--
>   hw/hw.h           |    2 +-
>   migration.c       |    2 +-
>   savevm.c          |    8 ++++----
>   6 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index a6c69c7..941d585 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -263,7 +263,7 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
>       }
>
>       if (cpu_physical_sync_dirty_bitmap(0, TARGET_PHYS_ADDR_MAX) != 0) {
> -        qemu_file_set_error(f);
> +        qemu_file_set_error(f, -EINVAL);
>           return 0;
>       }
>
> diff --git a/block-migration.c b/block-migration.c
> index e2775ee..2638f51 100644
> --- a/block-migration.c
> +++ b/block-migration.c
> @@ -263,7 +263,7 @@ static int mig_save_device_bulk(Monitor *mon, QEMUFile *f,
>
>   error:
>       monitor_printf(mon, "Error reading sector %" PRId64 "\n", cur_sector);
> -    qemu_file_set_error(f);
> +    qemu_file_set_error(f, -EINVAL);
>       g_free(blk->buf);
>       g_free(blk);
>       return 0;
> @@ -439,7 +439,7 @@ static int mig_save_device_dirty(Monitor *mon, QEMUFile *f,
>
>   error:
>       monitor_printf(mon, "Error reading sector %" PRId64 "\n", sector);
> -    qemu_file_set_error(f);
> +    qemu_file_set_error(f, -EINVAL);
>       g_free(blk->buf);
>       g_free(blk);
>       return 0;
> @@ -473,7 +473,7 @@ static void flush_blks(QEMUFile* f)
>               break;
>           }
>           if (blk->ret<  0) {
> -            qemu_file_set_error(f);
> +            qemu_file_set_error(f, -EINVAL);
>               break;
>           }
>           blk_send(f, blk);
> diff --git a/buffered_file.c b/buffered_file.c
> index 3dadec0..3e5333c 100644
> --- a/buffered_file.c
> +++ b/buffered_file.c
> @@ -92,7 +92,7 @@ static void buffered_flush(QEMUFileBuffered *s)
>
>           if (ret<= 0) {
>               DPRINTF("error flushing data, %zd\n", ret);
> -            qemu_file_set_error(s->file);
> +            qemu_file_set_error(s->file, ret);
>               break;
>           } else {
>               DPRINTF("flushed %zd byte(s)\n", ret);
> @@ -138,7 +138,7 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
>
>           if (ret<= 0) {
>               DPRINTF("error putting\n");
> -            qemu_file_set_error(s->file);
> +            qemu_file_set_error(s->file, ret);
>               offset = -EINVAL;
>               break;
>           }
> diff --git a/hw/hw.h b/hw/hw.h
> index a124da9..6cf8cd2 100644
> --- a/hw/hw.h
> +++ b/hw/hw.h
> @@ -86,7 +86,7 @@ int qemu_file_rate_limit(QEMUFile *f);
>   int64_t qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
>   int64_t qemu_file_get_rate_limit(QEMUFile *f);
>   int qemu_file_has_error(QEMUFile *f);
> -void qemu_file_set_error(QEMUFile *f);
> +void qemu_file_set_error(QEMUFile *f, int error);
>
>   /* Try to send any outstanding data.  This function is useful when output is
>    * halted due to rate limiting or EAGAIN errors occur as it can be used to
> diff --git a/migration.c b/migration.c
> index 56c2b1c..541da98 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -459,7 +459,7 @@ void migrate_fd_wait_for_unfreeze(void *opaque)
>       } while (ret == -1&&  (s->get_error(s)) == EINTR);
>
>       if (ret == -1) {
> -        qemu_file_set_error(s->file);
> +        qemu_file_set_error(s->file, s->get_error(s));
>       }
>   }
>
> diff --git a/savevm.c b/savevm.c
> index 15c9c52..8bc7272 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -430,9 +430,9 @@ int qemu_file_has_error(QEMUFile *f)
>       return f->has_error;
>   }
>
> -void qemu_file_set_error(QEMUFile *f)
> +void qemu_file_set_error(QEMUFile *f, int ret)
>   {
> -    f->has_error = 1;
> +    f->has_error = ret;
>   }
>
>   void qemu_fflush(QEMUFile *f)
> @@ -447,7 +447,7 @@ void qemu_fflush(QEMUFile *f)
>           if (len>  0)
>               f->buf_offset += f->buf_index;
>           else
> -            f->has_error = 1;
> +            f->has_error = -EINVAL;
>           f->buf_index = 0;
>       }
>   }
> @@ -468,7 +468,7 @@ static void qemu_fill_buffer(QEMUFile *f)
>           f->buf_size = len;
>           f->buf_offset += len;
>       } else if (len != -EAGAIN)
> -        f->has_error = 1;
> +        f->has_error = len;
>   }
>
>   int qemu_fclose(QEMUFile *f)

[Qemu-devel] [PATCH 12/36] migration: rename qemu_file_has_error to qemu_file_get_error

[Juan Quintela]

Now the function returned errno, so it is better the new name.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c       |    2 +-
 block-migration.c |    8 ++++----
 buffered_file.c   |   14 +++++++-------
 hw/hw.h           |    2 +-
 migration.c       |    2 +-
 savevm.c          |   13 +++++++------
 6 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 941d585..9128be0 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -451,7 +451,7 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)

             qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
         }
-        if (qemu_file_has_error(f)) {
+        if (qemu_file_get_error(f)) {
             return -EIO;
         }
     } while (!(flags & RAM_SAVE_FLAG_EOS));
diff --git a/block-migration.c b/block-migration.c
index 2638f51..8308753 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -579,7 +579,7 @@ static int block_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)

     flush_blks(f);

-    if (qemu_file_has_error(f)) {
+    if (qemu_file_get_error(f)) {
         blk_mig_cleanup(mon);
         return 0;
     }
@@ -607,7 +607,7 @@ static int block_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)

         flush_blks(f);

-        if (qemu_file_has_error(f)) {
+        if (qemu_file_get_error(f)) {
             blk_mig_cleanup(mon);
             return 0;
         }
@@ -624,7 +624,7 @@ static int block_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
         /* report completion */
         qemu_put_be64(f, (100 << BDRV_SECTOR_BITS) | BLK_MIG_FLAG_PROGRESS);

-        if (qemu_file_has_error(f)) {
+        if (qemu_file_get_error(f)) {
             return 0;
         }

@@ -704,7 +704,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
             fprintf(stderr, "Unknown flags\n");
             return -EINVAL;
         }
-        if (qemu_file_has_error(f)) {
+        if (qemu_file_get_error(f)) {
             return -EIO;
         }
     } while (!(flags & BLK_MIG_FLAG_EOS));
diff --git a/buffered_file.c b/buffered_file.c
index 3e5333c..82f4001 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -72,7 +72,7 @@ static void buffered_flush(QEMUFileBuffered *s)
 {
     size_t offset = 0;

-    if (qemu_file_has_error(s->file)) {
+    if (qemu_file_get_error(s->file)) {
         DPRINTF("flush when error, bailing\n");
         return;
     }
@@ -113,7 +113,7 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in

     DPRINTF("putting %d bytes at %" PRId64 "\n", size, pos);

-    if (qemu_file_has_error(s->file)) {
+    if (qemu_file_get_error(s->file)) {
         DPRINTF("flush when error, bailing\n");
         return -EINVAL;
     }
@@ -172,7 +172,7 @@ static int buffered_close(void *opaque)

     DPRINTF("closing\n");

-    while (!qemu_file_has_error(s->file) && s->buffer_size) {
+    while (!qemu_file_get_error(s->file) && s->buffer_size) {
         buffered_flush(s);
         if (s->freeze_output)
             s->wait_for_unfreeze(s->opaque);
@@ -192,7 +192,7 @@ static int buffered_rate_limit(void *opaque)
 {
     QEMUFileBuffered *s = opaque;

-    if (qemu_file_has_error(s->file)) {
+    if (qemu_file_get_error(s->file)) {
         return 1;
     }
     if (s->freeze_output)
@@ -207,9 +207,9 @@ static int buffered_rate_limit(void *opaque)
 static int64_t buffered_set_rate_limit(void *opaque, int64_t new_rate)
 {
     QEMUFileBuffered *s = opaque;
-    if (qemu_file_has_error(s->file))
+    if (qemu_file_get_error(s->file)) {
         goto out;
-
+    }
     if (new_rate > SIZE_MAX) {
         new_rate = SIZE_MAX;
     }
@@ -231,7 +231,7 @@ static void buffered_rate_tick(void *opaque)
 {
     QEMUFileBuffered *s = opaque;

-    if (qemu_file_has_error(s->file)) {
+    if (qemu_file_get_error(s->file)) {
         buffered_close(s);
         return;
     }
diff --git a/hw/hw.h b/hw/hw.h
index 6cf8cd2..ed20f5a 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -85,7 +85,7 @@ uint64_t qemu_get_be64(QEMUFile *f);
 int qemu_file_rate_limit(QEMUFile *f);
 int64_t qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
 int64_t qemu_file_get_rate_limit(QEMUFile *f);
-int qemu_file_has_error(QEMUFile *f);
+int qemu_file_get_error(QEMUFile *f);
 void qemu_file_set_error(QEMUFile *f, int error);

 /* Try to send any outstanding data.  This function is useful when output is
diff --git a/migration.c b/migration.c
index 541da98..6bac734 100644
--- a/migration.c
+++ b/migration.c
@@ -313,7 +313,7 @@ void migrate_fd_put_notify(void *opaque)

     qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
     qemu_file_put_notify(s->file);
-    if (qemu_file_has_error(s->file)) {
+    if (qemu_file_get_error(s->file)) {
         migrate_fd_error(s);
     }
 }
diff --git a/savevm.c b/savevm.c
index 8bc7272..c037eb3 100644
--- a/savevm.c
+++ b/savevm.c
@@ -425,7 +425,7 @@ QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
     return f;
 }

-int qemu_file_has_error(QEMUFile *f)
+int qemu_file_get_error(QEMUFile *f)
 {
     return f->has_error;
 }
@@ -1498,7 +1498,7 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable,

         se->save_live_state(mon, f, QEMU_VM_SECTION_START, se->opaque);
     }
-    ret = qemu_file_has_error(f);
+    ret = qemu_file_get_error(f);
     if (ret != 0) {
         qemu_savevm_state_cancel(mon, f);
     }
@@ -1532,7 +1532,7 @@ int qemu_savevm_state_iterate(Monitor *mon, QEMUFile *f)
     if (ret != 0) {
         return ret;
     }
-    ret = qemu_file_has_error(f);
+    ret = qemu_file_get_error(f);
     if (ret != 0) {
         qemu_savevm_state_cancel(mon, f);
     }
@@ -1579,7 +1579,7 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f)

     qemu_put_byte(f, QEMU_VM_EOF);

-    return qemu_file_has_error(f);
+    return qemu_file_get_error(f);
 }

 void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f)
@@ -1620,7 +1620,7 @@ static int qemu_savevm_state(Monitor *mon, QEMUFile *f)

 out:
     if (ret == 0) {
-        ret = qemu_file_has_error(f);
+        ret = qemu_file_get_error(f);
     }

     if (!ret && saved_vm_running)
@@ -1835,8 +1835,9 @@ out:
         g_free(le);
     }

-    if (qemu_file_has_error(f))
+    if (qemu_file_get_error(f)) {
         ret = -EIO;
+    }

     return ret;
 }
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 12/36] migration: rename qemu_file_has_error to qemu_file_get_error

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> Now the function returned errno, so it is better the new name.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   arch_init.c       |    2 +-
>   block-migration.c |    8 ++++----
>   buffered_file.c   |   14 +++++++-------
>   hw/hw.h           |    2 +-
>   migration.c       |    2 +-
>   savevm.c          |   13 +++++++------
>   6 files changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 941d585..9128be0 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -451,7 +451,7 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)
>
>               qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
>           }
> -        if (qemu_file_has_error(f)) {
> +        if (qemu_file_get_error(f)) {
>               return -EIO;
>           }
>       } while (!(flags&  RAM_SAVE_FLAG_EOS));
> diff --git a/block-migration.c b/block-migration.c
> index 2638f51..8308753 100644
> --- a/block-migration.c
> +++ b/block-migration.c
> @@ -579,7 +579,7 @@ static int block_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
>
>       flush_blks(f);
>
> -    if (qemu_file_has_error(f)) {
> +    if (qemu_file_get_error(f)) {
>           blk_mig_cleanup(mon);
>           return 0;
>       }
> @@ -607,7 +607,7 @@ static int block_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
>
>           flush_blks(f);
>
> -        if (qemu_file_has_error(f)) {
> +        if (qemu_file_get_error(f)) {
>               blk_mig_cleanup(mon);
>               return 0;
>           }
> @@ -624,7 +624,7 @@ static int block_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
>           /* report completion */
>           qemu_put_be64(f, (100<<  BDRV_SECTOR_BITS) | BLK_MIG_FLAG_PROGRESS);
>
> -        if (qemu_file_has_error(f)) {
> +        if (qemu_file_get_error(f)) {
>               return 0;
>           }
>
> @@ -704,7 +704,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
>               fprintf(stderr, "Unknown flags\n");
>               return -EINVAL;
>           }
> -        if (qemu_file_has_error(f)) {
> +        if (qemu_file_get_error(f)) {
>               return -EIO;
>           }
>       } while (!(flags&  BLK_MIG_FLAG_EOS));
> diff --git a/buffered_file.c b/buffered_file.c
> index 3e5333c..82f4001 100644
> --- a/buffered_file.c
> +++ b/buffered_file.c
> @@ -72,7 +72,7 @@ static void buffered_flush(QEMUFileBuffered *s)
>   {
>       size_t offset = 0;
>
> -    if (qemu_file_has_error(s->file)) {
> +    if (qemu_file_get_error(s->file)) {
>           DPRINTF("flush when error, bailing\n");
>           return;
>       }
> @@ -113,7 +113,7 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
>
>       DPRINTF("putting %d bytes at %" PRId64 "\n", size, pos);
>
> -    if (qemu_file_has_error(s->file)) {
> +    if (qemu_file_get_error(s->file)) {
>           DPRINTF("flush when error, bailing\n");
>           return -EINVAL;
>       }
> @@ -172,7 +172,7 @@ static int buffered_close(void *opaque)
>
>       DPRINTF("closing\n");
>
> -    while (!qemu_file_has_error(s->file)&&  s->buffer_size) {
> +    while (!qemu_file_get_error(s->file)&&  s->buffer_size) {
>           buffered_flush(s);
>           if (s->freeze_output)
>               s->wait_for_unfreeze(s->opaque);
> @@ -192,7 +192,7 @@ static int buffered_rate_limit(void *opaque)
>   {
>       QEMUFileBuffered *s = opaque;
>
> -    if (qemu_file_has_error(s->file)) {
> +    if (qemu_file_get_error(s->file)) {
>           return 1;
>       }
>       if (s->freeze_output)
> @@ -207,9 +207,9 @@ static int buffered_rate_limit(void *opaque)
>   static int64_t buffered_set_rate_limit(void *opaque, int64_t new_rate)
>   {
>       QEMUFileBuffered *s = opaque;
> -    if (qemu_file_has_error(s->file))
> +    if (qemu_file_get_error(s->file)) {
>           goto out;
> -
> +    }
>       if (new_rate>  SIZE_MAX) {
>           new_rate = SIZE_MAX;
>       }
> @@ -231,7 +231,7 @@ static void buffered_rate_tick(void *opaque)
>   {
>       QEMUFileBuffered *s = opaque;
>
> -    if (qemu_file_has_error(s->file)) {
> +    if (qemu_file_get_error(s->file)) {
>           buffered_close(s);
>           return;
>       }
> diff --git a/hw/hw.h b/hw/hw.h
> index 6cf8cd2..ed20f5a 100644
> --- a/hw/hw.h
> +++ b/hw/hw.h
> @@ -85,7 +85,7 @@ uint64_t qemu_get_be64(QEMUFile *f);
>   int qemu_file_rate_limit(QEMUFile *f);
>   int64_t qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
>   int64_t qemu_file_get_rate_limit(QEMUFile *f);
> -int qemu_file_has_error(QEMUFile *f);
> +int qemu_file_get_error(QEMUFile *f);
>   void qemu_file_set_error(QEMUFile *f, int error);
>
>   /* Try to send any outstanding data.  This function is useful when output is
> diff --git a/migration.c b/migration.c
> index 541da98..6bac734 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -313,7 +313,7 @@ void migrate_fd_put_notify(void *opaque)
>
>       qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
>       qemu_file_put_notify(s->file);
> -    if (qemu_file_has_error(s->file)) {
> +    if (qemu_file_get_error(s->file)) {
>           migrate_fd_error(s);
>       }
>   }
> diff --git a/savevm.c b/savevm.c
> index 8bc7272..c037eb3 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -425,7 +425,7 @@ QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
>       return f;
>   }
>
> -int qemu_file_has_error(QEMUFile *f)
> +int qemu_file_get_error(QEMUFile *f)
>   {
>       return f->has_error;
>   }
> @@ -1498,7 +1498,7 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable,
>
>           se->save_live_state(mon, f, QEMU_VM_SECTION_START, se->opaque);
>       }
> -    ret = qemu_file_has_error(f);
> +    ret = qemu_file_get_error(f);
>       if (ret != 0) {
>           qemu_savevm_state_cancel(mon, f);
>       }
> @@ -1532,7 +1532,7 @@ int qemu_savevm_state_iterate(Monitor *mon, QEMUFile *f)
>       if (ret != 0) {
>           return ret;
>       }
> -    ret = qemu_file_has_error(f);
> +    ret = qemu_file_get_error(f);
>       if (ret != 0) {
>           qemu_savevm_state_cancel(mon, f);
>       }
> @@ -1579,7 +1579,7 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f)
>
>       qemu_put_byte(f, QEMU_VM_EOF);
>
> -    return qemu_file_has_error(f);
> +    return qemu_file_get_error(f);
>   }
>
>   void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f)
> @@ -1620,7 +1620,7 @@ static int qemu_savevm_state(Monitor *mon, QEMUFile *f)
>
>   out:
>       if (ret == 0) {
> -        ret = qemu_file_has_error(f);
> +        ret = qemu_file_get_error(f);
>       }
>
>       if (!ret&&  saved_vm_running)
> @@ -1835,8 +1835,9 @@ out:
>           g_free(le);
>       }
>
> -    if (qemu_file_has_error(f))
> +    if (qemu_file_get_error(f)) {
>           ret = -EIO;
> +    }
>
>       return ret;
>   }

[Qemu-devel] [PATCH 13/36] savevm: Rename has_error to last_error field

[Juan Quintela]

Now the field contains the last error name, so rename acordingly.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 savevm.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/savevm.c b/savevm.c
index c037eb3..69a2ccd 100644
--- a/savevm.c
+++ b/savevm.c
@@ -173,7 +173,7 @@ struct QEMUFile {
     int buf_size; /* 0 when writing */
     uint8_t buf[IO_BUF_SIZE];

-    int has_error;
+    int last_error;
 };

 typedef struct QEMUFileStdio
@@ -427,12 +427,12 @@ QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,

 int qemu_file_get_error(QEMUFile *f)
 {
-    return f->has_error;
+    return f->last_error;
 }

 void qemu_file_set_error(QEMUFile *f, int ret)
 {
-    f->has_error = ret;
+    f->last_error = ret;
 }

 void qemu_fflush(QEMUFile *f)
@@ -447,7 +447,7 @@ void qemu_fflush(QEMUFile *f)
         if (len > 0)
             f->buf_offset += f->buf_index;
         else
-            f->has_error = -EINVAL;
+            f->last_error = -EINVAL;
         f->buf_index = 0;
     }
 }
@@ -468,7 +468,7 @@ static void qemu_fill_buffer(QEMUFile *f)
         f->buf_size = len;
         f->buf_offset += len;
     } else if (len != -EAGAIN)
-        f->has_error = len;
+        f->last_error = len;
 }

 int qemu_fclose(QEMUFile *f)
@@ -490,13 +490,13 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
 {
     int l;

-    if (!f->has_error && f->is_write == 0 && f->buf_index > 0) {
+    if (!f->last_error && f->is_write == 0 && f->buf_index > 0) {
         fprintf(stderr,
                 "Attempted to write to buffer while read buffer is not empty\n");
         abort();
     }

-    while (!f->has_error && size > 0) {
+    while (!f->last_error && size > 0) {
         l = IO_BUF_SIZE - f->buf_index;
         if (l > size)
             l = size;
@@ -512,7 +512,7 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)

 void qemu_put_byte(QEMUFile *f, int v)
 {
-    if (!f->has_error && f->is_write == 0 && f->buf_index > 0) {
+    if (!f->last_error && f->is_write == 0 && f->buf_index > 0) {
         fprintf(stderr,
                 "Attempted to write to buffer while read buffer is not empty\n");
         abort();
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 13/36] savevm: Rename has_error to last_error field

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> Now the field contains the last error name, so rename acordingly.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   savevm.c |   16 ++++++++--------
>   1 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/savevm.c b/savevm.c
> index c037eb3..69a2ccd 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -173,7 +173,7 @@ struct QEMUFile {
>       int buf_size; /* 0 when writing */
>       uint8_t buf[IO_BUF_SIZE];
>
> -    int has_error;
> +    int last_error;
>   };
>
>   typedef struct QEMUFileStdio
> @@ -427,12 +427,12 @@ QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
>
>   int qemu_file_get_error(QEMUFile *f)
>   {
> -    return f->has_error;
> +    return f->last_error;
>   }
>
>   void qemu_file_set_error(QEMUFile *f, int ret)
>   {
> -    f->has_error = ret;
> +    f->last_error = ret;
>   }
>
>   void qemu_fflush(QEMUFile *f)
> @@ -447,7 +447,7 @@ void qemu_fflush(QEMUFile *f)
>           if (len>  0)
>               f->buf_offset += f->buf_index;
>           else
> -            f->has_error = -EINVAL;
> +            f->last_error = -EINVAL;
>           f->buf_index = 0;
>       }
>   }
> @@ -468,7 +468,7 @@ static void qemu_fill_buffer(QEMUFile *f)
>           f->buf_size = len;
>           f->buf_offset += len;
>       } else if (len != -EAGAIN)
> -        f->has_error = len;
> +        f->last_error = len;
>   }
>
>   int qemu_fclose(QEMUFile *f)
> @@ -490,13 +490,13 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
>   {
>       int l;
>
> -    if (!f->has_error&&  f->is_write == 0&&  f->buf_index>  0) {
> +    if (!f->last_error&&  f->is_write == 0&&  f->buf_index>  0) {
>           fprintf(stderr,
>                   "Attempted to write to buffer while read buffer is not empty\n");
>           abort();
>       }
>
> -    while (!f->has_error&&  size>  0) {
> +    while (!f->last_error&&  size>  0) {
>           l = IO_BUF_SIZE - f->buf_index;
>           if (l>  size)
>               l = size;
> @@ -512,7 +512,7 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
>
>   void qemu_put_byte(QEMUFile *f, int v)
>   {
> -    if (!f->has_error&&  f->is_write == 0&&  f->buf_index>  0) {
> +    if (!f->last_error&&  f->is_write == 0&&  f->buf_index>  0) {
>           fprintf(stderr,
>                   "Attempted to write to buffer while read buffer is not empty\n");
>           abort();

[Qemu-devel] [PATCH 14/36] migration: use qemu_file_get_error() return value when possible

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c       |    6 ++++--
 block-migration.c |    7 ++++---
 buffered_file.c   |   13 ++++++++-----
 savevm.c          |    4 ++--
 4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 9128be0..98daaf3 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -371,6 +371,7 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)
 {
     ram_addr_t addr;
     int flags;
+    int error;

     if (version_id < 3 || version_id > 4) {
         return -EINVAL;
@@ -451,8 +452,9 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)

             qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
         }
-        if (qemu_file_get_error(f)) {
-            return -EIO;
+        error = qemu_file_get_error(f);
+        if (error) {
+            return error;
         }
     } while (!(flags & RAM_SAVE_FLAG_EOS));

diff --git a/block-migration.c b/block-migration.c
index 8308753..e36f2e3 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -646,6 +646,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
     uint8_t *buf;
     int64_t total_sectors = 0;
     int nr_sectors;
+    int ret;

     do {
         addr = qemu_get_be64(f);
@@ -654,7 +655,6 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
         addr >>= BDRV_SECTOR_BITS;

         if (flags & BLK_MIG_FLAG_DEVICE_BLOCK) {
-            int ret;
             /* get device name */
             len = qemu_get_byte(f);
             qemu_get_buffer(f, (uint8_t *)device_name, len);
@@ -704,8 +704,9 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
             fprintf(stderr, "Unknown flags\n");
             return -EINVAL;
         }
-        if (qemu_file_get_error(f)) {
-            return -EIO;
+        ret = qemu_file_get_error(f);
+        if (ret != 0) {
+            return ret;
         }
     } while (!(flags & BLK_MIG_FLAG_EOS));

diff --git a/buffered_file.c b/buffered_file.c
index 82f4001..42bbbd3 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -71,9 +71,11 @@ static void buffered_append(QEMUFileBuffered *s,
 static void buffered_flush(QEMUFileBuffered *s)
 {
     size_t offset = 0;
+    int error;

-    if (qemu_file_get_error(s->file)) {
-        DPRINTF("flush when error, bailing\n");
+    error = qemu_file_get_error(s->file);
+    if (error != 0) {
+        DPRINTF("flush when error, bailing: %s\n", strerror(-error));
         return;
     }

@@ -108,13 +110,14 @@ static void buffered_flush(QEMUFileBuffered *s)
 static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, int size)
 {
     QEMUFileBuffered *s = opaque;
-    int offset = 0;
+    int offset = 0, error;
     ssize_t ret;

     DPRINTF("putting %d bytes at %" PRId64 "\n", size, pos);

-    if (qemu_file_get_error(s->file)) {
-        DPRINTF("flush when error, bailing\n");
+    error = qemu_file_get_error(s->file);
+    if (error) {
+        DPRINTF("flush when error, bailing: %s\n", strerror(-error));
         return -EINVAL;
     }

diff --git a/savevm.c b/savevm.c
index 69a2ccd..3042ae5 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1835,8 +1835,8 @@ out:
         g_free(le);
     }

-    if (qemu_file_get_error(f)) {
-        ret = -EIO;
+    if (ret == 0) {
+        ret = qemu_file_get_error(f);
     }

     return ret;
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 14/36] migration: use qemu_file_get_error() return value when possible

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   arch_init.c       |    6 ++++--
>   block-migration.c |    7 ++++---
>   buffered_file.c   |   13 ++++++++-----
>   savevm.c          |    4 ++--
>   4 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 9128be0..98daaf3 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -371,6 +371,7 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)
>   {
>       ram_addr_t addr;
>       int flags;
> +    int error;
>
>       if (version_id<  3 || version_id>  4) {
>           return -EINVAL;
> @@ -451,8 +452,9 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)
>
>               qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
>           }
> -        if (qemu_file_get_error(f)) {
> -            return -EIO;
> +        error = qemu_file_get_error(f);
> +        if (error) {
> +            return error;
>           }
>       } while (!(flags&  RAM_SAVE_FLAG_EOS));
>
> diff --git a/block-migration.c b/block-migration.c
> index 8308753..e36f2e3 100644
> --- a/block-migration.c
> +++ b/block-migration.c
> @@ -646,6 +646,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
>       uint8_t *buf;
>       int64_t total_sectors = 0;
>       int nr_sectors;
> +    int ret;
>
>       do {
>           addr = qemu_get_be64(f);
> @@ -654,7 +655,6 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
>           addr>>= BDRV_SECTOR_BITS;
>
>           if (flags&  BLK_MIG_FLAG_DEVICE_BLOCK) {
> -            int ret;
>               /* get device name */
>               len = qemu_get_byte(f);
>               qemu_get_buffer(f, (uint8_t *)device_name, len);
> @@ -704,8 +704,9 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
>               fprintf(stderr, "Unknown flags\n");
>               return -EINVAL;
>           }
> -        if (qemu_file_get_error(f)) {
> -            return -EIO;
> +        ret = qemu_file_get_error(f);
> +        if (ret != 0) {
> +            return ret;
>           }
>       } while (!(flags&  BLK_MIG_FLAG_EOS));
>
> diff --git a/buffered_file.c b/buffered_file.c
> index 82f4001..42bbbd3 100644
> --- a/buffered_file.c
> +++ b/buffered_file.c
> @@ -71,9 +71,11 @@ static void buffered_append(QEMUFileBuffered *s,
>   static void buffered_flush(QEMUFileBuffered *s)
>   {
>       size_t offset = 0;
> +    int error;
>
> -    if (qemu_file_get_error(s->file)) {
> -        DPRINTF("flush when error, bailing\n");
> +    error = qemu_file_get_error(s->file);
> +    if (error != 0) {
> +        DPRINTF("flush when error, bailing: %s\n", strerror(-error));
>           return;
>       }
>
> @@ -108,13 +110,14 @@ static void buffered_flush(QEMUFileBuffered *s)
>   static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, int size)
>   {
>       QEMUFileBuffered *s = opaque;
> -    int offset = 0;
> +    int offset = 0, error;
>       ssize_t ret;
>
>       DPRINTF("putting %d bytes at %" PRId64 "\n", size, pos);
>
> -    if (qemu_file_get_error(s->file)) {
> -        DPRINTF("flush when error, bailing\n");
> +    error = qemu_file_get_error(s->file);
> +    if (error) {
> +        DPRINTF("flush when error, bailing: %s\n", strerror(-error));
>           return -EINVAL;
>       }
>
> diff --git a/savevm.c b/savevm.c
> index 69a2ccd..3042ae5 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1835,8 +1835,8 @@ out:
>           g_free(le);
>       }
>
> -    if (qemu_file_get_error(f)) {
> -        ret = -EIO;
> +    if (ret == 0) {
> +        ret = qemu_file_get_error(f);
>       }
>
>       return ret;

[Qemu-devel] [PATCH 15/36] migration: Make *start_outgoing_migration return FdMigrationState

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration-exec.c |    4 ++--
 migration-fd.c   |    4 ++--
 migration-tcp.c  |    4 ++--
 migration-unix.c |    4 ++--
 migration.c      |    4 ++--
 migration.h      |    8 ++++----
 6 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/migration-exec.c b/migration-exec.c
index 2cfb6f2..759aa79 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -61,7 +61,7 @@ static int exec_close(FdMigrationState *s)
     return ret;
 }

-MigrationState *exec_start_outgoing_migration(Monitor *mon,
+FdMigrationState *exec_start_outgoing_migration(Monitor *mon,
                                               const char *command,
 					      int64_t bandwidth_limit,
 					      int detach,
@@ -108,7 +108,7 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
     }

     migrate_fd_connect(s);
-    return &s->mig_state;
+    return s;

 err_after_open:
     pclose(f);
diff --git a/migration-fd.c b/migration-fd.c
index aee690a..8036a27 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -50,7 +50,7 @@ static int fd_close(FdMigrationState *s)
     return 0;
 }

-MigrationState *fd_start_outgoing_migration(Monitor *mon,
+FdMigrationState *fd_start_outgoing_migration(Monitor *mon,
 					    const char *fdname,
 					    int64_t bandwidth_limit,
 					    int detach,
@@ -91,7 +91,7 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
     }

     migrate_fd_connect(s);
-    return &s->mig_state;
+    return s;

 err_after_open:
     close(s->fd);
diff --git a/migration-tcp.c b/migration-tcp.c
index c431e03..05a121f 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -75,7 +75,7 @@ static void tcp_wait_for_connect(void *opaque)
     }
 }

-MigrationState *tcp_start_outgoing_migration(Monitor *mon,
+FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,
                                              const char *host_port,
                                              int64_t bandwidth_limit,
                                              int detach,
@@ -131,7 +131,7 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
     } else if (ret >= 0)
         migrate_fd_connect(s);

-    return &s->mig_state;
+    return s;
 }

 static void tcp_accept_incoming_migration(void *opaque)
diff --git a/migration-unix.c b/migration-unix.c
index 6dc985d..0eeedde 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -74,7 +74,7 @@ static void unix_wait_for_connect(void *opaque)
     }
 }

-MigrationState *unix_start_outgoing_migration(Monitor *mon,
+FdMigrationState *unix_start_outgoing_migration(Monitor *mon,
                                               const char *path,
 					      int64_t bandwidth_limit,
 					      int detach,
@@ -132,7 +132,7 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
     if (ret >= 0)
         migrate_fd_connect(s);

-    return &s->mig_state;
+    return s;

 err_after_open:
     close(s->fd);
diff --git a/migration.c b/migration.c
index 6bac734..4449594 100644
--- a/migration.c
+++ b/migration.c
@@ -79,7 +79,7 @@ void process_incoming_migration(QEMUFile *f)

 int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
-    MigrationState *s = NULL;
+    FdMigrationState *s = NULL;
     const char *p;
     int detach = qdict_get_try_bool(qdict, "detach", 0);
     int blk = qdict_get_try_bool(qdict, "blk", 0);
@@ -124,7 +124,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
         current_migration->release(current_migration);
     }

-    current_migration = s;
+    current_migration = &s->mig_state;
     notifier_list_notify(&migration_state_notifiers, NULL);
     return 0;
 }
diff --git a/migration.h b/migration.h
index 050c56c..58354c7 100644
--- a/migration.h
+++ b/migration.h
@@ -72,7 +72,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data);

 int exec_start_incoming_migration(const char *host_port);

-MigrationState *exec_start_outgoing_migration(Monitor *mon,
+FdMigrationState *exec_start_outgoing_migration(Monitor *mon,
                                               const char *host_port,
 					      int64_t bandwidth_limit,
 					      int detach,
@@ -81,7 +81,7 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,

 int tcp_start_incoming_migration(const char *host_port);

-MigrationState *tcp_start_outgoing_migration(Monitor *mon,
+FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,
                                              const char *host_port,
 					     int64_t bandwidth_limit,
 					     int detach,
@@ -90,7 +90,7 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,

 int unix_start_incoming_migration(const char *path);

-MigrationState *unix_start_outgoing_migration(Monitor *mon,
+FdMigrationState *unix_start_outgoing_migration(Monitor *mon,
                                               const char *path,
 					      int64_t bandwidth_limit,
 					      int detach,
@@ -99,7 +99,7 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,

 int fd_start_incoming_migration(const char *path);

-MigrationState *fd_start_outgoing_migration(Monitor *mon,
+FdMigrationState *fd_start_outgoing_migration(Monitor *mon,
 					    const char *fdname,
 					    int64_t bandwidth_limit,
 					    int detach,
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 15/36] migration: Make *start_outgoing_migration return FdMigrationState

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   migration-exec.c |    4 ++--
>   migration-fd.c   |    4 ++--
>   migration-tcp.c  |    4 ++--
>   migration-unix.c |    4 ++--
>   migration.c      |    4 ++--
>   migration.h      |    8 ++++----
>   6 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/migration-exec.c b/migration-exec.c
> index 2cfb6f2..759aa79 100644
> --- a/migration-exec.c
> +++ b/migration-exec.c
> @@ -61,7 +61,7 @@ static int exec_close(FdMigrationState *s)
>       return ret;
>   }
>
> -MigrationState *exec_start_outgoing_migration(Monitor *mon,
> +FdMigrationState *exec_start_outgoing_migration(Monitor *mon,
>                                                 const char *command,
>   					      int64_t bandwidth_limit,
>   					      int detach,
> @@ -108,7 +108,7 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
>       }
>
>       migrate_fd_connect(s);
> -    return&s->mig_state;
> +    return s;
>
>   err_after_open:
>       pclose(f);
> diff --git a/migration-fd.c b/migration-fd.c
> index aee690a..8036a27 100644
> --- a/migration-fd.c
> +++ b/migration-fd.c
> @@ -50,7 +50,7 @@ static int fd_close(FdMigrationState *s)
>       return 0;
>   }
>
> -MigrationState *fd_start_outgoing_migration(Monitor *mon,
> +FdMigrationState *fd_start_outgoing_migration(Monitor *mon,
>   					    const char *fdname,
>   					    int64_t bandwidth_limit,
>   					    int detach,
> @@ -91,7 +91,7 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
>       }
>
>       migrate_fd_connect(s);
> -    return&s->mig_state;
> +    return s;
>
>   err_after_open:
>       close(s->fd);
> diff --git a/migration-tcp.c b/migration-tcp.c
> index c431e03..05a121f 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -75,7 +75,7 @@ static void tcp_wait_for_connect(void *opaque)
>       }
>   }
>
> -MigrationState *tcp_start_outgoing_migration(Monitor *mon,
> +FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,
>                                                const char *host_port,
>                                                int64_t bandwidth_limit,
>                                                int detach,
> @@ -131,7 +131,7 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
>       } else if (ret>= 0)
>           migrate_fd_connect(s);
>
> -    return&s->mig_state;
> +    return s;
>   }
>
>   static void tcp_accept_incoming_migration(void *opaque)
> diff --git a/migration-unix.c b/migration-unix.c
> index 6dc985d..0eeedde 100644
> --- a/migration-unix.c
> +++ b/migration-unix.c
> @@ -74,7 +74,7 @@ static void unix_wait_for_connect(void *opaque)
>       }
>   }
>
> -MigrationState *unix_start_outgoing_migration(Monitor *mon,
> +FdMigrationState *unix_start_outgoing_migration(Monitor *mon,
>                                                 const char *path,
>   					      int64_t bandwidth_limit,
>   					      int detach,
> @@ -132,7 +132,7 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
>       if (ret>= 0)
>           migrate_fd_connect(s);
>
> -    return&s->mig_state;
> +    return s;
>
>   err_after_open:
>       close(s->fd);
> diff --git a/migration.c b/migration.c
> index 6bac734..4449594 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -79,7 +79,7 @@ void process_incoming_migration(QEMUFile *f)
>
>   int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
>   {
> -    MigrationState *s = NULL;
> +    FdMigrationState *s = NULL;
>       const char *p;
>       int detach = qdict_get_try_bool(qdict, "detach", 0);
>       int blk = qdict_get_try_bool(qdict, "blk", 0);
> @@ -124,7 +124,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
>           current_migration->release(current_migration);
>       }
>
> -    current_migration = s;
> +    current_migration =&s->mig_state;
>       notifier_list_notify(&migration_state_notifiers, NULL);
>       return 0;
>   }
> diff --git a/migration.h b/migration.h
> index 050c56c..58354c7 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -72,7 +72,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data);
>
>   int exec_start_incoming_migration(const char *host_port);
>
> -MigrationState *exec_start_outgoing_migration(Monitor *mon,
> +FdMigrationState *exec_start_outgoing_migration(Monitor *mon,
>                                                 const char *host_port,
>   					      int64_t bandwidth_limit,
>   					      int detach,
> @@ -81,7 +81,7 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
>
>   int tcp_start_incoming_migration(const char *host_port);
>
> -MigrationState *tcp_start_outgoing_migration(Monitor *mon,
> +FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,
>                                                const char *host_port,
>   					     int64_t bandwidth_limit,
>   					     int detach,
> @@ -90,7 +90,7 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
>
>   int unix_start_incoming_migration(const char *path);
>
> -MigrationState *unix_start_outgoing_migration(Monitor *mon,
> +FdMigrationState *unix_start_outgoing_migration(Monitor *mon,
>                                                 const char *path,
>   					      int64_t bandwidth_limit,
>   					      int detach,
> @@ -99,7 +99,7 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
>
>   int fd_start_incoming_migration(const char *path);
>
> -MigrationState *fd_start_outgoing_migration(Monitor *mon,
> +FdMigrationState *fd_start_outgoing_migration(Monitor *mon,
>   					    const char *fdname,
>   					    int64_t bandwidth_limit,
>   					    int detach,

[Qemu-devel] [PATCH 16/36] migration: Use FdMigrationState instead of MigrationState when possible

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
---
 migration.c |   34 ++++++++++++++++------------------
 migration.h |   16 ++++++++--------
 2 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/migration.c b/migration.c
index 4449594..f8eefa6 100644
--- a/migration.c
+++ b/migration.c
@@ -34,7 +34,7 @@
 /* Migration speed throttling */
 static int64_t max_throttle = (32 << 20);

-static MigrationState *current_migration;
+static FdMigrationState *current_migration;

 static NotifierList migration_state_notifiers =
     NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
@@ -87,7 +87,8 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
     const char *uri = qdict_get_str(qdict, "uri");

     if (current_migration &&
-        current_migration->get_status(current_migration) == MIG_STATE_ACTIVE) {
+        current_migration->mig_state.get_status(current_migration) ==
+        MIG_STATE_ACTIVE) {
         monitor_printf(mon, "migration already in progress\n");
         return -1;
     }
@@ -121,20 +122,20 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
     }

     if (current_migration) {
-        current_migration->release(current_migration);
+        current_migration->mig_state.release(current_migration);
     }

-    current_migration = &s->mig_state;
+    current_migration = s;
     notifier_list_notify(&migration_state_notifiers, NULL);
     return 0;
 }

 int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
-    MigrationState *s = current_migration;
+    FdMigrationState *s = current_migration;

-    if (s && s->get_status(s) == MIG_STATE_ACTIVE) {
-        s->cancel(s);
+    if (s && s->mig_state.get_status(s) == MIG_STATE_ACTIVE) {
+        s->mig_state.cancel(s);
     }
     return 0;
 }
@@ -150,7 +151,7 @@ int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
     }
     max_throttle = d;

-    s = migrate_to_fms(current_migration);
+    s = current_migration;
     if (s && s->file) {
         qemu_file_set_rate_limit(s->file, max_throttle);
     }
@@ -228,10 +229,11 @@ static void migrate_put_status(QDict *qdict, const char *name,
 void do_info_migrate(Monitor *mon, QObject **ret_data)
 {
     QDict *qdict;
-    MigrationState *s = current_migration;

-    if (s) {
-        switch (s->get_status(s)) {
+    if (current_migration) {
+        MigrationState *s = &current_migration->mig_state;
+
+        switch (s->get_status(current_migration)) {
         case MIG_STATE_ACTIVE:
             qdict = qdict_new();
             qdict_put(qdict, "status", qstring_from_str("active"));
@@ -404,16 +406,13 @@ void migrate_fd_put_ready(void *opaque)
     }
 }

-int migrate_fd_get_status(MigrationState *mig_state)
+int migrate_fd_get_status(FdMigrationState *s)
 {
-    FdMigrationState *s = migrate_to_fms(mig_state);
     return s->state;
 }

-void migrate_fd_cancel(MigrationState *mig_state)
+void migrate_fd_cancel(FdMigrationState *s)
 {
-    FdMigrationState *s = migrate_to_fms(mig_state);
-
     if (s->state != MIG_STATE_ACTIVE)
         return;

@@ -426,9 +425,8 @@ void migrate_fd_cancel(MigrationState *mig_state)
     migrate_fd_cleanup(s);
 }

-void migrate_fd_release(MigrationState *mig_state)
+void migrate_fd_release(FdMigrationState *s)
 {
-    FdMigrationState *s = migrate_to_fms(mig_state);

     DPRINTF("releasing state\n");
    
diff --git a/migration.h b/migration.h
index 58354c7..b10bb6e 100644
--- a/migration.h
+++ b/migration.h
@@ -25,18 +25,18 @@

 typedef struct MigrationState MigrationState;

+typedef struct FdMigrationState FdMigrationState;
+
 struct MigrationState
 {
     /* FIXME: add more accessors to print migration info */
-    void (*cancel)(MigrationState *s);
-    int (*get_status)(MigrationState *s);
-    void (*release)(MigrationState *s);
+    void (*cancel)(FdMigrationState *s);
+    int (*get_status)(FdMigrationState *s);
+    void (*release)(FdMigrationState *s);
     int blk;
     int shared;
 };

-typedef struct FdMigrationState FdMigrationState;
-
 struct FdMigrationState
 {
     MigrationState mig_state;
@@ -120,11 +120,11 @@ void migrate_fd_connect(FdMigrationState *s);

 void migrate_fd_put_ready(void *opaque);

-int migrate_fd_get_status(MigrationState *mig_state);
+int migrate_fd_get_status(FdMigrationState *mig_state);

-void migrate_fd_cancel(MigrationState *mig_state);
+void migrate_fd_cancel(FdMigrationState *mig_state);

-void migrate_fd_release(MigrationState *mig_state);
+void migrate_fd_release(FdMigrationState *mig_state);

 void migrate_fd_wait_for_unfreeze(void *opaque);

-- 
1.7.6.4

[Qemu-devel] [PATCH 17/36] migration: Fold MigrationState into FdMigrationState

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
---
 migration-exec.c |   10 +++++-----
 migration-fd.c   |   10 +++++-----
 migration-tcp.c  |   10 +++++-----
 migration-unix.c |   10 +++++-----
 migration.c      |   14 ++++++--------
 migration.h      |   23 +++++------------------
 6 files changed, 31 insertions(+), 46 deletions(-)

diff --git a/migration-exec.c b/migration-exec.c
index 759aa79..39fd416 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -92,12 +92,12 @@ FdMigrationState *exec_start_outgoing_migration(Monitor *mon,
     s->close = exec_close;
     s->get_error = file_errno;
     s->write = file_write;
-    s->mig_state.cancel = migrate_fd_cancel;
-    s->mig_state.get_status = migrate_fd_get_status;
-    s->mig_state.release = migrate_fd_release;
+    s->cancel = migrate_fd_cancel;
+    s->get_status = migrate_fd_get_status;
+    s->release = migrate_fd_release;

-    s->mig_state.blk = blk;
-    s->mig_state.shared = inc;
+    s->blk = blk;
+    s->shared = inc;

     s->state = MIG_STATE_ACTIVE;
     s->mon = NULL;
diff --git a/migration-fd.c b/migration-fd.c
index 8036a27..c3c7b0e 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -75,12 +75,12 @@ FdMigrationState *fd_start_outgoing_migration(Monitor *mon,
     s->get_error = fd_errno;
     s->write = fd_write;
     s->close = fd_close;
-    s->mig_state.cancel = migrate_fd_cancel;
-    s->mig_state.get_status = migrate_fd_get_status;
-    s->mig_state.release = migrate_fd_release;
+    s->cancel = migrate_fd_cancel;
+    s->get_status = migrate_fd_get_status;
+    s->release = migrate_fd_release;

-    s->mig_state.blk = blk;
-    s->mig_state.shared = inc;
+    s->blk = blk;
+    s->shared = inc;

     s->state = MIG_STATE_ACTIVE;
     s->mon = NULL;
diff --git a/migration-tcp.c b/migration-tcp.c
index 05a121f..5ce93d9 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -94,12 +94,12 @@ FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,
     s->get_error = socket_errno;
     s->write = socket_write;
     s->close = tcp_close;
-    s->mig_state.cancel = migrate_fd_cancel;
-    s->mig_state.get_status = migrate_fd_get_status;
-    s->mig_state.release = migrate_fd_release;
+    s->cancel = migrate_fd_cancel;
+    s->get_status = migrate_fd_get_status;
+    s->release = migrate_fd_release;

-    s->mig_state.blk = blk;
-    s->mig_state.shared = inc;
+    s->blk = blk;
+    s->shared = inc;

     s->state = MIG_STATE_ACTIVE;
     s->mon = NULL;
diff --git a/migration-unix.c b/migration-unix.c
index 0eeedde..00a6ed5 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -93,12 +93,12 @@ FdMigrationState *unix_start_outgoing_migration(Monitor *mon,
     s->get_error = unix_errno;
     s->write = unix_write;
     s->close = unix_close;
-    s->mig_state.cancel = migrate_fd_cancel;
-    s->mig_state.get_status = migrate_fd_get_status;
-    s->mig_state.release = migrate_fd_release;
+    s->cancel = migrate_fd_cancel;
+    s->get_status = migrate_fd_get_status;
+    s->release = migrate_fd_release;

-    s->mig_state.blk = blk;
-    s->mig_state.shared = inc;
+    s->blk = blk;
+    s->shared = inc;

     s->state = MIG_STATE_ACTIVE;
     s->mon = NULL;
diff --git a/migration.c b/migration.c
index f8eefa6..d51f737 100644
--- a/migration.c
+++ b/migration.c
@@ -87,8 +87,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
     const char *uri = qdict_get_str(qdict, "uri");

     if (current_migration &&
-        current_migration->mig_state.get_status(current_migration) ==
-        MIG_STATE_ACTIVE) {
+        current_migration->get_status(current_migration) == MIG_STATE_ACTIVE) {
         monitor_printf(mon, "migration already in progress\n");
         return -1;
     }
@@ -122,7 +121,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
     }

     if (current_migration) {
-        current_migration->mig_state.release(current_migration);
+        current_migration->release(current_migration);
     }

     current_migration = s;
@@ -134,8 +133,8 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     FdMigrationState *s = current_migration;

-    if (s && s->mig_state.get_status(s) == MIG_STATE_ACTIVE) {
-        s->mig_state.cancel(s);
+    if (s && s->get_status(s) == MIG_STATE_ACTIVE) {
+        s->cancel(s);
     }
     return 0;
 }
@@ -231,7 +230,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
     QDict *qdict;

     if (current_migration) {
-        MigrationState *s = &current_migration->mig_state;
+        FdMigrationState *s = current_migration;

         switch (s->get_status(current_migration)) {
         case MIG_STATE_ACTIVE:
@@ -355,8 +354,7 @@ void migrate_fd_connect(FdMigrationState *s)
                                       migrate_fd_close);

     DPRINTF("beginning savevm\n");
-    ret = qemu_savevm_state_begin(s->mon, s->file, s->mig_state.blk,
-                                  s->mig_state.shared);
+    ret = qemu_savevm_state_begin(s->mon, s->file, s->blk, s->shared);
     if (ret < 0) {
         DPRINTF("failed, %d\n", ret);
         migrate_fd_error(s);
diff --git a/migration.h b/migration.h
index b10bb6e..f0caf7b 100644
--- a/migration.h
+++ b/migration.h
@@ -23,23 +23,10 @@
 #define MIG_STATE_CANCELLED	1
 #define MIG_STATE_ACTIVE	2

-typedef struct MigrationState MigrationState;
-
 typedef struct FdMigrationState FdMigrationState;

-struct MigrationState
-{
-    /* FIXME: add more accessors to print migration info */
-    void (*cancel)(FdMigrationState *s);
-    int (*get_status)(FdMigrationState *s);
-    void (*release)(FdMigrationState *s);
-    int blk;
-    int shared;
-};
-
 struct FdMigrationState
 {
-    MigrationState mig_state;
     int64_t bandwidth_limit;
     QEMUFile *file;
     int fd;
@@ -48,7 +35,12 @@ struct FdMigrationState
     int (*get_error)(struct FdMigrationState*);
     int (*close)(struct FdMigrationState*);
     int (*write)(struct FdMigrationState*, const void *, size_t);
+    void (*cancel)(FdMigrationState *s);
+    int (*get_status)(FdMigrationState *s);
+    void (*release)(FdMigrationState *s);
     void *opaque;
+    int blk;
+    int shared;
 };

 void process_incoming_migration(QEMUFile *f);
@@ -130,11 +122,6 @@ void migrate_fd_wait_for_unfreeze(void *opaque);

 int migrate_fd_close(void *opaque);

-static inline FdMigrationState *migrate_to_fms(MigrationState *mig_state)
-{
-    return container_of(mig_state, FdMigrationState, mig_state);
-}
-
 void add_migration_state_change_notifier(Notifier *notify);
 void remove_migration_state_change_notifier(Notifier *notify);
 int get_migration_state(void);
-- 
1.7.6.4

[Qemu-devel] [PATCH 18/36] migration: Rename FdMigrationState MigrationState

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
---
 migration-exec.c |   10 +++++-----
 migration-fd.c   |   10 +++++-----
 migration-tcp.c  |   12 ++++++------
 migration-unix.c |   12 ++++++------
 migration.c      |   34 +++++++++++++++++-----------------
 migration.h      |   38 +++++++++++++++++++-------------------
 6 files changed, 58 insertions(+), 58 deletions(-)

diff --git a/migration-exec.c b/migration-exec.c
index 39fd416..0ed5976 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -32,17 +32,17 @@
     do { } while (0)
 #endif

-static int file_errno(FdMigrationState *s)
+static int file_errno(MigrationState *s)
 {
     return errno;
 }

-static int file_write(FdMigrationState *s, const void * buf, size_t size)
+static int file_write(MigrationState *s, const void * buf, size_t size)
 {
     return write(s->fd, buf, size);
 }

-static int exec_close(FdMigrationState *s)
+static int exec_close(MigrationState *s)
 {
     int ret = 0;
     DPRINTF("exec_close\n");
@@ -61,14 +61,14 @@ static int exec_close(FdMigrationState *s)
     return ret;
 }

-FdMigrationState *exec_start_outgoing_migration(Monitor *mon,
+MigrationState *exec_start_outgoing_migration(Monitor *mon,
                                               const char *command,
 					      int64_t bandwidth_limit,
 					      int detach,
 					      int blk,
 					      int inc)
 {
-    FdMigrationState *s;
+    MigrationState *s;
     FILE *f;

     s = g_malloc0(sizeof(*s));
diff --git a/migration-fd.c b/migration-fd.c
index c3c7b0e..e78fd4e 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -30,17 +30,17 @@
     do { } while (0)
 #endif

-static int fd_errno(FdMigrationState *s)
+static int fd_errno(MigrationState *s)
 {
     return errno;
 }

-static int fd_write(FdMigrationState *s, const void * buf, size_t size)
+static int fd_write(MigrationState *s, const void * buf, size_t size)
 {
     return write(s->fd, buf, size);
 }

-static int fd_close(FdMigrationState *s)
+static int fd_close(MigrationState *s)
 {
     DPRINTF("fd_close\n");
     if (s->fd != -1) {
@@ -50,14 +50,14 @@ static int fd_close(FdMigrationState *s)
     return 0;
 }

-FdMigrationState *fd_start_outgoing_migration(Monitor *mon,
+MigrationState *fd_start_outgoing_migration(Monitor *mon,
 					    const char *fdname,
 					    int64_t bandwidth_limit,
 					    int detach,
 					    int blk,
 					    int inc)
 {
-    FdMigrationState *s;
+    MigrationState *s;

     s = g_malloc0(sizeof(*s));

diff --git a/migration-tcp.c b/migration-tcp.c
index 5ce93d9..d6feb23 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -28,17 +28,17 @@
     do { } while (0)
 #endif

-static int socket_errno(FdMigrationState *s)
+static int socket_errno(MigrationState *s)
 {
     return socket_error();
 }

-static int socket_write(FdMigrationState *s, const void * buf, size_t size)
+static int socket_write(MigrationState *s, const void * buf, size_t size)
 {
     return send(s->fd, buf, size, 0);
 }

-static int tcp_close(FdMigrationState *s)
+static int tcp_close(MigrationState *s)
 {
     DPRINTF("tcp_close\n");
     if (s->fd != -1) {
@@ -51,7 +51,7 @@ static int tcp_close(FdMigrationState *s)

 static void tcp_wait_for_connect(void *opaque)
 {
-    FdMigrationState *s = opaque;
+    MigrationState *s = opaque;
     int val, ret;
     socklen_t valsize = sizeof(val);

@@ -75,7 +75,7 @@ static void tcp_wait_for_connect(void *opaque)
     }
 }

-FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,
+MigrationState *tcp_start_outgoing_migration(Monitor *mon,
                                              const char *host_port,
                                              int64_t bandwidth_limit,
                                              int detach,
@@ -83,7 +83,7 @@ FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,
 					     int inc)
 {
     struct sockaddr_in addr;
-    FdMigrationState *s;
+    MigrationState *s;
     int ret;

     if (parse_host_port(&addr, host_port) < 0)
diff --git a/migration-unix.c b/migration-unix.c
index 00a6ed5..3b9017b 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -28,17 +28,17 @@
     do { } while (0)
 #endif

-static int unix_errno(FdMigrationState *s)
+static int unix_errno(MigrationState *s)
 {
     return errno;
 }

-static int unix_write(FdMigrationState *s, const void * buf, size_t size)
+static int unix_write(MigrationState *s, const void * buf, size_t size)
 {
     return write(s->fd, buf, size);
 }

-static int unix_close(FdMigrationState *s)
+static int unix_close(MigrationState *s)
 {
     DPRINTF("unix_close\n");
     if (s->fd != -1) {
@@ -50,7 +50,7 @@ static int unix_close(FdMigrationState *s)

 static void unix_wait_for_connect(void *opaque)
 {
-    FdMigrationState *s = opaque;
+    MigrationState *s = opaque;
     int val, ret;
     socklen_t valsize = sizeof(val);

@@ -74,14 +74,14 @@ static void unix_wait_for_connect(void *opaque)
     }
 }

-FdMigrationState *unix_start_outgoing_migration(Monitor *mon,
+MigrationState *unix_start_outgoing_migration(Monitor *mon,
                                               const char *path,
 					      int64_t bandwidth_limit,
 					      int detach,
 					      int blk,
 					      int inc)
 {
-    FdMigrationState *s;
+    MigrationState *s;
     struct sockaddr_un addr;
     int ret;

diff --git a/migration.c b/migration.c
index d51f737..5509a8f 100644
--- a/migration.c
+++ b/migration.c
@@ -34,7 +34,7 @@
 /* Migration speed throttling */
 static int64_t max_throttle = (32 << 20);

-static FdMigrationState *current_migration;
+static MigrationState *current_migration;

 static NotifierList migration_state_notifiers =
     NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
@@ -79,7 +79,7 @@ void process_incoming_migration(QEMUFile *f)

 int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
-    FdMigrationState *s = NULL;
+    MigrationState *s = NULL;
     const char *p;
     int detach = qdict_get_try_bool(qdict, "detach", 0);
     int blk = qdict_get_try_bool(qdict, "blk", 0);
@@ -131,7 +131,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)

 int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
-    FdMigrationState *s = current_migration;
+    MigrationState *s = current_migration;

     if (s && s->get_status(s) == MIG_STATE_ACTIVE) {
         s->cancel(s);
@@ -142,7 +142,7 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
 int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     int64_t d;
-    FdMigrationState *s;
+    MigrationState *s;

     d = qdict_get_int(qdict, "value");
     if (d < 0) {
@@ -230,7 +230,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
     QDict *qdict;

     if (current_migration) {
-        FdMigrationState *s = current_migration;
+        MigrationState *s = current_migration;

         switch (s->get_status(current_migration)) {
         case MIG_STATE_ACTIVE:
@@ -263,7 +263,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)

 /* shared migration helpers */

-void migrate_fd_monitor_suspend(FdMigrationState *s, Monitor *mon)
+void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon)
 {
     s->mon = mon;
     if (monitor_suspend(mon) == 0) {
@@ -274,7 +274,7 @@ void migrate_fd_monitor_suspend(FdMigrationState *s, Monitor *mon)
     }
 }

-void migrate_fd_error(FdMigrationState *s)
+void migrate_fd_error(MigrationState *s)
 {
     DPRINTF("setting error state\n");
     s->state = MIG_STATE_ERROR;
@@ -282,7 +282,7 @@ void migrate_fd_error(FdMigrationState *s)
     migrate_fd_cleanup(s);
 }

-int migrate_fd_cleanup(FdMigrationState *s)
+int migrate_fd_cleanup(MigrationState *s)
 {
     int ret = 0;

@@ -310,7 +310,7 @@ int migrate_fd_cleanup(FdMigrationState *s)

 void migrate_fd_put_notify(void *opaque)
 {
-    FdMigrationState *s = opaque;
+    MigrationState *s = opaque;

     qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
     qemu_file_put_notify(s->file);
@@ -321,7 +321,7 @@ void migrate_fd_put_notify(void *opaque)

 ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
 {
-    FdMigrationState *s = opaque;
+    MigrationState *s = opaque;
     ssize_t ret;

     if (s->state != MIG_STATE_ACTIVE) {
@@ -342,7 +342,7 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
     return ret;
 }

-void migrate_fd_connect(FdMigrationState *s)
+void migrate_fd_connect(MigrationState *s)
 {
     int ret;

@@ -366,7 +366,7 @@ void migrate_fd_connect(FdMigrationState *s)

 void migrate_fd_put_ready(void *opaque)
 {
-    FdMigrationState *s = opaque;
+    MigrationState *s = opaque;
     int ret;

     if (s->state != MIG_STATE_ACTIVE) {
@@ -404,12 +404,12 @@ void migrate_fd_put_ready(void *opaque)
     }
 }

-int migrate_fd_get_status(FdMigrationState *s)
+int migrate_fd_get_status(MigrationState *s)
 {
     return s->state;
 }

-void migrate_fd_cancel(FdMigrationState *s)
+void migrate_fd_cancel(MigrationState *s)
 {
     if (s->state != MIG_STATE_ACTIVE)
         return;
@@ -423,7 +423,7 @@ void migrate_fd_cancel(FdMigrationState *s)
     migrate_fd_cleanup(s);
 }

-void migrate_fd_release(FdMigrationState *s)
+void migrate_fd_release(MigrationState *s)
 {

     DPRINTF("releasing state\n");
@@ -438,7 +438,7 @@ void migrate_fd_release(FdMigrationState *s)

 void migrate_fd_wait_for_unfreeze(void *opaque)
 {
-    FdMigrationState *s = opaque;
+    MigrationState *s = opaque;
     int ret;

     DPRINTF("wait for unfreeze\n");
@@ -461,7 +461,7 @@ void migrate_fd_wait_for_unfreeze(void *opaque)

 int migrate_fd_close(void *opaque)
 {
-    FdMigrationState *s = opaque;
+    MigrationState *s = opaque;

     if (s->mon) {
         monitor_resume(s->mon);
diff --git a/migration.h b/migration.h
index f0caf7b..e24efed 100644
--- a/migration.h
+++ b/migration.h
@@ -23,21 +23,21 @@
 #define MIG_STATE_CANCELLED	1
 #define MIG_STATE_ACTIVE	2

-typedef struct FdMigrationState FdMigrationState;
+typedef struct MigrationState MigrationState;

-struct FdMigrationState
+struct MigrationState
 {
     int64_t bandwidth_limit;
     QEMUFile *file;
     int fd;
     Monitor *mon;
     int state;
-    int (*get_error)(struct FdMigrationState*);
-    int (*close)(struct FdMigrationState*);
-    int (*write)(struct FdMigrationState*, const void *, size_t);
-    void (*cancel)(FdMigrationState *s);
-    int (*get_status)(FdMigrationState *s);
-    void (*release)(FdMigrationState *s);
+    int (*get_error)(MigrationState *s);
+    int (*close)(MigrationState *s);
+    int (*write)(MigrationState *s, const void *buff, size_t size);
+    void (*cancel)(MigrationState *s);
+    int (*get_status)(MigrationState *s);
+    void (*release)(MigrationState *s);
     void *opaque;
     int blk;
     int shared;
@@ -64,7 +64,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data);

 int exec_start_incoming_migration(const char *host_port);

-FdMigrationState *exec_start_outgoing_migration(Monitor *mon,
+MigrationState *exec_start_outgoing_migration(Monitor *mon,
                                               const char *host_port,
 					      int64_t bandwidth_limit,
 					      int detach,
@@ -73,7 +73,7 @@ FdMigrationState *exec_start_outgoing_migration(Monitor *mon,

 int tcp_start_incoming_migration(const char *host_port);

-FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,
+MigrationState *tcp_start_outgoing_migration(Monitor *mon,
                                              const char *host_port,
 					     int64_t bandwidth_limit,
 					     int detach,
@@ -82,7 +82,7 @@ FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,

 int unix_start_incoming_migration(const char *path);

-FdMigrationState *unix_start_outgoing_migration(Monitor *mon,
+MigrationState *unix_start_outgoing_migration(Monitor *mon,
                                               const char *path,
 					      int64_t bandwidth_limit,
 					      int detach,
@@ -91,32 +91,32 @@ FdMigrationState *unix_start_outgoing_migration(Monitor *mon,

 int fd_start_incoming_migration(const char *path);

-FdMigrationState *fd_start_outgoing_migration(Monitor *mon,
+MigrationState *fd_start_outgoing_migration(Monitor *mon,
 					    const char *fdname,
 					    int64_t bandwidth_limit,
 					    int detach,
 					    int blk,
 					    int inc);

-void migrate_fd_monitor_suspend(FdMigrationState *s, Monitor *mon);
+void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon);

-void migrate_fd_error(FdMigrationState *s);
+void migrate_fd_error(MigrationState *s);

-int migrate_fd_cleanup(FdMigrationState *s);
+int migrate_fd_cleanup(MigrationState *s);

 void migrate_fd_put_notify(void *opaque);

 ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size);

-void migrate_fd_connect(FdMigrationState *s);
+void migrate_fd_connect(MigrationState *s);

 void migrate_fd_put_ready(void *opaque);

-int migrate_fd_get_status(FdMigrationState *mig_state);
+int migrate_fd_get_status(MigrationState *mig_state);

-void migrate_fd_cancel(FdMigrationState *mig_state);
+void migrate_fd_cancel(MigrationState *mig_state);

-void migrate_fd_release(FdMigrationState *mig_state);
+void migrate_fd_release(MigrationState *mig_state);

 void migrate_fd_wait_for_unfreeze(void *opaque);

-- 
1.7.6.4

[Qemu-devel] [PATCH 19/36] migration: Refactor MigrationState creation

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
---
 migration-exec.c |   16 +---------------
 migration-fd.c   |   16 +---------------
 migration-tcp.c  |   15 +--------------
 migration-unix.c |   15 +--------------
 migration.c      |   29 +++++++++++++++++++++++++----
 migration.h      |   11 +++--------
 6 files changed, 32 insertions(+), 70 deletions(-)

diff --git a/migration-exec.c b/migration-exec.c
index 0ed5976..d0119c6 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -71,7 +71,7 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
     MigrationState *s;
     FILE *f;

-    s = g_malloc0(sizeof(*s));
+    s = migrate_new(mon, bandwidth_limit, detach, blk, inc);

     f = popen(command, "w");
     if (f == NULL) {
@@ -92,20 +92,6 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
     s->close = exec_close;
     s->get_error = file_errno;
     s->write = file_write;
-    s->cancel = migrate_fd_cancel;
-    s->get_status = migrate_fd_get_status;
-    s->release = migrate_fd_release;
-
-    s->blk = blk;
-    s->shared = inc;
-
-    s->state = MIG_STATE_ACTIVE;
-    s->mon = NULL;
-    s->bandwidth_limit = bandwidth_limit;
-
-    if (!detach) {
-        migrate_fd_monitor_suspend(s, mon);
-    }

     migrate_fd_connect(s);
     return s;
diff --git a/migration-fd.c b/migration-fd.c
index e78fd4e..9d3ca42 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -59,7 +59,7 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
 {
     MigrationState *s;

-    s = g_malloc0(sizeof(*s));
+    s = migrate_new(mon, bandwidth_limit, detach, blk, inc);

     s->fd = monitor_get_fd(mon, fdname);
     if (s->fd == -1) {
@@ -75,20 +75,6 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
     s->get_error = fd_errno;
     s->write = fd_write;
     s->close = fd_close;
-    s->cancel = migrate_fd_cancel;
-    s->get_status = migrate_fd_get_status;
-    s->release = migrate_fd_release;
-
-    s->blk = blk;
-    s->shared = inc;
-
-    s->state = MIG_STATE_ACTIVE;
-    s->mon = NULL;
-    s->bandwidth_limit = bandwidth_limit;
-
-    if (!detach) {
-        migrate_fd_monitor_suspend(s, mon);
-    }

     migrate_fd_connect(s);
     return s;
diff --git a/migration-tcp.c b/migration-tcp.c
index d6feb23..999d4c9 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -89,21 +89,12 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
     if (parse_host_port(&addr, host_port) < 0)
         return NULL;

-    s = g_malloc0(sizeof(*s));
+    s = migrate_new(mon, bandwidth_limit, detach, blk, inc);

     s->get_error = socket_errno;
     s->write = socket_write;
     s->close = tcp_close;
-    s->cancel = migrate_fd_cancel;
-    s->get_status = migrate_fd_get_status;
-    s->release = migrate_fd_release;

-    s->blk = blk;
-    s->shared = inc;
-
-    s->state = MIG_STATE_ACTIVE;
-    s->mon = NULL;
-    s->bandwidth_limit = bandwidth_limit;
     s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
     if (s->fd == -1) {
         g_free(s);
@@ -112,10 +103,6 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,

     socket_set_nonblock(s->fd);

-    if (!detach) {
-        migrate_fd_monitor_suspend(s, mon);
-    }
-
     do {
         ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
         if (ret == -1)
diff --git a/migration-unix.c b/migration-unix.c
index 3b9017b..bee71d9 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -88,21 +88,12 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
     addr.sun_family = AF_UNIX;
     snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path);

-    s = g_malloc0(sizeof(*s));
+    s = migrate_new(mon, bandwidth_limit, detach, blk, inc);

     s->get_error = unix_errno;
     s->write = unix_write;
     s->close = unix_close;
-    s->cancel = migrate_fd_cancel;
-    s->get_status = migrate_fd_get_status;
-    s->release = migrate_fd_release;

-    s->blk = blk;
-    s->shared = inc;
-
-    s->state = MIG_STATE_ACTIVE;
-    s->mon = NULL;
-    s->bandwidth_limit = bandwidth_limit;
     s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
     if (s->fd < 0) {
         DPRINTF("Unable to open socket");
@@ -125,10 +116,6 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
         goto err_after_open;
     }

-    if (!detach) {
-        migrate_fd_monitor_suspend(s, mon);
-    }
-
     if (ret >= 0)
         migrate_fd_connect(s);

diff --git a/migration.c b/migration.c
index 5509a8f..f80ad17 100644
--- a/migration.c
+++ b/migration.c
@@ -263,7 +263,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)

 /* shared migration helpers */

-void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon)
+static void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon)
 {
     s->mon = mon;
     if (monitor_suspend(mon) == 0) {
@@ -404,12 +404,12 @@ void migrate_fd_put_ready(void *opaque)
     }
 }

-int migrate_fd_get_status(MigrationState *s)
+static int migrate_fd_get_status(MigrationState *s)
 {
     return s->state;
 }

-void migrate_fd_cancel(MigrationState *s)
+static void migrate_fd_cancel(MigrationState *s)
 {
     if (s->state != MIG_STATE_ACTIVE)
         return;
@@ -423,7 +423,7 @@ void migrate_fd_cancel(MigrationState *s)
     migrate_fd_cleanup(s);
 }

-void migrate_fd_release(MigrationState *s)
+static void migrate_fd_release(MigrationState *s)
 {

     DPRINTF("releasing state\n");
@@ -488,3 +488,24 @@ int get_migration_state(void)
         return MIG_STATE_ERROR;
     }
 }
+
+MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
+                                     int detach, int blk, int inc)
+{
+    MigrationState *s = g_malloc0(sizeof(*s));
+
+    s->cancel = migrate_fd_cancel;
+    s->get_status = migrate_fd_get_status;
+    s->release = migrate_fd_release;
+    s->blk = blk;
+    s->shared = inc;
+    s->mon = NULL;
+    s->bandwidth_limit = bandwidth_limit;
+    s->state = MIG_STATE_ACTIVE;
+
+    if (!detach) {
+        migrate_fd_monitor_suspend(s, mon);
+    }
+
+    return s;
+}
diff --git a/migration.h b/migration.h
index e24efed..5f933e8 100644
--- a/migration.h
+++ b/migration.h
@@ -98,8 +98,6 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
 					    int blk,
 					    int inc);

-void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon);
-
 void migrate_fd_error(MigrationState *s);

 int migrate_fd_cleanup(MigrationState *s);
@@ -112,16 +110,13 @@ void migrate_fd_connect(MigrationState *s);

 void migrate_fd_put_ready(void *opaque);

-int migrate_fd_get_status(MigrationState *mig_state);
-
-void migrate_fd_cancel(MigrationState *mig_state);
-
-void migrate_fd_release(MigrationState *mig_state);
-
 void migrate_fd_wait_for_unfreeze(void *opaque);

 int migrate_fd_close(void *opaque);

+MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
+                            int detach, int blk, int inc);
+
 void add_migration_state_change_notifier(Notifier *notify);
 void remove_migration_state_change_notifier(Notifier *notify);
 int get_migration_state(void);
-- 
1.7.6.4

[Qemu-devel] [PATCH 20/36] migration: Make all posible migration functions static

[Juan Quintela]

I have to move two functions postions to avoid forward declarations

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |   72 +++++++++++++++++++++++++++++-----------------------------
 migration.h |   12 ---------
 2 files changed, 36 insertions(+), 48 deletions(-)

diff --git a/migration.c b/migration.c
index f80ad17..5a3e3ad 100644
--- a/migration.c
+++ b/migration.c
@@ -274,15 +274,7 @@ static void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon)
     }
 }

-void migrate_fd_error(MigrationState *s)
-{
-    DPRINTF("setting error state\n");
-    s->state = MIG_STATE_ERROR;
-    notifier_list_notify(&migration_state_notifiers, NULL);
-    migrate_fd_cleanup(s);
-}
-
-int migrate_fd_cleanup(MigrationState *s)
+static int migrate_fd_cleanup(MigrationState *s)
 {
     int ret = 0;

@@ -308,7 +300,15 @@ int migrate_fd_cleanup(MigrationState *s)
     return ret;
 }

-void migrate_fd_put_notify(void *opaque)
+void migrate_fd_error(MigrationState *s)
+{
+    DPRINTF("setting error state\n");
+    s->state = MIG_STATE_ERROR;
+    notifier_list_notify(&migration_state_notifiers, NULL);
+    migrate_fd_cleanup(s);
+}
+
+static void migrate_fd_put_notify(void *opaque)
 {
     MigrationState *s = opaque;

@@ -319,7 +319,8 @@ void migrate_fd_put_notify(void *opaque)
     }
 }

-ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
+static ssize_t migrate_fd_put_buffer(void *opaque, const void *data,
+                                     size_t size)
 {
     MigrationState *s = opaque;
     ssize_t ret;
@@ -342,29 +343,7 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
     return ret;
 }

-void migrate_fd_connect(MigrationState *s)
-{
-    int ret;
-
-    s->file = qemu_fopen_ops_buffered(s,
-                                      s->bandwidth_limit,
-                                      migrate_fd_put_buffer,
-                                      migrate_fd_put_ready,
-                                      migrate_fd_wait_for_unfreeze,
-                                      migrate_fd_close);
-
-    DPRINTF("beginning savevm\n");
-    ret = qemu_savevm_state_begin(s->mon, s->file, s->blk, s->shared);
-    if (ret < 0) {
-        DPRINTF("failed, %d\n", ret);
-        migrate_fd_error(s);
-        return;
-    }
-    
-    migrate_fd_put_ready(s);
-}
-
-void migrate_fd_put_ready(void *opaque)
+static void migrate_fd_put_ready(void *opaque)
 {
     MigrationState *s = opaque;
     int ret;
@@ -436,7 +415,7 @@ static void migrate_fd_release(MigrationState *s)
     g_free(s);
 }

-void migrate_fd_wait_for_unfreeze(void *opaque)
+static void migrate_fd_wait_for_unfreeze(void *opaque)
 {
     MigrationState *s = opaque;
     int ret;
@@ -459,7 +438,7 @@ void migrate_fd_wait_for_unfreeze(void *opaque)
     }
 }

-int migrate_fd_close(void *opaque)
+static int migrate_fd_close(void *opaque)
 {
     MigrationState *s = opaque;

@@ -489,6 +468,27 @@ int get_migration_state(void)
     }
 }

+void migrate_fd_connect(MigrationState *s)
+{
+    int ret;
+
+    s->file = qemu_fopen_ops_buffered(s,
+                                      s->bandwidth_limit,
+                                      migrate_fd_put_buffer,
+                                      migrate_fd_put_ready,
+                                      migrate_fd_wait_for_unfreeze,
+                                      migrate_fd_close);
+
+    DPRINTF("beginning savevm\n");
+    ret = qemu_savevm_state_begin(s->mon, s->file, s->blk, s->shared);
+    if (ret < 0) {
+        DPRINTF("failed, %d\n", ret);
+        migrate_fd_error(s);
+        return;
+    }
+    migrate_fd_put_ready(s);
+}
+
 MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
                                      int detach, int blk, int inc)
 {
diff --git a/migration.h b/migration.h
index 5f933e8..892b636 100644
--- a/migration.h
+++ b/migration.h
@@ -100,20 +100,8 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,

 void migrate_fd_error(MigrationState *s);

-int migrate_fd_cleanup(MigrationState *s);
-
-void migrate_fd_put_notify(void *opaque);
-
-ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size);
-
 void migrate_fd_connect(MigrationState *s);

-void migrate_fd_put_ready(void *opaque);
-
-void migrate_fd_wait_for_unfreeze(void *opaque);
-
-int migrate_fd_close(void *opaque);
-
 MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
                             int detach, int blk, int inc);

-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 20/36] migration: Make all posible migration functions static

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> I have to move two functions postions to avoid forward declarations
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   migration.c |   72 +++++++++++++++++++++++++++++-----------------------------
>   migration.h |   12 ---------
>   2 files changed, 36 insertions(+), 48 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index f80ad17..5a3e3ad 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -274,15 +274,7 @@ static void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon)
>       }
>   }
>
> -void migrate_fd_error(MigrationState *s)
> -{
> -    DPRINTF("setting error state\n");
> -    s->state = MIG_STATE_ERROR;
> -    notifier_list_notify(&migration_state_notifiers, NULL);
> -    migrate_fd_cleanup(s);
> -}
> -
> -int migrate_fd_cleanup(MigrationState *s)
> +static int migrate_fd_cleanup(MigrationState *s)
>   {
>       int ret = 0;
>
> @@ -308,7 +300,15 @@ int migrate_fd_cleanup(MigrationState *s)
>       return ret;
>   }
>
> -void migrate_fd_put_notify(void *opaque)
> +void migrate_fd_error(MigrationState *s)
> +{
> +    DPRINTF("setting error state\n");
> +    s->state = MIG_STATE_ERROR;
> +    notifier_list_notify(&migration_state_notifiers, NULL);
> +    migrate_fd_cleanup(s);
> +}
> +
> +static void migrate_fd_put_notify(void *opaque)
>   {
>       MigrationState *s = opaque;
>
> @@ -319,7 +319,8 @@ void migrate_fd_put_notify(void *opaque)
>       }
>   }
>
> -ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
> +static ssize_t migrate_fd_put_buffer(void *opaque, const void *data,
> +                                     size_t size)
>   {
>       MigrationState *s = opaque;
>       ssize_t ret;
> @@ -342,29 +343,7 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
>       return ret;
>   }
>
> -void migrate_fd_connect(MigrationState *s)
> -{
> -    int ret;
> -
> -    s->file = qemu_fopen_ops_buffered(s,
> -                                      s->bandwidth_limit,
> -                                      migrate_fd_put_buffer,
> -                                      migrate_fd_put_ready,
> -                                      migrate_fd_wait_for_unfreeze,
> -                                      migrate_fd_close);
> -
> -    DPRINTF("beginning savevm\n");
> -    ret = qemu_savevm_state_begin(s->mon, s->file, s->blk, s->shared);
> -    if (ret<  0) {
> -        DPRINTF("failed, %d\n", ret);
> -        migrate_fd_error(s);
> -        return;
> -    }
> -
> -    migrate_fd_put_ready(s);
> -}
> -
> -void migrate_fd_put_ready(void *opaque)
> +static void migrate_fd_put_ready(void *opaque)
>   {
>       MigrationState *s = opaque;
>       int ret;
> @@ -436,7 +415,7 @@ static void migrate_fd_release(MigrationState *s)
>       g_free(s);
>   }
>
> -void migrate_fd_wait_for_unfreeze(void *opaque)
> +static void migrate_fd_wait_for_unfreeze(void *opaque)
>   {
>       MigrationState *s = opaque;
>       int ret;
> @@ -459,7 +438,7 @@ void migrate_fd_wait_for_unfreeze(void *opaque)
>       }
>   }
>
> -int migrate_fd_close(void *opaque)
> +static int migrate_fd_close(void *opaque)
>   {
>       MigrationState *s = opaque;
>
> @@ -489,6 +468,27 @@ int get_migration_state(void)
>       }
>   }
>
> +void migrate_fd_connect(MigrationState *s)
> +{
> +    int ret;
> +
> +    s->file = qemu_fopen_ops_buffered(s,
> +                                      s->bandwidth_limit,
> +                                      migrate_fd_put_buffer,
> +                                      migrate_fd_put_ready,
> +                                      migrate_fd_wait_for_unfreeze,
> +                                      migrate_fd_close);
> +
> +    DPRINTF("beginning savevm\n");
> +    ret = qemu_savevm_state_begin(s->mon, s->file, s->blk, s->shared);
> +    if (ret<  0) {
> +        DPRINTF("failed, %d\n", ret);
> +        migrate_fd_error(s);
> +        return;
> +    }
> +    migrate_fd_put_ready(s);
> +}
> +
>   MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
>                                        int detach, int blk, int inc)
>   {
> diff --git a/migration.h b/migration.h
> index 5f933e8..892b636 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -100,20 +100,8 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
>
>   void migrate_fd_error(MigrationState *s);
>
> -int migrate_fd_cleanup(MigrationState *s);
> -
> -void migrate_fd_put_notify(void *opaque);
> -
> -ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size);
> -
>   void migrate_fd_connect(MigrationState *s);
>
> -void migrate_fd_put_ready(void *opaque);
> -
> -void migrate_fd_wait_for_unfreeze(void *opaque);
> -
> -int migrate_fd_close(void *opaque);
> -
>   MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
>                               int detach, int blk, int inc);
>

[Qemu-devel] [PATCH 21/36] migration: move migrate_new to do_migrate

[Juan Quintela]

Once there, remove all parameters that don't need to be passed to
*start_outgoing_migration() functions

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration-exec.c |   19 +++++--------------
 migration-fd.c   |   22 ++++++----------------
 migration-tcp.c  |   22 +++++++---------------
 migration-unix.c |   20 +++++---------------
 migration.c      |   32 +++++++++++++++++++-------------
 migration.h      |   31 ++++---------------------------
 6 files changed, 46 insertions(+), 100 deletions(-)

diff --git a/migration-exec.c b/migration-exec.c
index d0119c6..b7b1055 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -61,22 +61,14 @@ static int exec_close(MigrationState *s)
     return ret;
 }

-MigrationState *exec_start_outgoing_migration(Monitor *mon,
-                                              const char *command,
-					      int64_t bandwidth_limit,
-					      int detach,
-					      int blk,
-					      int inc)
+int exec_start_outgoing_migration(MigrationState *s, const char *command)
 {
-    MigrationState *s;
     FILE *f;

-    s = migrate_new(mon, bandwidth_limit, detach, blk, inc);
-
     f = popen(command, "w");
     if (f == NULL) {
         DPRINTF("Unable to popen exec target\n");
-        goto err_after_alloc;
+        goto err_after_popen;
     }

     s->fd = fileno(f);
@@ -94,13 +86,12 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
     s->write = file_write;

     migrate_fd_connect(s);
-    return s;
+    return 0;

 err_after_open:
     pclose(f);
-err_after_alloc:
-    g_free(s);
-    return NULL;
+err_after_popen:
+    return -1;
 }

 static void exec_accept_incoming_migration(void *opaque)
diff --git a/migration-fd.c b/migration-fd.c
index 9d3ca42..d0aec89 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -50,21 +50,12 @@ static int fd_close(MigrationState *s)
     return 0;
 }

-MigrationState *fd_start_outgoing_migration(Monitor *mon,
-					    const char *fdname,
-					    int64_t bandwidth_limit,
-					    int detach,
-					    int blk,
-					    int inc)
+int fd_start_outgoing_migration(MigrationState *s, const char *fdname)
 {
-    MigrationState *s;
-
-    s = migrate_new(mon, bandwidth_limit, detach, blk, inc);
-
-    s->fd = monitor_get_fd(mon, fdname);
+    s->fd = monitor_get_fd(s->mon, fdname);
     if (s->fd == -1) {
         DPRINTF("fd_migration: invalid file descriptor identifier\n");
-        goto err_after_alloc;
+        goto err_after_get_fd;
     }

     if (fcntl(s->fd, F_SETFL, O_NONBLOCK) == -1) {
@@ -77,13 +68,12 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
     s->close = fd_close;

     migrate_fd_connect(s);
-    return s;
+    return 0;

 err_after_open:
     close(s->fd);
-err_after_alloc:
-    g_free(s);
-    return NULL;
+err_after_get_fd:
+    return -1;
 }

 static void fd_accept_incoming_migration(void *opaque)
diff --git a/migration-tcp.c b/migration-tcp.c
index 999d4c9..f6b2288 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -75,30 +75,22 @@ static void tcp_wait_for_connect(void *opaque)
     }
 }

-MigrationState *tcp_start_outgoing_migration(Monitor *mon,
-                                             const char *host_port,
-                                             int64_t bandwidth_limit,
-                                             int detach,
-					     int blk,
-					     int inc)
+int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
 {
     struct sockaddr_in addr;
-    MigrationState *s;
     int ret;

-    if (parse_host_port(&addr, host_port) < 0)
-        return NULL;
-
-    s = migrate_new(mon, bandwidth_limit, detach, blk, inc);
-
+    ret = parse_host_port(&addr, host_port);
+    if (ret < 0) {
+        return ret;
+    }
     s->get_error = socket_errno;
     s->write = socket_write;
     s->close = tcp_close;

     s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
     if (s->fd == -1) {
-        g_free(s);
-        return NULL;
+        return -1;
     }

     socket_set_nonblock(s->fd);
@@ -118,7 +110,7 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
     } else if (ret >= 0)
         migrate_fd_connect(s);

-    return s;
+    return 0;
 }

 static void tcp_accept_incoming_migration(void *opaque)
diff --git a/migration-unix.c b/migration-unix.c
index bee71d9..bd8d40f 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -74,22 +74,13 @@ static void unix_wait_for_connect(void *opaque)
     }
 }

-MigrationState *unix_start_outgoing_migration(Monitor *mon,
-                                              const char *path,
-					      int64_t bandwidth_limit,
-					      int detach,
-					      int blk,
-					      int inc)
+int unix_start_outgoing_migration(MigrationState *s, const char *path)
 {
-    MigrationState *s;
     struct sockaddr_un addr;
     int ret;

     addr.sun_family = AF_UNIX;
     snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path);
-
-    s = migrate_new(mon, bandwidth_limit, detach, blk, inc);
-
     s->get_error = unix_errno;
     s->write = unix_write;
     s->close = unix_close;
@@ -97,7 +88,7 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
     s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
     if (s->fd < 0) {
         DPRINTF("Unable to open socket");
-        goto err_after_alloc;
+        goto err_after_socket;
     }

     socket_set_nonblock(s->fd);
@@ -119,14 +110,13 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
     if (ret >= 0)
         migrate_fd_connect(s);

-    return s;
+    return 0;

 err_after_open:
     close(s->fd);

-err_after_alloc:
-    g_free(s);
-    return NULL;
+err_after_socket:
+    return -1;
 }

 static void unix_accept_incoming_migration(void *opaque)
diff --git a/migration.c b/migration.c
index 5a3e3ad..e93f3f7 100644
--- a/migration.c
+++ b/migration.c
@@ -77,6 +77,9 @@ void process_incoming_migration(QEMUFile *f)
     }
 }

+static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
+                                   int detach, int blk, int inc);
+
 int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     MigrationState *s = NULL;
@@ -85,6 +88,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
     int blk = qdict_get_try_bool(qdict, "blk", 0);
     int inc = qdict_get_try_bool(qdict, "inc", 0);
     const char *uri = qdict_get_str(qdict, "uri");
+    int ret;

     if (current_migration &&
         current_migration->get_status(current_migration) == MIG_STATE_ACTIVE) {
@@ -96,28 +100,27 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
         return -1;
     }

+    s = migrate_new(mon, max_throttle, detach, blk, inc);
+
     if (strstart(uri, "tcp:", &p)) {
-        s = tcp_start_outgoing_migration(mon, p, max_throttle, detach,
-                                         blk, inc);
+        ret = tcp_start_outgoing_migration(s, p);
 #if !defined(WIN32)
     } else if (strstart(uri, "exec:", &p)) {
-        s = exec_start_outgoing_migration(mon, p, max_throttle, detach,
-                                          blk, inc);
+        ret = exec_start_outgoing_migration(s, p);
     } else if (strstart(uri, "unix:", &p)) {
-        s = unix_start_outgoing_migration(mon, p, max_throttle, detach,
-                                          blk, inc);
+        ret = unix_start_outgoing_migration(s, p);
     } else if (strstart(uri, "fd:", &p)) {
-        s = fd_start_outgoing_migration(mon, p, max_throttle, detach, 
-                                        blk, inc);
+        ret = fd_start_outgoing_migration(s, p);
 #endif
     } else {
         monitor_printf(mon, "unknown migration protocol: %s\n", uri);
-        return -1;
+        ret  = -EINVAL;
+        goto free_migrate_state;
     }

-    if (s == NULL) {
+    if (ret < 0) {
         monitor_printf(mon, "migration failed\n");
-        return -1;
+        goto free_migrate_state;
     }

     if (current_migration) {
@@ -127,6 +130,9 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
     current_migration = s;
     notifier_list_notify(&migration_state_notifiers, NULL);
     return 0;
+free_migrate_state:
+    g_free(s);
+    return -1;
 }

 int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
@@ -489,8 +495,8 @@ void migrate_fd_connect(MigrationState *s)
     migrate_fd_put_ready(s);
 }

-MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
-                                     int detach, int blk, int inc)
+static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
+                                   int detach, int blk, int inc)
 {
     MigrationState *s = g_malloc0(sizeof(*s));

diff --git a/migration.h b/migration.h
index 892b636..14c3ebc 100644
--- a/migration.h
+++ b/migration.h
@@ -64,47 +64,24 @@ void do_info_migrate(Monitor *mon, QObject **ret_data);

 int exec_start_incoming_migration(const char *host_port);

-MigrationState *exec_start_outgoing_migration(Monitor *mon,
-                                              const char *host_port,
-					      int64_t bandwidth_limit,
-					      int detach,
-					      int blk,
-					      int inc);
+int exec_start_outgoing_migration(MigrationState *s, const char *host_port);

 int tcp_start_incoming_migration(const char *host_port);

-MigrationState *tcp_start_outgoing_migration(Monitor *mon,
-                                             const char *host_port,
-					     int64_t bandwidth_limit,
-					     int detach,
-					     int blk,
-					     int inc);
+int tcp_start_outgoing_migration(MigrationState *s, const char *host_port);

 int unix_start_incoming_migration(const char *path);

-MigrationState *unix_start_outgoing_migration(Monitor *mon,
-                                              const char *path,
-					      int64_t bandwidth_limit,
-					      int detach,
-					      int blk,
-					      int inc);
+int unix_start_outgoing_migration(MigrationState *s, const char *path);

 int fd_start_incoming_migration(const char *path);

-MigrationState *fd_start_outgoing_migration(Monitor *mon,
-					    const char *fdname,
-					    int64_t bandwidth_limit,
-					    int detach,
-					    int blk,
-					    int inc);
+int fd_start_outgoing_migration(MigrationState *s, const char *fdname);

 void migrate_fd_error(MigrationState *s);

 void migrate_fd_connect(MigrationState *s);

-MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
-                            int detach, int blk, int inc);
-
 void add_migration_state_change_notifier(Notifier *notify);
 void remove_migration_state_change_notifier(Notifier *notify);
 int get_migration_state(void);
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 21/36] migration: move migrate_new to do_migrate

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> Once there, remove all parameters that don't need to be passed to
> *start_outgoing_migration() functions
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   migration-exec.c |   19 +++++--------------
>   migration-fd.c   |   22 ++++++----------------
>   migration-tcp.c  |   22 +++++++---------------
>   migration-unix.c |   20 +++++---------------
>   migration.c      |   32 +++++++++++++++++++-------------
>   migration.h      |   31 ++++---------------------------
>   6 files changed, 46 insertions(+), 100 deletions(-)
>
> diff --git a/migration-exec.c b/migration-exec.c
> index d0119c6..b7b1055 100644
> --- a/migration-exec.c
> +++ b/migration-exec.c
> @@ -61,22 +61,14 @@ static int exec_close(MigrationState *s)
>       return ret;
>   }
>
> -MigrationState *exec_start_outgoing_migration(Monitor *mon,
> -                                              const char *command,
> -					      int64_t bandwidth_limit,
> -					      int detach,
> -					      int blk,
> -					      int inc)
> +int exec_start_outgoing_migration(MigrationState *s, const char *command)
>   {
> -    MigrationState *s;
>       FILE *f;
>
> -    s = migrate_new(mon, bandwidth_limit, detach, blk, inc);
> -
>       f = popen(command, "w");
>       if (f == NULL) {
>           DPRINTF("Unable to popen exec target\n");
> -        goto err_after_alloc;
> +        goto err_after_popen;
>       }
>
>       s->fd = fileno(f);
> @@ -94,13 +86,12 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
>       s->write = file_write;
>
>       migrate_fd_connect(s);
> -    return s;
> +    return 0;
>
>   err_after_open:
>       pclose(f);
> -err_after_alloc:
> -    g_free(s);
> -    return NULL;
> +err_after_popen:
> +    return -1;
>   }
>
>   static void exec_accept_incoming_migration(void *opaque)
> diff --git a/migration-fd.c b/migration-fd.c
> index 9d3ca42..d0aec89 100644
> --- a/migration-fd.c
> +++ b/migration-fd.c
> @@ -50,21 +50,12 @@ static int fd_close(MigrationState *s)
>       return 0;
>   }
>
> -MigrationState *fd_start_outgoing_migration(Monitor *mon,
> -					    const char *fdname,
> -					    int64_t bandwidth_limit,
> -					    int detach,
> -					    int blk,
> -					    int inc)
> +int fd_start_outgoing_migration(MigrationState *s, const char *fdname)
>   {
> -    MigrationState *s;
> -
> -    s = migrate_new(mon, bandwidth_limit, detach, blk, inc);
> -
> -    s->fd = monitor_get_fd(mon, fdname);
> +    s->fd = monitor_get_fd(s->mon, fdname);
>       if (s->fd == -1) {
>           DPRINTF("fd_migration: invalid file descriptor identifier\n");
> -        goto err_after_alloc;
> +        goto err_after_get_fd;
>       }
>
>       if (fcntl(s->fd, F_SETFL, O_NONBLOCK) == -1) {
> @@ -77,13 +68,12 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
>       s->close = fd_close;
>
>       migrate_fd_connect(s);
> -    return s;
> +    return 0;
>
>   err_after_open:
>       close(s->fd);
> -err_after_alloc:
> -    g_free(s);
> -    return NULL;
> +err_after_get_fd:
> +    return -1;
>   }
>
>   static void fd_accept_incoming_migration(void *opaque)
> diff --git a/migration-tcp.c b/migration-tcp.c
> index 999d4c9..f6b2288 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -75,30 +75,22 @@ static void tcp_wait_for_connect(void *opaque)
>       }
>   }
>
> -MigrationState *tcp_start_outgoing_migration(Monitor *mon,
> -                                             const char *host_port,
> -                                             int64_t bandwidth_limit,
> -                                             int detach,
> -					     int blk,
> -					     int inc)
> +int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
>   {
>       struct sockaddr_in addr;
> -    MigrationState *s;
>       int ret;
>
> -    if (parse_host_port(&addr, host_port)<  0)
> -        return NULL;
> -
> -    s = migrate_new(mon, bandwidth_limit, detach, blk, inc);
> -
> +    ret = parse_host_port(&addr, host_port);
> +    if (ret<  0) {
> +        return ret;
> +    }
>       s->get_error = socket_errno;
>       s->write = socket_write;
>       s->close = tcp_close;
>
>       s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
>       if (s->fd == -1) {
> -        g_free(s);
> -        return NULL;
> +        return -1;
>       }
>
>       socket_set_nonblock(s->fd);
> @@ -118,7 +110,7 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
>       } else if (ret>= 0)
>           migrate_fd_connect(s);
>
> -    return s;
> +    return 0;
>   }
>
>   static void tcp_accept_incoming_migration(void *opaque)
> diff --git a/migration-unix.c b/migration-unix.c
> index bee71d9..bd8d40f 100644
> --- a/migration-unix.c
> +++ b/migration-unix.c
> @@ -74,22 +74,13 @@ static void unix_wait_for_connect(void *opaque)
>       }
>   }
>
> -MigrationState *unix_start_outgoing_migration(Monitor *mon,
> -                                              const char *path,
> -					      int64_t bandwidth_limit,
> -					      int detach,
> -					      int blk,
> -					      int inc)
> +int unix_start_outgoing_migration(MigrationState *s, const char *path)
>   {
> -    MigrationState *s;
>       struct sockaddr_un addr;
>       int ret;
>
>       addr.sun_family = AF_UNIX;
>       snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path);
> -
> -    s = migrate_new(mon, bandwidth_limit, detach, blk, inc);
> -
>       s->get_error = unix_errno;
>       s->write = unix_write;
>       s->close = unix_close;
> @@ -97,7 +88,7 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
>       s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
>       if (s->fd<  0) {
>           DPRINTF("Unable to open socket");
> -        goto err_after_alloc;
> +        goto err_after_socket;
>       }
>
>       socket_set_nonblock(s->fd);
> @@ -119,14 +110,13 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
>       if (ret>= 0)
>           migrate_fd_connect(s);
>
> -    return s;
> +    return 0;
>
>   err_after_open:
>       close(s->fd);
>
> -err_after_alloc:
> -    g_free(s);
> -    return NULL;
> +err_after_socket:
> +    return -1;
>   }
>
>   static void unix_accept_incoming_migration(void *opaque)
> diff --git a/migration.c b/migration.c
> index 5a3e3ad..e93f3f7 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -77,6 +77,9 @@ void process_incoming_migration(QEMUFile *f)
>       }
>   }
>
> +static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
> +                                   int detach, int blk, int inc);
> +
>   int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
>   {
>       MigrationState *s = NULL;
> @@ -85,6 +88,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
>       int blk = qdict_get_try_bool(qdict, "blk", 0);
>       int inc = qdict_get_try_bool(qdict, "inc", 0);
>       const char *uri = qdict_get_str(qdict, "uri");
> +    int ret;
>
>       if (current_migration&&
>           current_migration->get_status(current_migration) == MIG_STATE_ACTIVE) {
> @@ -96,28 +100,27 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
>           return -1;
>       }
>
> +    s = migrate_new(mon, max_throttle, detach, blk, inc);
> +
>       if (strstart(uri, "tcp:",&p)) {
> -        s = tcp_start_outgoing_migration(mon, p, max_throttle, detach,
> -                                         blk, inc);
> +        ret = tcp_start_outgoing_migration(s, p);
>   #if !defined(WIN32)
>       } else if (strstart(uri, "exec:",&p)) {
> -        s = exec_start_outgoing_migration(mon, p, max_throttle, detach,
> -                                          blk, inc);
> +        ret = exec_start_outgoing_migration(s, p);
>       } else if (strstart(uri, "unix:",&p)) {
> -        s = unix_start_outgoing_migration(mon, p, max_throttle, detach,
> -                                          blk, inc);
> +        ret = unix_start_outgoing_migration(s, p);
>       } else if (strstart(uri, "fd:",&p)) {
> -        s = fd_start_outgoing_migration(mon, p, max_throttle, detach,
> -                                        blk, inc);
> +        ret = fd_start_outgoing_migration(s, p);
>   #endif
>       } else {
>           monitor_printf(mon, "unknown migration protocol: %s\n", uri);
> -        return -1;
> +        ret  = -EINVAL;
> +        goto free_migrate_state;
>       }
>
> -    if (s == NULL) {
> +    if (ret<  0) {
>           monitor_printf(mon, "migration failed\n");
> -        return -1;
> +        goto free_migrate_state;
>       }
>
>       if (current_migration) {
> @@ -127,6 +130,9 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
>       current_migration = s;
>       notifier_list_notify(&migration_state_notifiers, NULL);
>       return 0;
> +free_migrate_state:
> +    g_free(s);
> +    return -1;
>   }
>
>   int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
> @@ -489,8 +495,8 @@ void migrate_fd_connect(MigrationState *s)
>       migrate_fd_put_ready(s);
>   }
>
> -MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
> -                                     int detach, int blk, int inc)
> +static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
> +                                   int detach, int blk, int inc)
>   {
>       MigrationState *s = g_malloc0(sizeof(*s));
>
> diff --git a/migration.h b/migration.h
> index 892b636..14c3ebc 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -64,47 +64,24 @@ void do_info_migrate(Monitor *mon, QObject **ret_data);
>
>   int exec_start_incoming_migration(const char *host_port);
>
> -MigrationState *exec_start_outgoing_migration(Monitor *mon,
> -                                              const char *host_port,
> -					      int64_t bandwidth_limit,
> -					      int detach,
> -					      int blk,
> -					      int inc);
> +int exec_start_outgoing_migration(MigrationState *s, const char *host_port);
>
>   int tcp_start_incoming_migration(const char *host_port);
>
> -MigrationState *tcp_start_outgoing_migration(Monitor *mon,
> -                                             const char *host_port,
> -					     int64_t bandwidth_limit,
> -					     int detach,
> -					     int blk,
> -					     int inc);
> +int tcp_start_outgoing_migration(MigrationState *s, const char *host_port);
>
>   int unix_start_incoming_migration(const char *path);
>
> -MigrationState *unix_start_outgoing_migration(Monitor *mon,
> -                                              const char *path,
> -					      int64_t bandwidth_limit,
> -					      int detach,
> -					      int blk,
> -					      int inc);
> +int unix_start_outgoing_migration(MigrationState *s, const char *path);
>
>   int fd_start_incoming_migration(const char *path);
>
> -MigrationState *fd_start_outgoing_migration(Monitor *mon,
> -					    const char *fdname,
> -					    int64_t bandwidth_limit,
> -					    int detach,
> -					    int blk,
> -					    int inc);
> +int fd_start_outgoing_migration(MigrationState *s, const char *fdname);
>
>   void migrate_fd_error(MigrationState *s);
>
>   void migrate_fd_connect(MigrationState *s);
>
> -MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
> -                            int detach, int blk, int inc);
> -
>   void add_migration_state_change_notifier(Notifier *notify);
>   void remove_migration_state_change_notifier(Notifier *notify);
>   int get_migration_state(void);

[Qemu-devel] [PATCH 22/36] migration: Introduce MIG_STATE_SETUP

[Juan Quintela]

Use MIG_STATE_ACTIVE only when migration has really started.  Use this
new state to setup migration parameters.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |    6 +++++-
 migration.h |   11 +++++++----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/migration.c b/migration.c
index e93f3f7..a01bf4f 100644
--- a/migration.c
+++ b/migration.c
@@ -239,6 +239,9 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
         MigrationState *s = current_migration;

         switch (s->get_status(current_migration)) {
+        case MIG_STATE_SETUP:
+            /* no migration has happened ever */
+            break;
         case MIG_STATE_ACTIVE:
             qdict = qdict_new();
             qdict_put(qdict, "status", qstring_from_str("active"));
@@ -478,6 +481,7 @@ void migrate_fd_connect(MigrationState *s)
 {
     int ret;

+    s->state = MIG_STATE_ACTIVE;
     s->file = qemu_fopen_ops_buffered(s,
                                       s->bandwidth_limit,
                                       migrate_fd_put_buffer,
@@ -507,7 +511,7 @@ static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
     s->shared = inc;
     s->mon = NULL;
     s->bandwidth_limit = bandwidth_limit;
-    s->state = MIG_STATE_ACTIVE;
+    s->state = MIG_STATE_SETUP;

     if (!detach) {
         migrate_fd_monitor_suspend(s, mon);
diff --git a/migration.h b/migration.h
index 14c3ebc..3165140 100644
--- a/migration.h
+++ b/migration.h
@@ -18,10 +18,13 @@
 #include "qemu-common.h"
 #include "notify.h"

-#define MIG_STATE_ERROR		-1
-#define MIG_STATE_COMPLETED	0
-#define MIG_STATE_CANCELLED	1
-#define MIG_STATE_ACTIVE	2
+enum migration_state {
+    MIG_STATE_ERROR,
+    MIG_STATE_SETUP,
+    MIG_STATE_CANCELLED,
+    MIG_STATE_ACTIVE,
+    MIG_STATE_COMPLETED,
+};

 typedef struct MigrationState MigrationState;

-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 22/36] migration: Introduce MIG_STATE_SETUP

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> Use MIG_STATE_ACTIVE only when migration has really started.  Use this
> new state to setup migration parameters.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>
> ---
>   migration.c |    6 +++++-
>   migration.h |   11 +++++++----
>   2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index e93f3f7..a01bf4f 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -239,6 +239,9 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
>           MigrationState *s = current_migration;
>
>           switch (s->get_status(current_migration)) {
> +        case MIG_STATE_SETUP:
> +            /* no migration has happened ever */
> +            break;
>           case MIG_STATE_ACTIVE:
>               qdict = qdict_new();
>               qdict_put(qdict, "status", qstring_from_str("active"));
> @@ -478,6 +481,7 @@ void migrate_fd_connect(MigrationState *s)
>   {
>       int ret;
>
> +    s->state = MIG_STATE_ACTIVE;
>       s->file = qemu_fopen_ops_buffered(s,
>                                         s->bandwidth_limit,
>                                         migrate_fd_put_buffer,
> @@ -507,7 +511,7 @@ static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
>       s->shared = inc;
>       s->mon = NULL;
>       s->bandwidth_limit = bandwidth_limit;
> -    s->state = MIG_STATE_ACTIVE;
> +    s->state = MIG_STATE_SETUP;
>
>       if (!detach) {
>           migrate_fd_monitor_suspend(s, mon);
> diff --git a/migration.h b/migration.h
> index 14c3ebc..3165140 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -18,10 +18,13 @@
>   #include "qemu-common.h"
>   #include "notify.h"
>
> -#define MIG_STATE_ERROR		-1
> -#define MIG_STATE_COMPLETED	0
> -#define MIG_STATE_CANCELLED	1
> -#define MIG_STATE_ACTIVE	2
> +enum migration_state {

CODING_STYLE.

Regards,

Anthony Liguori

> +    MIG_STATE_ERROR,
> +    MIG_STATE_SETUP,
> +    MIG_STATE_CANCELLED,
> +    MIG_STATE_ACTIVE,
> +    MIG_STATE_COMPLETED,
> +};
>
>   typedef struct MigrationState MigrationState;
>

Re: [Qemu-devel] [PATCH 22/36] migration: Introduce MIG_STATE_SETUP

[Juan Quintela]

Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 10/11/2011 05:00 AM, Juan Quintela wrote:
> CODING_STYLE.
>
> Regards,
>
> Anthony Liguori

Attached v2 of this patch.  Only change is s/enum migration_state/enum/

>From 990ccd38f0ff916ada859d28831b2be63983c309 Mon Sep 17 00:00:00 2001
Message-Id: <990ccd38f0ff916ada859d28831b2be63983c309.1318901257.git.quintela@redhat.com>
In-Reply-To: <cover.1318901257.git.quintela@redhat.com>
References: <cover.1318901257.git.quintela@redhat.com>
From: Juan Quintela <quintela@redhat.com>
Date: Tue, 11 May 2010 23:01:53 +0200
Subject: [PATCH 22/36] migration: Introduce MIG_STATE_SETUP

Use MIG_STATE_ACTIVE only when migration has really started.  Use this
new state to setup migration parameters.  Change defines for an
anonymous struct.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |    6 +++++-
 migration.h |   11 +++++++----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/migration.c b/migration.c
index e93f3f7..a01bf4f 100644
--- a/migration.c
+++ b/migration.c
@@ -239,6 +239,9 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
         MigrationState *s = current_migration;

         switch (s->get_status(current_migration)) {
+        case MIG_STATE_SETUP:
+            /* no migration has happened ever */
+            break;
         case MIG_STATE_ACTIVE:
             qdict = qdict_new();
             qdict_put(qdict, "status", qstring_from_str("active"));
@@ -478,6 +481,7 @@ void migrate_fd_connect(MigrationState *s)
 {
     int ret;

+    s->state = MIG_STATE_ACTIVE;
     s->file = qemu_fopen_ops_buffered(s,
                                       s->bandwidth_limit,
                                       migrate_fd_put_buffer,
@@ -507,7 +511,7 @@ static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
     s->shared = inc;
     s->mon = NULL;
     s->bandwidth_limit = bandwidth_limit;
-    s->state = MIG_STATE_ACTIVE;
+    s->state = MIG_STATE_SETUP;

     if (!detach) {
         migrate_fd_monitor_suspend(s, mon);
diff --git a/migration.h b/migration.h
index 14c3ebc..fed1cf1 100644
--- a/migration.h
+++ b/migration.h
@@ -18,10 +18,13 @@
 #include "qemu-common.h"
 #include "notify.h"

-#define MIG_STATE_ERROR		-1
-#define MIG_STATE_COMPLETED	0
-#define MIG_STATE_CANCELLED	1
-#define MIG_STATE_ACTIVE	2
+enum {
+    MIG_STATE_ERROR,
+    MIG_STATE_SETUP,
+    MIG_STATE_CANCELLED,
+    MIG_STATE_ACTIVE,
+    MIG_STATE_COMPLETED,
+};

 typedef struct MigrationState MigrationState;

-- 
1.7.6.4

[Qemu-devel] [PATCH 23/36] migration: Refactor and simplify error checking in migrate_fd_put_ready

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/migration.c b/migration.c
index a01bf4f..432afe6 100644
--- a/migration.c
+++ b/migration.c
@@ -372,23 +372,22 @@ static void migrate_fd_put_ready(void *opaque)
         DPRINTF("done iterating\n");
         vm_stop(RUN_STATE_FINISH_MIGRATE);

-        if ((qemu_savevm_state_complete(s->mon, s->file)) < 0) {
-            if (old_vm_running) {
-                vm_start();
+        if (qemu_savevm_state_complete(s->mon, s->file) < 0) {
+            migrate_fd_error(s);
+        } else {
+            if (migrate_fd_cleanup(s) < 0) {
+                migrate_fd_error(s);
+            } else {
+                s->state = MIG_STATE_COMPLETED;
+                runstate_set(RUN_STATE_POSTMIGRATE);
+                notifier_list_notify(&migration_state_notifiers, NULL);
             }
-            s->state = MIG_STATE_ERROR;
         }
-        if (migrate_fd_cleanup(s) < 0) {
+        if (s->get_status(s) != MIG_STATE_COMPLETED) {
             if (old_vm_running) {
                 vm_start();
             }
-            s->state = MIG_STATE_ERROR;
         }
-        if (s->state == MIG_STATE_ACTIVE) {
-            s->state = MIG_STATE_COMPLETED;
-            runstate_set(RUN_STATE_POSTMIGRATE);
-        }
-        notifier_list_notify(&migration_state_notifiers, NULL);
     }
 }

-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 23/36] migration: Refactor and simplify error checking in migrate_fd_put_ready

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Just a general comment.  This series is fairly touch to review because you're 
repeated refactoring the same function with no other comments beyond "refactor 
and simplify".

As a reviewer, I really have no help in understanding your motiviation for 
making this changes, why it can't be done all at once, etc.

I'm not rejecting this patch, but in the future, please put a little more care 
into better rationale in commit messages to simplify reviewing.

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori


> ---
>   migration.c |   21 ++++++++++-----------
>   1 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index a01bf4f..432afe6 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -372,23 +372,22 @@ static void migrate_fd_put_ready(void *opaque)
>           DPRINTF("done iterating\n");
>           vm_stop(RUN_STATE_FINISH_MIGRATE);
>
> -        if ((qemu_savevm_state_complete(s->mon, s->file))<  0) {
> -            if (old_vm_running) {
> -                vm_start();
> +        if (qemu_savevm_state_complete(s->mon, s->file)<  0) {
> +            migrate_fd_error(s);
> +        } else {
> +            if (migrate_fd_cleanup(s)<  0) {
> +                migrate_fd_error(s);
> +            } else {
> +                s->state = MIG_STATE_COMPLETED;
> +                runstate_set(RUN_STATE_POSTMIGRATE);
> +                notifier_list_notify(&migration_state_notifiers, NULL);
>               }
> -            s->state = MIG_STATE_ERROR;
>           }
> -        if (migrate_fd_cleanup(s)<  0) {
> +        if (s->get_status(s) != MIG_STATE_COMPLETED) {
>               if (old_vm_running) {
>                   vm_start();
>               }
> -            s->state = MIG_STATE_ERROR;
>           }
> -        if (s->state == MIG_STATE_ACTIVE) {
> -            s->state = MIG_STATE_COMPLETED;
> -            runstate_set(RUN_STATE_POSTMIGRATE);
> -        }
> -        notifier_list_notify(&migration_state_notifiers, NULL);
>       }
>   }
>

[Qemu-devel] [PATCH 24/36] migration: Introduce migrate_fd_completed() for consistency

[Juan Quintela]

This function is a bit different of the others that change the state,
in the sense that if migrate_fd_cleanup() returns an error, it set the
status to error, not completed.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |   20 +++++++++++++-------
 1 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/migration.c b/migration.c
index 432afe6..a8e936e 100644
--- a/migration.c
+++ b/migration.c
@@ -317,6 +317,18 @@ void migrate_fd_error(MigrationState *s)
     migrate_fd_cleanup(s);
 }

+static void migrate_fd_completed(MigrationState *s)
+{
+    DPRINTF("setting completed state\n");
+    if (migrate_fd_cleanup(s) < 0) {
+        s->state = MIG_STATE_ERROR;
+    } else {
+        s->state = MIG_STATE_COMPLETED;
+        runstate_set(RUN_STATE_POSTMIGRATE);
+    }
+    notifier_list_notify(&migration_state_notifiers, NULL);
+}
+
 static void migrate_fd_put_notify(void *opaque)
 {
     MigrationState *s = opaque;
@@ -375,13 +387,7 @@ static void migrate_fd_put_ready(void *opaque)
         if (qemu_savevm_state_complete(s->mon, s->file) < 0) {
             migrate_fd_error(s);
         } else {
-            if (migrate_fd_cleanup(s) < 0) {
-                migrate_fd_error(s);
-            } else {
-                s->state = MIG_STATE_COMPLETED;
-                runstate_set(RUN_STATE_POSTMIGRATE);
-                notifier_list_notify(&migration_state_notifiers, NULL);
-            }
+            migrate_fd_completed(s);
         }
         if (s->get_status(s) != MIG_STATE_COMPLETED) {
             if (old_vm_running) {
-- 
1.7.6.4

[Qemu-devel] [PATCH 25/36] migration: Our release callback was just free

[Juan Quintela]

We called it from a single place, and always with state !=
MIG_STATE_ACTIVE.  Just remove the whole callback.  For users of the
notifier, notice that this is exactly the case where they don't care,
we are just freeing the state from previous failed migration (it can't
be a sucessful one, otherwise we would not be running on that machine
in the first place).

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |   19 +------------------
 migration.h |    1 -
 2 files changed, 1 insertions(+), 19 deletions(-)

diff --git a/migration.c b/migration.c
index a8e936e..689464d 100644
--- a/migration.c
+++ b/migration.c
@@ -123,10 +123,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
         goto free_migrate_state;
     }

-    if (current_migration) {
-        current_migration->release(current_migration);
-    }
-
+    g_free(current_migration);
     current_migration = s;
     notifier_list_notify(&migration_state_notifiers, NULL);
     return 0;
@@ -416,19 +413,6 @@ static void migrate_fd_cancel(MigrationState *s)
     migrate_fd_cleanup(s);
 }

-static void migrate_fd_release(MigrationState *s)
-{
-
-    DPRINTF("releasing state\n");
-   
-    if (s->state == MIG_STATE_ACTIVE) {
-        s->state = MIG_STATE_CANCELLED;
-        notifier_list_notify(&migration_state_notifiers, NULL);
-        migrate_fd_cleanup(s);
-    }
-    g_free(s);
-}
-
 static void migrate_fd_wait_for_unfreeze(void *opaque)
 {
     MigrationState *s = opaque;
@@ -511,7 +495,6 @@ static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,

     s->cancel = migrate_fd_cancel;
     s->get_status = migrate_fd_get_status;
-    s->release = migrate_fd_release;
     s->blk = blk;
     s->shared = inc;
     s->mon = NULL;
diff --git a/migration.h b/migration.h
index 3165140..1cdb539 100644
--- a/migration.h
+++ b/migration.h
@@ -40,7 +40,6 @@ struct MigrationState
     int (*write)(MigrationState *s, const void *buff, size_t size);
     void (*cancel)(MigrationState *s);
     int (*get_status)(MigrationState *s);
-    void (*release)(MigrationState *s);
     void *opaque;
     int blk;
     int shared;
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 25/36] migration: Our release callback was just free

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> We called it from a single place, and always with state !=
> MIG_STATE_ACTIVE.  Just remove the whole callback.  For users of the
> notifier, notice that this is exactly the case where they don't care,
> we are just freeing the state from previous failed migration (it can't
> be a sucessful one, otherwise we would not be running on that machine
> in the first place).
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>
> ---
>   migration.c |   19 +------------------
>   migration.h |    1 -
>   2 files changed, 1 insertions(+), 19 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index a8e936e..689464d 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -123,10 +123,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
>           goto free_migrate_state;
>       }
>
> -    if (current_migration) {
> -        current_migration->release(current_migration);
> -    }
> -
> +    g_free(current_migration);

migrate_fd_cleanup() is no longer being called.

Regards,

Anthony Liguori

>       current_migration = s;
>       notifier_list_notify(&migration_state_notifiers, NULL);
>       return 0;
> @@ -416,19 +413,6 @@ static void migrate_fd_cancel(MigrationState *s)
>       migrate_fd_cleanup(s);
>   }
>
> -static void migrate_fd_release(MigrationState *s)
> -{
> -
> -    DPRINTF("releasing state\n");
> -
> -    if (s->state == MIG_STATE_ACTIVE) {
> -        s->state = MIG_STATE_CANCELLED;
> -        notifier_list_notify(&migration_state_notifiers, NULL);
> -        migrate_fd_cleanup(s);
> -    }
> -    g_free(s);
> -}
> -
>   static void migrate_fd_wait_for_unfreeze(void *opaque)
>   {
>       MigrationState *s = opaque;
> @@ -511,7 +495,6 @@ static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
>
>       s->cancel = migrate_fd_cancel;
>       s->get_status = migrate_fd_get_status;
> -    s->release = migrate_fd_release;
>       s->blk = blk;
>       s->shared = inc;
>       s->mon = NULL;
> diff --git a/migration.h b/migration.h
> index 3165140..1cdb539 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -40,7 +40,6 @@ struct MigrationState
>       int (*write)(MigrationState *s, const void *buff, size_t size);
>       void (*cancel)(MigrationState *s);
>       int (*get_status)(MigrationState *s);
> -    void (*release)(MigrationState *s);
>       void *opaque;
>       int blk;
>       int shared;

Re: [Qemu-devel] [PATCH 25/36] migration: Our release callback was just free

[Juan Quintela]

Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 10/11/2011 05:00 AM, Juan Quintela wrote:
>> We called it from a single place, and always with state !=
>> MIG_STATE_ACTIVE.  Just remove the whole callback.  For users of the
>> notifier, notice that this is exactly the case where they don't care,
>> we are just freeing the state from previous failed migration (it can't
>> be a sucessful one, otherwise we would not be running on that machine
>> in the first place).
>>
>> Signed-off-by: Juan Quintela<quintela@redhat.com>
>> ---
>>   migration.c |   19 +------------------
>>   migration.h |    1 -
>>   2 files changed, 1 insertions(+), 19 deletions(-)
>>
>> diff --git a/migration.c b/migration.c
>> index a8e936e..689464d 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -123,10 +123,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
>>           goto free_migrate_state;
>>       }
>>
>> -    if (current_migration) {
>> -        current_migration->release(current_migration);
>> -    }
>> -
>> +    g_free(current_migration);
>
> migrate_fd_cleanup() is no longer being called.

It was never called.

> Regards,
>
> Anthony Liguori
>
>>       current_migration = s;
>>       notifier_list_notify(&migration_state_notifiers, NULL);
>>       return 0;
>> @@ -416,19 +413,6 @@ static void migrate_fd_cancel(MigrationState *s)
>>       migrate_fd_cleanup(s);
>>   }
>>
>> -static void migrate_fd_release(MigrationState *s)
>> -{
>> -
>> -    DPRINTF("releasing state\n");
>> -
>> -    if (s->state == MIG_STATE_ACTIVE) {

The first thing that we look is that MIG_STATE_ACTIVE, and we call
migrate_fd_cleanup() on that case.


See the beggining of do_migrate().  If we are in MIG_STATE_ACTIVE, we
just don't continue. the function.  Notice that you complained about
comments being bad on the other patches, and in this very case, it was
explicitely stated on the comment O:-)

int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
    MigrationState *s = NULL;
    const char *p;
    int detach = qdict_get_try_bool(qdict, "detach", 0);
    int blk = qdict_get_try_bool(qdict, "blk", 0);
    int inc = qdict_get_try_bool(qdict, "inc", 0);
    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");
        return -1;

        ************ We stop here ****
        call to ->release() is below this.
    }

    if (qemu_savevm_state_blocked(mon)) {
        return -1;
    }



>> -        s->state = MIG_STATE_CANCELLED;
>> -        notifier_list_notify(&migration_state_notifiers, NULL);
>> -        migrate_fd_cleanup(s);
>> -    }
>> -    g_free(s);
>> -}
>> -
>>   static void migrate_fd_wait_for_unfreeze(void *opaque)
>>   {
>>       MigrationState *s = opaque;
>> @@ -511,7 +495,6 @@ static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
>>
>>       s->cancel = migrate_fd_cancel;
>>       s->get_status = migrate_fd_get_status;
>> -    s->release = migrate_fd_release;
>>       s->blk = blk;
>>       s->shared = inc;
>>       s->mon = NULL;
>> diff --git a/migration.h b/migration.h
>> index 3165140..1cdb539 100644
>> --- a/migration.h
>> +++ b/migration.h
>> @@ -40,7 +40,6 @@ struct MigrationState
>>       int (*write)(MigrationState *s, const void *buff, size_t size);
>>       void (*cancel)(MigrationState *s);
>>       int (*get_status)(MigrationState *s);
>> -    void (*release)(MigrationState *s);
>>       void *opaque;
>>       int blk;
>>       int shared;

Re: [Qemu-devel] [PATCH 25/36] migration: Our release callback was just free

[Anthony Liguori]

On 10/17/2011 10:12 AM, Juan Quintela wrote:
> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> On 10/11/2011 05:00 AM, Juan Quintela wrote:
>>> We called it from a single place, and always with state !=
>>> MIG_STATE_ACTIVE.  Just remove the whole callback.  For users of the
>>> notifier, notice that this is exactly the case where they don't care,
>>> we are just freeing the state from previous failed migration (it can't
>>> be a sucessful one, otherwise we would not be running on that machine
>>> in the first place).
>>>
>>> Signed-off-by: Juan Quintela<quintela@redhat.com>
>>> ---
>>>    migration.c |   19 +------------------
>>>    migration.h |    1 -
>>>    2 files changed, 1 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/migration.c b/migration.c
>>> index a8e936e..689464d 100644
>>> --- a/migration.c
>>> +++ b/migration.c
>>> @@ -123,10 +123,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
>>>            goto free_migrate_state;
>>>        }
>>>
>>> -    if (current_migration) {
>>> -        current_migration->release(current_migration);
>>> -    }
>>> -
>>> +    g_free(current_migration);
>>
>> migrate_fd_cleanup() is no longer being called.
>
> It was never called.
>
>> Regards,
>>
>> Anthony Liguori
>>
>>>        current_migration = s;
>>>        notifier_list_notify(&migration_state_notifiers, NULL);
>>>        return 0;
>>> @@ -416,19 +413,6 @@ static void migrate_fd_cancel(MigrationState *s)
>>>        migrate_fd_cleanup(s);
>>>    }
>>>
>>> -static void migrate_fd_release(MigrationState *s)
>>> -{
>>> -
>>> -    DPRINTF("releasing state\n");
>>> -
>>> -    if (s->state == MIG_STATE_ACTIVE) {
>
> The first thing that we look is that MIG_STATE_ACTIVE, and we call
> migrate_fd_cleanup() on that case.

It was meant to cancel existing migration but there was a later patch that 
didn't allow the implicit cancel anymore.

So ignore my comments here.

Regards,

Anthony Liguori

>
>
> See the beggining of do_migrate().  If we are in MIG_STATE_ACTIVE, we
> just don't continue. the function.  Notice that you complained about
> comments being bad on the other patches, and in this very case, it was
> explicitely stated on the comment O:-)
>
> int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
> {
>      MigrationState *s = NULL;
>      const char *p;
>      int detach = qdict_get_try_bool(qdict, "detach", 0);
>      int blk = qdict_get_try_bool(qdict, "blk", 0);
>      int inc = qdict_get_try_bool(qdict, "inc", 0);
>      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");
>          return -1;
>
>          ************ We stop here ****
>          call to ->release() is below this.
>      }
>
>      if (qemu_savevm_state_blocked(mon)) {
>          return -1;
>      }
>
>
>
>>> -        s->state = MIG_STATE_CANCELLED;
>>> -        notifier_list_notify(&migration_state_notifiers, NULL);
>>> -        migrate_fd_cleanup(s);
>>> -    }
>>> -    g_free(s);
>>> -}
>>> -
>>>    static void migrate_fd_wait_for_unfreeze(void *opaque)
>>>    {
>>>        MigrationState *s = opaque;
>>> @@ -511,7 +495,6 @@ static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
>>>
>>>        s->cancel = migrate_fd_cancel;
>>>        s->get_status = migrate_fd_get_status;
>>> -    s->release = migrate_fd_release;
>>>        s->blk = blk;
>>>        s->shared = inc;
>>>        s->mon = NULL;
>>> diff --git a/migration.h b/migration.h
>>> index 3165140..1cdb539 100644
>>> --- a/migration.h
>>> +++ b/migration.h
>>> @@ -40,7 +40,6 @@ struct MigrationState
>>>        int (*write)(MigrationState *s, const void *buff, size_t size);
>>>        void (*cancel)(MigrationState *s);
>>>        int (*get_status)(MigrationState *s);
>>> -    void (*release)(MigrationState *s);
>>>        void *opaque;
>>>        int blk;
>>>        int shared;
>

Re: [Qemu-devel] [PATCH 25/36] migration: Our release callback was just free

[Anthony Liguori]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> We called it from a single place, and always with state !=
> MIG_STATE_ACTIVE.  Just remove the whole callback.  For users of the
> notifier, notice that this is exactly the case where they don't care,
> we are just freeing the state from previous failed migration (it can't
> be a sucessful one, otherwise we would not be running on that machine
> in the first place).
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>
> ---
>   migration.c |   19 +------------------
>   migration.h |    1 -
>   2 files changed, 1 insertions(+), 19 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index a8e936e..689464d 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -123,10 +123,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
>           goto free_migrate_state;
>       }
>
> -    if (current_migration) {
> -        current_migration->release(current_migration);
> -    }

So ->release() calls migrate_fd_release() which if (s->state == 
MIG_STATE_ACTIVE) changes the state to MIG_STATE_CANCELLED and calls 
migrate_fd_cleanup().

This path is triggered *if* you start a migration while another is active.

Now you just free() the old migration and never bother cancelling it (and 
calling migrate_fd_cleanup).

Regards,

Anthony Liguori

> -
> +    g_free(current_migration);
>       current_migration = s;
>       notifier_list_notify(&migration_state_notifiers, NULL);
>       return 0;
> @@ -416,19 +413,6 @@ static void migrate_fd_cancel(MigrationState *s)
>       migrate_fd_cleanup(s);
>   }
>
> -static void migrate_fd_release(MigrationState *s)
> -{
> -
> -    DPRINTF("releasing state\n");
> -
> -    if (s->state == MIG_STATE_ACTIVE) {
> -        s->state = MIG_STATE_CANCELLED;
> -        notifier_list_notify(&migration_state_notifiers, NULL);
> -        migrate_fd_cleanup(s);
> -    }
> -    g_free(s);
> -}
> -
>   static void migrate_fd_wait_for_unfreeze(void *opaque)
>   {
>       MigrationState *s = opaque;
> @@ -511,7 +495,6 @@ static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
>
>       s->cancel = migrate_fd_cancel;
>       s->get_status = migrate_fd_get_status;
> -    s->release = migrate_fd_release;
>       s->blk = blk;
>       s->shared = inc;
>       s->mon = NULL;
> diff --git a/migration.h b/migration.h
> index 3165140..1cdb539 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -40,7 +40,6 @@ struct MigrationState
>       int (*write)(MigrationState *s, const void *buff, size_t size);
>       void (*cancel)(MigrationState *s);
>       int (*get_status)(MigrationState *s);
> -    void (*release)(MigrationState *s);
>       void *opaque;
>       int blk;
>       int shared;

[Qemu-devel] [PATCH 26/36] migration: Remove get_status() accessor

[Juan Quintela]

It is only used inside migration.c, and fields on that struct are
accessed all around the place on that file.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |   16 +++++-----------
 migration.h |    1 -
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/migration.c b/migration.c
index 689464d..0b7ffa7 100644
--- a/migration.c
+++ b/migration.c
@@ -91,7 +91,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
     int ret;

     if (current_migration &&
-        current_migration->get_status(current_migration) == MIG_STATE_ACTIVE) {
+        current_migration->state == MIG_STATE_ACTIVE) {
         monitor_printf(mon, "migration already in progress\n");
         return -1;
     }
@@ -136,7 +136,7 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     MigrationState *s = current_migration;

-    if (s && s->get_status(s) == MIG_STATE_ACTIVE) {
+    if (s && s->state == MIG_STATE_ACTIVE) {
         s->cancel(s);
     }
     return 0;
@@ -235,7 +235,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
     if (current_migration) {
         MigrationState *s = current_migration;

-        switch (s->get_status(current_migration)) {
+        switch (s->state) {
         case MIG_STATE_SETUP:
             /* no migration has happened ever */
             break;
@@ -386,7 +386,7 @@ static void migrate_fd_put_ready(void *opaque)
         } else {
             migrate_fd_completed(s);
         }
-        if (s->get_status(s) != MIG_STATE_COMPLETED) {
+        if (s->state != MIG_STATE_COMPLETED) {
             if (old_vm_running) {
                 vm_start();
             }
@@ -394,11 +394,6 @@ static void migrate_fd_put_ready(void *opaque)
     }
 }

-static int migrate_fd_get_status(MigrationState *s)
-{
-    return s->state;
-}
-
 static void migrate_fd_cancel(MigrationState *s)
 {
     if (s->state != MIG_STATE_ACTIVE)
@@ -460,7 +455,7 @@ void remove_migration_state_change_notifier(Notifier *notify)
 int get_migration_state(void)
 {
     if (current_migration) {
-        return migrate_fd_get_status(current_migration);
+        return current_migration->state;
     } else {
         return MIG_STATE_ERROR;
     }
@@ -494,7 +489,6 @@ static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
     MigrationState *s = g_malloc0(sizeof(*s));

     s->cancel = migrate_fd_cancel;
-    s->get_status = migrate_fd_get_status;
     s->blk = blk;
     s->shared = inc;
     s->mon = NULL;
diff --git a/migration.h b/migration.h
index 1cdb539..b634994 100644
--- a/migration.h
+++ b/migration.h
@@ -39,7 +39,6 @@ struct MigrationState
     int (*close)(MigrationState *s);
     int (*write)(MigrationState *s, const void *buff, size_t size);
     void (*cancel)(MigrationState *s);
-    int (*get_status)(MigrationState *s);
     void *opaque;
     int blk;
     int shared;
-- 
1.7.6.4

[Qemu-devel] [PATCH 27/36] migration: Remove migration cancel() callback

[Juan Quintela]

It is used only in one place

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |    9 ++++-----
 migration.h |    1 -
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/migration.c b/migration.c
index 0b7ffa7..33e89a2 100644
--- a/migration.c
+++ b/migration.c
@@ -132,12 +132,12 @@ free_migrate_state:
     return -1;
 }

+static void migrate_fd_cancel(MigrationState *s);
+
 int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
-    MigrationState *s = current_migration;
-
-    if (s && s->state == MIG_STATE_ACTIVE) {
-        s->cancel(s);
+    if (current_migration) {
+        migrate_fd_cancel(current_migration);
     }
     return 0;
 }
@@ -488,7 +488,6 @@ static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
 {
     MigrationState *s = g_malloc0(sizeof(*s));

-    s->cancel = migrate_fd_cancel;
     s->blk = blk;
     s->shared = inc;
     s->mon = NULL;
diff --git a/migration.h b/migration.h
index b634994..f059183 100644
--- a/migration.h
+++ b/migration.h
@@ -38,7 +38,6 @@ struct MigrationState
     int (*get_error)(MigrationState *s);
     int (*close)(MigrationState *s);
     int (*write)(MigrationState *s, const void *buff, size_t size);
-    void (*cancel)(MigrationState *s);
     void *opaque;
     int blk;
     int shared;
-- 
1.7.6.4

[Qemu-devel] [PATCH 28/36] migration: Move exported functions to the end of the file

[Juan Quintela]

This means we can remove the two forward declarations.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
---
 migration.c |  187 +++++++++++++++++++++++++++++------------------------------
 1 files changed, 91 insertions(+), 96 deletions(-)

diff --git a/migration.c b/migration.c
index 33e89a2..73d66d9 100644
--- a/migration.c
+++ b/migration.c
@@ -77,90 +77,6 @@ void process_incoming_migration(QEMUFile *f)
     }
 }

-static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
-                                   int detach, int blk, int inc);
-
-int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
-{
-    MigrationState *s = NULL;
-    const char *p;
-    int detach = qdict_get_try_bool(qdict, "detach", 0);
-    int blk = qdict_get_try_bool(qdict, "blk", 0);
-    int inc = qdict_get_try_bool(qdict, "inc", 0);
-    const char *uri = qdict_get_str(qdict, "uri");
-    int ret;
-
-    if (current_migration &&
-        current_migration->state == MIG_STATE_ACTIVE) {
-        monitor_printf(mon, "migration already in progress\n");
-        return -1;
-    }
-
-    if (qemu_savevm_state_blocked(mon)) {
-        return -1;
-    }
-
-    s = migrate_new(mon, max_throttle, detach, blk, inc);
-
-    if (strstart(uri, "tcp:", &p)) {
-        ret = tcp_start_outgoing_migration(s, p);
-#if !defined(WIN32)
-    } else if (strstart(uri, "exec:", &p)) {
-        ret = exec_start_outgoing_migration(s, p);
-    } else if (strstart(uri, "unix:", &p)) {
-        ret = unix_start_outgoing_migration(s, p);
-    } else if (strstart(uri, "fd:", &p)) {
-        ret = fd_start_outgoing_migration(s, p);
-#endif
-    } else {
-        monitor_printf(mon, "unknown migration protocol: %s\n", uri);
-        ret  = -EINVAL;
-        goto free_migrate_state;
-    }
-
-    if (ret < 0) {
-        monitor_printf(mon, "migration failed\n");
-        goto free_migrate_state;
-    }
-
-    g_free(current_migration);
-    current_migration = s;
-    notifier_list_notify(&migration_state_notifiers, NULL);
-    return 0;
-free_migrate_state:
-    g_free(s);
-    return -1;
-}
-
-static void migrate_fd_cancel(MigrationState *s);
-
-int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
-{
-    if (current_migration) {
-        migrate_fd_cancel(current_migration);
-    }
-    return 0;
-}
-
-int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
-{
-    int64_t d;
-    MigrationState *s;
-
-    d = qdict_get_int(qdict, "value");
-    if (d < 0) {
-        d = 0;
-    }
-    max_throttle = d;
-
-    s = current_migration;
-    if (s && s->file) {
-        qemu_file_set_rate_limit(s->file, max_throttle);
-    }
-
-    return 0;
-}
-
 /* amount of nanoseconds we are willing to wait for migration to be down.
  * the choice of nanoseconds is because it is the maximum resolution that
  * get_clock() can achieve. It is an internal measure. All user-visible
@@ -172,18 +88,6 @@ uint64_t migrate_max_downtime(void)
     return max_downtime;
 }

-int do_migrate_set_downtime(Monitor *mon, const QDict *qdict,
-                            QObject **ret_data)
-{
-    double d;
-
-    d = qdict_get_double(qdict, "value") * 1e9;
-    d = MAX(0, MIN(UINT64_MAX, d));
-    max_downtime = (uint64_t)d;
-
-    return 0;
-}
-
 static void migrate_print_status(Monitor *mon, const char *name,
                                  const QDict *status_dict)
 {
@@ -500,3 +404,94 @@ static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,

     return s;
 }
+
+int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    MigrationState *s = NULL;
+    const char *p;
+    int detach = qdict_get_try_bool(qdict, "detach", 0);
+    int blk = qdict_get_try_bool(qdict, "blk", 0);
+    int inc = qdict_get_try_bool(qdict, "inc", 0);
+    const char *uri = qdict_get_str(qdict, "uri");
+    int ret;
+
+    if (current_migration &&
+        current_migration->state == MIG_STATE_ACTIVE) {
+        monitor_printf(mon, "migration already in progress\n");
+        return -1;
+    }
+
+    if (qemu_savevm_state_blocked(mon)) {
+        return -1;
+    }
+
+    s = migrate_new(mon, max_throttle, detach, blk, inc);
+
+    if (strstart(uri, "tcp:", &p)) {
+        ret = tcp_start_outgoing_migration(s, p);
+#if !defined(WIN32)
+    } else if (strstart(uri, "exec:", &p)) {
+        ret = exec_start_outgoing_migration(s, p);
+    } else if (strstart(uri, "unix:", &p)) {
+        ret = unix_start_outgoing_migration(s, p);
+    } else if (strstart(uri, "fd:", &p)) {
+        ret = fd_start_outgoing_migration(s, p);
+#endif
+    } else {
+        monitor_printf(mon, "unknown migration protocol: %s\n", uri);
+        ret  = -EINVAL;
+        goto free_migrate_state;
+    }
+
+    if (ret < 0) {
+        monitor_printf(mon, "migration failed\n");
+        goto free_migrate_state;
+    }
+
+    g_free(current_migration);
+    current_migration = s;
+    notifier_list_notify(&migration_state_notifiers, NULL);
+    return 0;
+free_migrate_state:
+    g_free(s);
+    return -1;
+}
+
+int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    if (current_migration) {
+        migrate_fd_cancel(current_migration);
+    }
+    return 0;
+}
+
+int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    int64_t d;
+    MigrationState *s;
+
+    d = qdict_get_int(qdict, "value");
+    if (d < 0) {
+        d = 0;
+    }
+    max_throttle = d;
+
+    s = current_migration;
+    if (s && s->file) {
+        qemu_file_set_rate_limit(s->file, max_throttle);
+    }
+
+    return 0;
+}
+
+int do_migrate_set_downtime(Monitor *mon, const QDict *qdict,
+                            QObject **ret_data)
+{
+    double d;
+
+    d = qdict_get_double(qdict, "value") * 1e9;
+    d = MAX(0, MIN(UINT64_MAX, d));
+    max_downtime = (uint64_t)d;
+
+    return 0;
+}
-- 
1.7.6.4

[Qemu-devel] [PATCH 29/36] migration: create accessor for current_migration

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |  116 ++++++++++++++++++++++++++++-------------------------------
 1 files changed, 55 insertions(+), 61 deletions(-)

diff --git a/migration.c b/migration.c
index 73d66d9..8422ec3 100644
--- a/migration.c
+++ b/migration.c
@@ -34,11 +34,22 @@
 /* Migration speed throttling */
 static int64_t max_throttle = (32 << 20);

-static MigrationState *current_migration;
-
 static NotifierList migration_state_notifiers =
     NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);

+/* When we add fault tolerance, we could have several
+   migrations at once.  For now we don't need to add
+   dynamic creation of migration */
+
+static MigrationState *migrate_get_current(void)
+{
+    static MigrationState current_migration = {
+        .state = MIG_STATE_SETUP,
+    };
+
+    return &current_migration;
+}
+
 int qemu_start_incoming_migration(const char *uri)
 {
     const char *p;
@@ -135,39 +146,36 @@ static void migrate_put_status(QDict *qdict, const char *name,
 void do_info_migrate(Monitor *mon, QObject **ret_data)
 {
     QDict *qdict;
-
-    if (current_migration) {
-        MigrationState *s = current_migration;
-
-        switch (s->state) {
-        case MIG_STATE_SETUP:
-            /* no migration has happened ever */
-            break;
-        case MIG_STATE_ACTIVE:
-            qdict = qdict_new();
-            qdict_put(qdict, "status", qstring_from_str("active"));
-
-            migrate_put_status(qdict, "ram", ram_bytes_transferred(),
-                               ram_bytes_remaining(), ram_bytes_total());
-
-            if (blk_mig_active()) {
-                migrate_put_status(qdict, "disk", blk_mig_bytes_transferred(),
-                                   blk_mig_bytes_remaining(),
-                                   blk_mig_bytes_total());
-            }
-
-            *ret_data = QOBJECT(qdict);
-            break;
-        case MIG_STATE_COMPLETED:
-            *ret_data = qobject_from_jsonf("{ 'status': 'completed' }");
-            break;
-        case MIG_STATE_ERROR:
-            *ret_data = qobject_from_jsonf("{ 'status': 'failed' }");
-            break;
-        case MIG_STATE_CANCELLED:
-            *ret_data = qobject_from_jsonf("{ 'status': 'cancelled' }");
-            break;
+    MigrationState *s = migrate_get_current();
+
+    switch (s->state) {
+    case MIG_STATE_SETUP:
+        /* no migration has happened ever */
+        break;
+    case MIG_STATE_ACTIVE:
+        qdict = qdict_new();
+        qdict_put(qdict, "status", qstring_from_str("active"));
+
+        migrate_put_status(qdict, "ram", ram_bytes_transferred(),
+                           ram_bytes_remaining(), ram_bytes_total());
+
+        if (blk_mig_active()) {
+            migrate_put_status(qdict, "disk", blk_mig_bytes_transferred(),
+                               blk_mig_bytes_remaining(),
+                               blk_mig_bytes_total());
         }
+
+        *ret_data = QOBJECT(qdict);
+        break;
+    case MIG_STATE_COMPLETED:
+        *ret_data = qobject_from_jsonf("{ 'status': 'completed' }");
+        break;
+    case MIG_STATE_ERROR:
+        *ret_data = qobject_from_jsonf("{ 'status': 'failed' }");
+        break;
+    case MIG_STATE_CANCELLED:
+        *ret_data = qobject_from_jsonf("{ 'status': 'cancelled' }");
+        break;
     }
 }

@@ -358,11 +366,7 @@ void remove_migration_state_change_notifier(Notifier *notify)

 int get_migration_state(void)
 {
-    if (current_migration) {
-        return current_migration->state;
-    } else {
-        return MIG_STATE_ERROR;
-    }
+    return migrate_get_current()->state;
 }

 void migrate_fd_connect(MigrationState *s)
@@ -387,11 +391,12 @@ void migrate_fd_connect(MigrationState *s)
     migrate_fd_put_ready(s);
 }

-static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
-                                   int detach, int blk, int inc)
+static MigrationState *migrate_init(Monitor *mon, int64_t bandwidth_limit,
+                                    int detach, int blk, int inc)
 {
-    MigrationState *s = g_malloc0(sizeof(*s));
+    MigrationState *s = migrate_get_current();

+    memset(s, 0, sizeof(*s));
     s->blk = blk;
     s->shared = inc;
     s->mon = NULL;
@@ -407,7 +412,7 @@ static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,

 int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
-    MigrationState *s = NULL;
+    MigrationState *s = migrate_get_current();
     const char *p;
     int detach = qdict_get_try_bool(qdict, "detach", 0);
     int blk = qdict_get_try_bool(qdict, "blk", 0);
@@ -415,8 +420,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
     const char *uri = qdict_get_str(qdict, "uri");
     int ret;

-    if (current_migration &&
-        current_migration->state == MIG_STATE_ACTIVE) {
+    if (s->state == MIG_STATE_ACTIVE) {
         monitor_printf(mon, "migration already in progress\n");
         return -1;
     }
@@ -425,7 +429,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
         return -1;
     }

-    s = migrate_new(mon, max_throttle, detach, blk, inc);
+    s = migrate_init(mon, max_throttle, detach, blk, inc);

     if (strstart(uri, "tcp:", &p)) {
         ret = tcp_start_outgoing_migration(s, p);
@@ -440,28 +444,20 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
     } else {
         monitor_printf(mon, "unknown migration protocol: %s\n", uri);
         ret  = -EINVAL;
-        goto free_migrate_state;
     }

     if (ret < 0) {
-        monitor_printf(mon, "migration failed\n");
-        goto free_migrate_state;
+        monitor_printf(mon, "migration failed: %s\n", strerror(-ret));
+        return ret;
     }

-    g_free(current_migration);
-    current_migration = s;
     notifier_list_notify(&migration_state_notifiers, NULL);
     return 0;
-free_migrate_state:
-    g_free(s);
-    return -1;
 }

 int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
-    if (current_migration) {
-        migrate_fd_cancel(current_migration);
-    }
+    migrate_fd_cancel(migrate_get_current());
     return 0;
 }

@@ -476,10 +472,8 @@ int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
     }
     max_throttle = d;

-    s = current_migration;
-    if (s && s->file) {
-        qemu_file_set_rate_limit(s->file, max_throttle);
-    }
+    s = migrate_get_current();
+    qemu_file_set_rate_limit(s->file, max_throttle);

     return 0;
 }
-- 
1.7.6.4

[Qemu-devel] [PATCH 30/36] migration: Use bandwidth_limit directly

[Juan Quintela]

Now that current_migration always exist, there is no reason for
max_throotle variable.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/migration.c b/migration.c
index 8422ec3..4f308ce 100644
--- a/migration.c
+++ b/migration.c
@@ -31,8 +31,7 @@
     do { } while (0)
 #endif

-/* Migration speed throttling */
-static int64_t max_throttle = (32 << 20);
+#define MAX_THROTTLE  (32 << 20)      /* Migration speed throttling */

 static NotifierList migration_state_notifiers =
     NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
@@ -45,6 +44,7 @@ static MigrationState *migrate_get_current(void)
 {
     static MigrationState current_migration = {
         .state = MIG_STATE_SETUP,
+        .bandwidth_limit = MAX_THROTTLE,
     };

     return &current_migration;
@@ -391,12 +391,13 @@ void migrate_fd_connect(MigrationState *s)
     migrate_fd_put_ready(s);
 }

-static MigrationState *migrate_init(Monitor *mon, int64_t bandwidth_limit,
-                                    int detach, int blk, int inc)
+static MigrationState *migrate_init(Monitor *mon, int detach, int blk, int inc)
 {
     MigrationState *s = migrate_get_current();
+    int64_t bandwidth_limit = s->bandwidth_limit;

     memset(s, 0, sizeof(*s));
+    s->bandwidth_limit = bandwidth_limit;
     s->blk = blk;
     s->shared = inc;
     s->mon = NULL;
@@ -429,7 +430,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
         return -1;
     }

-    s = migrate_init(mon, max_throttle, detach, blk, inc);
+    s = migrate_init(mon, detach, blk, inc);

     if (strstart(uri, "tcp:", &p)) {
         ret = tcp_start_outgoing_migration(s, p);
@@ -470,10 +471,10 @@ int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
     if (d < 0) {
         d = 0;
     }
-    max_throttle = d;

     s = migrate_get_current();
-    qemu_file_set_rate_limit(s->file, max_throttle);
+    s->bandwidth_limit = d;
+    qemu_file_set_rate_limit(s->file, s->bandwidth_limit);

     return 0;
 }
-- 
1.7.6.4

[Qemu-devel] [PATCH 31/36] migration: Pass MigrationState in migration notifiers

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/migration.c b/migration.c
index 4f308ce..6129cb5 100644
--- a/migration.c
+++ b/migration.c
@@ -222,7 +222,7 @@ void migrate_fd_error(MigrationState *s)
 {
     DPRINTF("setting error state\n");
     s->state = MIG_STATE_ERROR;
-    notifier_list_notify(&migration_state_notifiers, NULL);
+    notifier_list_notify(&migration_state_notifiers, s);
     migrate_fd_cleanup(s);
 }

@@ -235,7 +235,7 @@ static void migrate_fd_completed(MigrationState *s)
         s->state = MIG_STATE_COMPLETED;
         runstate_set(RUN_STATE_POSTMIGRATE);
     }
-    notifier_list_notify(&migration_state_notifiers, NULL);
+    notifier_list_notify(&migration_state_notifiers, s);
 }

 static void migrate_fd_put_notify(void *opaque)
@@ -314,7 +314,7 @@ static void migrate_fd_cancel(MigrationState *s)
     DPRINTF("cancelling migration\n");

     s->state = MIG_STATE_CANCELLED;
-    notifier_list_notify(&migration_state_notifiers, NULL);
+    notifier_list_notify(&migration_state_notifiers, s);
     qemu_savevm_state_cancel(s->mon, s->file);

     migrate_fd_cleanup(s);
@@ -452,7 +452,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
         return ret;
     }

-    notifier_list_notify(&migration_state_notifiers, NULL);
+    notifier_list_notify(&migration_state_notifiers, s);
     return 0;
 }

-- 
1.7.6.4

[Qemu-devel] [PATCH 32/36] migration: Export a function that tells if the migration has finished correctly

[Juan Quintela]

This will allow us to hide the state values.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c     |    4 ++--
 migration.h     |    2 +-
 ui/spice-core.c |    4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/migration.c b/migration.c
index 6129cb5..c4e2ba6 100644
--- a/migration.c
+++ b/migration.c
@@ -364,9 +364,9 @@ void remove_migration_state_change_notifier(Notifier *notify)
     notifier_list_remove(&migration_state_notifiers, notify);
 }

-int get_migration_state(void)
+bool migration_has_finished(MigrationState *s)
 {
-    return migrate_get_current()->state;
+    return s->state == MIG_STATE_COMPLETED;
 }

 void migrate_fd_connect(MigrationState *s)
diff --git a/migration.h b/migration.h
index f059183..7ae127f 100644
--- a/migration.h
+++ b/migration.h
@@ -84,7 +84,7 @@ void migrate_fd_connect(MigrationState *s);

 void add_migration_state_change_notifier(Notifier *notify);
 void remove_migration_state_change_notifier(Notifier *notify);
-int get_migration_state(void);
+bool migration_has_finished(MigrationState *);

 uint64_t ram_bytes_remaining(void);
 uint64_t ram_bytes_transferred(void);
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 3cbc721..b33366e 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -447,9 +447,9 @@ void do_info_spice(Monitor *mon, QObject **ret_data)

 static void migration_state_notifier(Notifier *notifier, void *data)
 {
-    int state = get_migration_state();
+    MigrationState *s = data;

-    if (state == MIG_STATE_COMPLETED) {
+    if (migration_has_finished(s)) {
 #if SPICE_SERVER_VERSION >= 0x000701 /* 0.7.1 */
         spice_server_migrate_switch(spice_server);
 #endif
-- 
1.7.6.4

[Qemu-devel] [PATCH 33/36] migration: Make state definitions local

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |    8 ++++++++
 migration.h |    8 --------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/migration.c b/migration.c
index c4e2ba6..e9237f3 100644
--- a/migration.c
+++ b/migration.c
@@ -31,6 +31,14 @@
     do { } while (0)
 #endif

+enum migration_state {
+    MIG_STATE_ERROR,
+    MIG_STATE_SETUP,
+    MIG_STATE_CANCELLED,
+    MIG_STATE_ACTIVE,
+    MIG_STATE_COMPLETED,
+};
+
 #define MAX_THROTTLE  (32 << 20)      /* Migration speed throttling */

 static NotifierList migration_state_notifiers =
diff --git a/migration.h b/migration.h
index 7ae127f..a1f80d0 100644
--- a/migration.h
+++ b/migration.h
@@ -18,14 +18,6 @@
 #include "qemu-common.h"
 #include "notify.h"

-enum migration_state {
-    MIG_STATE_ERROR,
-    MIG_STATE_SETUP,
-    MIG_STATE_CANCELLED,
-    MIG_STATE_ACTIVE,
-    MIG_STATE_COMPLETED,
-};
-
 typedef struct MigrationState MigrationState;

 struct MigrationState
-- 
1.7.6.4

[Qemu-devel] [PATCH 34/36] migration: Don't use callback on file defining it

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration-tcp.c  |    4 ++--
 migration-unix.c |    6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/migration-tcp.c b/migration-tcp.c
index f6b2288..bd3aa3a 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -58,7 +58,7 @@ static void tcp_wait_for_connect(void *opaque)
     DPRINTF("connect completed\n");
     do {
         ret = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize);
-    } while (ret == -1 && (s->get_error(s)) == EINTR);
+    } while (ret == -1 && (socket_error()) == EINTR);

     if (ret < 0) {
         migrate_fd_error(s);
@@ -98,7 +98,7 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
     do {
         ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
         if (ret == -1)
-            ret = -(s->get_error(s));
+            ret = -(socket_error());

         if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
             qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
diff --git a/migration-unix.c b/migration-unix.c
index bd8d40f..ca66851 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -57,7 +57,7 @@ static void unix_wait_for_connect(void *opaque)
     DPRINTF("connect completed\n");
     do {
         ret = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize);
-    } while (ret == -1 && (s->get_error(s)) == EINTR);
+    } while (ret == -1 && errno == EINTR);

     if (ret < 0) {
         migrate_fd_error(s);
@@ -96,7 +96,7 @@ int unix_start_outgoing_migration(MigrationState *s, const char *path)
     do {
         ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
         if (ret == -1)
-	    ret = -(s->get_error(s));
+            ret = -errno;

         if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
 	    qemu_set_fd_handler2(s->fd, NULL, NULL, unix_wait_for_connect, s);
@@ -129,7 +129,7 @@ static void unix_accept_incoming_migration(void *opaque)

     do {
         c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
-    } while (c == -1 && socket_error() == EINTR);
+    } while (c == -1 && errno == EINTR);

     DPRINTF("accepted migration\n");

-- 
1.7.6.4

[Qemu-devel] [PATCH 35/36] migration: propagate error correctly

[Juan Quintela]

unix and tcp outgoing migration have error values, but didn't returned
it.  Make them return the error.  Notice that EINPROGRESS & EWOULDBLOCK
are not considered errors as call will be retry later.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
---
 migration-tcp.c  |   20 +++++++++++---------
 migration-unix.c |   26 ++++++++++----------------
 2 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/migration-tcp.c b/migration-tcp.c
index bd3aa3a..619df21 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -90,26 +90,28 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)

     s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
     if (s->fd == -1) {
-        return -1;
+        return -socket_error();
     }

     socket_set_nonblock(s->fd);

     do {
         ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
-        if (ret == -1)
-            ret = -(socket_error());
-
-        if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
+        if (ret == -1) {
+            ret = -socket_error();
+        }
+        if (ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
             qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
+            return 0;
+        }
     } while (ret == -EINTR);

-    if (ret < 0 && ret != -EINPROGRESS && ret != -EWOULDBLOCK) {
+    if (ret < 0) {
         DPRINTF("connect failed\n");
         migrate_fd_error(s);
-    } else if (ret >= 0)
-        migrate_fd_connect(s);
-
+        return ret;
+    }
+    migrate_fd_connect(s);
     return 0;
 }

diff --git a/migration-unix.c b/migration-unix.c
index ca66851..f979b5f 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -88,35 +88,29 @@ int unix_start_outgoing_migration(MigrationState *s, const char *path)
     s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
     if (s->fd < 0) {
         DPRINTF("Unable to open socket");
-        goto err_after_socket;
+        return -errno;
     }

     socket_set_nonblock(s->fd);

     do {
         ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
-        if (ret == -1)
+        if (ret == -1) {
             ret = -errno;
-
-        if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
+        }
+        if (ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
 	    qemu_set_fd_handler2(s->fd, NULL, NULL, unix_wait_for_connect, s);
+            return 0;
+        }
     } while (ret == -EINTR);

-    if (ret < 0 && ret != -EINPROGRESS && ret != -EWOULDBLOCK) {
+    if (ret < 0) {
         DPRINTF("connect failed\n");
-        goto err_after_open;
+        migrate_fd_error(s);
+        return ret;
     }
-
-    if (ret >= 0)
-        migrate_fd_connect(s);
-
+    migrate_fd_connect(s);
     return 0;
-
-err_after_open:
-    close(s->fd);
-
-err_after_socket:
-    return -1;
 }

 static void unix_accept_incoming_migration(void *opaque)
-- 
1.7.6.4

[Qemu-devel] [PATCH 36/36] migration: make migration-{tcp, unix} consistent

[Juan Quintela]

Files are almost identical in functionality, just remove the
differences that make no sense.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
---
 migration-tcp.c  |   15 ++++++++++-----
 migration-unix.c |   46 +++++++++++++++++++++++++---------------------
 2 files changed, 35 insertions(+), 26 deletions(-)

diff --git a/migration-tcp.c b/migration-tcp.c
index 619df21..5aa742c 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -48,7 +48,6 @@ static int tcp_close(MigrationState *s)
     return 0;
 }

-
 static void tcp_wait_for_connect(void *opaque)
 {
     MigrationState *s = opaque;
@@ -84,12 +83,14 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
     if (ret < 0) {
         return ret;
     }
+
     s->get_error = socket_errno;
     s->write = socket_write;
     s->close = tcp_close;

     s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
     if (s->fd == -1) {
+        DPRINTF("Unable to open socket");
         return -socket_error();
     }

@@ -155,23 +156,27 @@ int tcp_start_incoming_migration(const char *host_port)
     int val;
     int s;

+    DPRINTF("Attempting to start an incoming migration\n");
+
     if (parse_host_port(&addr, host_port) < 0) {
         fprintf(stderr, "invalid host/port combination: %s\n", host_port);
         return -EINVAL;
     }

     s = qemu_socket(PF_INET, SOCK_STREAM, 0);
-    if (s == -1)
+    if (s == -1) {
         return -socket_error();
+    }

     val = 1;
     setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));

-    if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1)
+    if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
         goto err;
-
-    if (listen(s, 1) == -1)
+    }
+    if (listen(s, 1) == -1) {
         goto err;
+    }

     qemu_set_fd_handler2(s, NULL, tcp_accept_incoming_migration, NULL,
                          (void *)(intptr_t)s);
diff --git a/migration-unix.c b/migration-unix.c
index f979b5f..8596353 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -86,7 +86,7 @@ int unix_start_outgoing_migration(MigrationState *s, const char *path)
     s->close = unix_close;

     s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
-    if (s->fd < 0) {
+    if (s->fd == -1) {
         DPRINTF("Unable to open socket");
         return -errno;
     }
@@ -129,7 +129,7 @@ static void unix_accept_incoming_migration(void *opaque)

     if (c == -1) {
         fprintf(stderr, "could not accept migration connection\n");
-        return;
+        goto out2;
     }

     f = qemu_fopen_socket(c);
@@ -141,45 +141,49 @@ static void unix_accept_incoming_migration(void *opaque)
     process_incoming_migration(f);
     qemu_fclose(f);
 out:
+    close(c);
+out2:
     qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
     close(s);
-    close(c);
 }

 int unix_start_incoming_migration(const char *path)
 {
-    struct sockaddr_un un;
-    int sock;
+    struct sockaddr_un addr;
+    int s;
+    int ret;

     DPRINTF("Attempting to start an incoming migration\n");

-    sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
-    if (sock < 0) {
+    s = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
+    if (s == -1) {
         fprintf(stderr, "Could not open unix socket: %s\n", strerror(errno));
-        return -EINVAL;
+        return -errno;
     }

-    memset(&un, 0, sizeof(un));
-    un.sun_family = AF_UNIX;
-    snprintf(un.sun_path, sizeof(un.sun_path), "%s", path);
+    memset(&addr, 0, sizeof(addr));
+    addr.sun_family = AF_UNIX;
+    snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path);

-    unlink(un.sun_path);
-    if (bind(sock, (struct sockaddr*) &un, sizeof(un)) < 0) {
-        fprintf(stderr, "bind(unix:%s): %s\n", un.sun_path, strerror(errno));
+    unlink(addr.sun_path);
+    if (bind(s, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
+        ret = -errno;
+        fprintf(stderr, "bind(unix:%s): %s\n", addr.sun_path, strerror(errno));
         goto err;
     }
-    if (listen(sock, 1) < 0) {
-        fprintf(stderr, "listen(unix:%s): %s\n", un.sun_path, strerror(errno));
+    if (listen(s, 1) == -1) {
+        fprintf(stderr, "listen(unix:%s): %s\n", addr.sun_path,
+                strerror(errno));
+        ret = -errno;
         goto err;
     }

-    qemu_set_fd_handler2(sock, NULL, unix_accept_incoming_migration, NULL,
-			 (void *)(intptr_t)sock);
+    qemu_set_fd_handler2(s, NULL, unix_accept_incoming_migration, NULL,
+                         (void *)(intptr_t)s);

     return 0;

 err:
-    close(sock);
-
-    return -EINVAL;
+    close(s);
+    return ret;
 }
-- 
1.7.6.4