qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/5] Migration cleanups
@ 2015-08-13 10:51 Dr. David Alan Gilbert (git)
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 1/5] migration/ram.c: Use RAMBlock rather than MemoryRegion Dr. David Alan Gilbert (git)
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2015-08-13 10:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: amit.shah, quintela

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

This series is a set of small cleanups, some of which are from
my postcopy series.

Dave

Dr. David Alan Gilbert (5):
  migration/ram.c: Use RAMBlock rather than MemoryRegion
  Split out end of migration code from migration_thread
  Init page sizes in qtest
  migration: size_t'ify some of qemu-file
  migration: qemu-file more size_t'ifying

 include/migration/qemu-file.h | 18 +++++------
 migration/migration.c         | 75 +++++++++++++++++++++++++++----------------
 migration/qemu-file-buf.c     |  7 ++--
 migration/qemu-file-stdio.c   | 11 ++++---
 migration/qemu-file-unix.c    |  6 ++--
 migration/qemu-file.c         | 22 ++++++-------
 migration/ram.c               | 26 +++++++--------
 migration/rdma.c              | 13 ++++----
 migration/savevm.c            |  7 ++--
 qtest.c                       |  1 +
 trace-events                  |  4 ++-
 11 files changed, 107 insertions(+), 83 deletions(-)

-- 
2.4.3

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 1/5] migration/ram.c: Use RAMBlock rather than MemoryRegion
  2015-08-13 10:51 [Qemu-devel] [PATCH 0/5] Migration cleanups Dr. David Alan Gilbert (git)
@ 2015-08-13 10:51 ` Dr. David Alan Gilbert (git)
  2015-08-13 12:46   ` Paolo Bonzini
  2015-09-15 10:43   ` Amit Shah
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 2/5] Split out end of migration code from migration_thread Dr. David Alan Gilbert (git)
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2015-08-13 10:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: amit.shah, quintela

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

RAM migration mainly works on RAMBlocks but in a few places
uses data from MemoryRegions to access the same information that's
already held in RAMBlocks; clean it up just to avoid the
MemoryRegion use.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/ram.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 7f007e6..7df9157 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -497,13 +497,13 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t **current_data,
 
 /* Called with rcu_read_lock() to protect migration_bitmap */
 static inline
-ram_addr_t migration_bitmap_find_and_reset_dirty(MemoryRegion *mr,
+ram_addr_t migration_bitmap_find_and_reset_dirty(RAMBlock *rb,
                                                  ram_addr_t start)
 {
-    unsigned long base = mr->ram_addr >> TARGET_PAGE_BITS;
+    unsigned long base = rb->offset >> TARGET_PAGE_BITS;
     unsigned long nr = base + (start >> TARGET_PAGE_BITS);
-    uint64_t mr_size = TARGET_PAGE_ALIGN(memory_region_size(mr));
-    unsigned long size = base + (mr_size >> TARGET_PAGE_BITS);
+    uint64_t rb_size = rb->used_length;
+    unsigned long size = base + (rb_size >> TARGET_PAGE_BITS);
     unsigned long *bitmap;
 
     unsigned long next;
@@ -573,7 +573,7 @@ static void migration_bitmap_sync(void)
     qemu_mutex_lock(&migration_bitmap_mutex);
     rcu_read_lock();
     QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
-        migration_bitmap_sync_range(block->mr->ram_addr, block->used_length);
+        migration_bitmap_sync_range(block->offset, block->used_length);
     }
     rcu_read_unlock();
     qemu_mutex_unlock(&migration_bitmap_mutex);
@@ -668,12 +668,11 @@ static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset,
     int pages = -1;
     uint64_t bytes_xmit;
     ram_addr_t current_addr;
-    MemoryRegion *mr = block->mr;
     uint8_t *p;
     int ret;
     bool send_async = true;
 
-    p = memory_region_get_ram_ptr(mr) + offset;
+    p = block->host + offset;
 
     /* In doubt sent page as normal */
     bytes_xmit = 0;
@@ -744,7 +743,7 @@ static int do_compress_ram_page(CompressParam *param)
     RAMBlock *block = param->block;
     ram_addr_t offset = param->offset;
 
-    p = memory_region_get_ram_ptr(block->mr) + (offset & TARGET_PAGE_MASK);
+    p = block->host + (offset & TARGET_PAGE_MASK);
 
     bytes_sent = save_page_header(param->file, block, offset |
                                   RAM_SAVE_FLAG_COMPRESS_PAGE);
@@ -852,11 +851,10 @@ static int ram_save_compressed_page(QEMUFile *f, RAMBlock *block,
 {
     int pages = -1;
     uint64_t bytes_xmit;
-    MemoryRegion *mr = block->mr;
     uint8_t *p;
     int ret;
 
-    p = memory_region_get_ram_ptr(mr) + offset;
+    p = block->host + offset;
 
     bytes_xmit = 0;
     ret = ram_control_save_page(f, block->offset,
@@ -929,14 +927,12 @@ static int ram_find_and_save_block(QEMUFile *f, bool last_stage,
     ram_addr_t offset = last_offset;
     bool complete_round = false;
     int pages = 0;
-    MemoryRegion *mr;
 
     if (!block)
         block = QLIST_FIRST_RCU(&ram_list.blocks);
 
     while (true) {
-        mr = block->mr;
-        offset = migration_bitmap_find_and_reset_dirty(mr, offset);
+        offset = migration_bitmap_find_and_reset_dirty(block, offset);
         if (complete_round && block == last_seen_block &&
             offset >= last_offset) {
             break;
@@ -1344,7 +1340,7 @@ static inline void *host_from_stream_offset(QEMUFile *f,
             return NULL;
         }
 
-        return memory_region_get_ram_ptr(block->mr) + offset;
+        return block->host + offset;
     }
 
     len = qemu_get_byte(f);
@@ -1354,7 +1350,7 @@ static inline void *host_from_stream_offset(QEMUFile *f,
     QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
         if (!strncmp(id, block->idstr, sizeof(id)) &&
             block->max_length > offset) {
-            return memory_region_get_ram_ptr(block->mr) + offset;
+            return block->host + offset;
         }
     }
 
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 2/5] Split out end of migration code from migration_thread
  2015-08-13 10:51 [Qemu-devel] [PATCH 0/5] Migration cleanups Dr. David Alan Gilbert (git)
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 1/5] migration/ram.c: Use RAMBlock rather than MemoryRegion Dr. David Alan Gilbert (git)
@ 2015-08-13 10:51 ` Dr. David Alan Gilbert (git)
  2015-08-13 12:14   ` zhanghailiang
  2015-09-15 11:12   ` Amit Shah
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 3/5] Init page sizes in qtest Dr. David Alan Gilbert (git)
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2015-08-13 10:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: amit.shah, quintela

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The code that gets run at the end of the migration process
is getting large, and I'm about to add more for postcopy.
Split it into a separate function.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c | 75 ++++++++++++++++++++++++++++++++-------------------
 trace-events          |  2 ++
 2 files changed, 49 insertions(+), 28 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 662e77e..46bb410 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -913,6 +913,50 @@ int64_t migrate_xbzrle_cache_size(void)
     return s->xbzrle_cache_size;
 }
 
+/**
+ * migration_completion: Used by migration_thread when there's not much left.
+ *   The caller 'breaks' the loop when this returns.
+ *
+ * @s: Current migration state
+ * @*old_vm_running: Pointer to old_vm_running flag
+ * @*start_time: Pointer to time to update
+ */
+static void migration_completion(MigrationState *s, bool *old_vm_running,
+                                 int64_t *start_time)
+{
+    int ret;
+
+    qemu_mutex_lock_iothread();
+    *start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+    qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
+    *old_vm_running = runstate_is_running();
+
+    ret = global_state_store();
+    if (!ret) {
+        ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
+        if (ret >= 0) {
+            qemu_file_set_rate_limit(s->file, INT64_MAX);
+            qemu_savevm_state_complete(s->file);
+        }
+    }
+    qemu_mutex_unlock_iothread();
+
+    if (ret < 0) {
+        goto fail;
+    }
+
+    if (qemu_file_get_error(s->file)) {
+        trace_migration_completion_file_err();
+        goto fail;
+    }
+
+    migrate_set_state(s, MIGRATION_STATUS_ACTIVE, MIGRATION_STATUS_COMPLETED);
+    return;
+
+fail:
+    migrate_set_state(s, MIGRATION_STATUS_ACTIVE, MIGRATION_STATUS_FAILED);
+}
+
 /* migration thread support */
 
 static void *migration_thread(void *opaque)
@@ -943,34 +987,9 @@ static void *migration_thread(void *opaque)
             if (pending_size && pending_size >= max_size) {
                 qemu_savevm_state_iterate(s->file);
             } else {
-                int ret;
-
-                qemu_mutex_lock_iothread();
-                start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
-                qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
-                old_vm_running = runstate_is_running();
-
-                ret = global_state_store();
-                if (!ret) {
-                    ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
-                    if (ret >= 0) {
-                        qemu_file_set_rate_limit(s->file, INT64_MAX);
-                        qemu_savevm_state_complete(s->file);
-                    }
-                }
-                qemu_mutex_unlock_iothread();
-
-                if (ret < 0) {
-                    migrate_set_state(s, MIGRATION_STATUS_ACTIVE,
-                                      MIGRATION_STATUS_FAILED);
-                    break;
-                }
-
-                if (!qemu_file_get_error(s->file)) {
-                    migrate_set_state(s, MIGRATION_STATUS_ACTIVE,
-                                      MIGRATION_STATUS_COMPLETED);
-                    break;
-                }
+                trace_migration_thread_low_pending(pending_size);
+                migration_completion(s, &old_vm_running, &start_time);
+                break;
             }
         }
 
diff --git a/trace-events b/trace-events
index 94bf3bb..1509e5b 100644
--- a/trace-events
+++ b/trace-events
@@ -1406,6 +1406,8 @@ migrate_transferred(uint64_t tranferred, uint64_t time_spent, double bandwidth,
 migrate_state_too_big(void) ""
 migrate_global_state_post_load(const char *state) "loaded state: %s"
 migrate_global_state_pre_save(const char *state) "saved state: %s"
+migration_completion_file_err(void) ""
+migration_thread_low_pending(uint64_t pending) "%" PRIu64
 
 # migration/rdma.c
 qemu_rdma_accept_incoming_migration(void) ""
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 3/5] Init page sizes in qtest
  2015-08-13 10:51 [Qemu-devel] [PATCH 0/5] Migration cleanups Dr. David Alan Gilbert (git)
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 1/5] migration/ram.c: Use RAMBlock rather than MemoryRegion Dr. David Alan Gilbert (git)
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 2/5] Split out end of migration code from migration_thread Dr. David Alan Gilbert (git)
@ 2015-08-13 10:51 ` Dr. David Alan Gilbert (git)
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 4/5] migration: size_t'ify some of qemu-file Dr. David Alan Gilbert (git)
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 5/5] migration: qemu-file more size_t'ifying Dr. David Alan Gilbert (git)
  4 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2015-08-13 10:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: amit.shah, quintela

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

One of my patches used a loop that was based on host page size;
it dies in qtest since qtest hadn't bothered init'ing it.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Amit Shah <amit.shah@redhat.com>
---
 qtest.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qtest.c b/qtest.c
index 05cefd2..8e10340 100644
--- a/qtest.c
+++ b/qtest.c
@@ -657,6 +657,7 @@ void qtest_init(const char *qtest_chrdev, const char *qtest_log, Error **errp)
 
     inbuf = g_string_new("");
     qtest_chr = chr;
+    page_size_init();
 }
 
 bool qtest_driver(void)
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 4/5] migration: size_t'ify some of qemu-file
  2015-08-13 10:51 [Qemu-devel] [PATCH 0/5] Migration cleanups Dr. David Alan Gilbert (git)
                   ` (2 preceding siblings ...)
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 3/5] Init page sizes in qtest Dr. David Alan Gilbert (git)
@ 2015-08-13 10:51 ` Dr. David Alan Gilbert (git)
  2015-08-13 12:15   ` zhanghailiang
  2015-09-15 11:39   ` Amit Shah
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 5/5] migration: qemu-file more size_t'ifying Dr. David Alan Gilbert (git)
  4 siblings, 2 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2015-08-13 10:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: amit.shah, quintela

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

This is a start on using size_t more in qemu-file and friends;
it fixes up QEMUFilePutBufferFunc and QEMUFileGetBufferFunc
to take size_t lengths and return ssize_t return values (like read(2))
and fixes up all the different implementations of them.

Note that I've not yet followed this deeply into bdrv_ implementations.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/migration/qemu-file.h |  8 ++++----
 migration/qemu-file-buf.c     |  7 ++++---
 migration/qemu-file-stdio.c   | 11 ++++++-----
 migration/qemu-file-unix.c    |  6 ++++--
 migration/rdma.c              | 13 +++++++------
 migration/savevm.c            |  7 ++++---
 trace-events                  |  2 +-
 7 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index ea49f33..e1e2bab 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -31,15 +31,15 @@
  * The pos argument can be ignored if the file is only being used for
  * streaming.  The handler should try to write all of the data it can.
  */
-typedef int (QEMUFilePutBufferFunc)(void *opaque, const uint8_t *buf,
-                                    int64_t pos, int size);
+typedef ssize_t (QEMUFilePutBufferFunc)(void *opaque, const uint8_t *buf,
+                                        int64_t pos, size_t size);
 
 /* Read a chunk of data from a file at the given position.  The pos argument
  * can be ignored if the file is only be used for streaming.  The number of
  * bytes actually read should be returned.
  */
-typedef int (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
-                                    int64_t pos, int size);
+typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
+                                        int64_t pos, size_t size);
 
 /* Close a file
  *
diff --git a/migration/qemu-file-buf.c b/migration/qemu-file-buf.c
index 2de9330..1d9528e 100644
--- a/migration/qemu-file-buf.c
+++ b/migration/qemu-file-buf.c
@@ -372,7 +372,8 @@ typedef struct QEMUBuffer {
     bool qsb_allocated;
 } QEMUBuffer;
 
-static int buf_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+static ssize_t buf_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
+                              size_t size)
 {
     QEMUBuffer *s = opaque;
     ssize_t len = qsb_get_length(s->qsb) - pos;
@@ -387,8 +388,8 @@ static int buf_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
     return qsb_get_buffer(s->qsb, pos, len, buf);
 }
 
-static int buf_put_buffer(void *opaque, const uint8_t *buf,
-                          int64_t pos, int size)
+static ssize_t buf_put_buffer(void *opaque, const uint8_t *buf,
+                              int64_t pos, size_t size)
 {
     QEMUBuffer *s = opaque;
 
diff --git a/migration/qemu-file-stdio.c b/migration/qemu-file-stdio.c
index 285068b..dc91137 100644
--- a/migration/qemu-file-stdio.c
+++ b/migration/qemu-file-stdio.c
@@ -37,11 +37,11 @@ static int stdio_get_fd(void *opaque)
     return fileno(s->stdio_file);
 }
 
-static int stdio_put_buffer(void *opaque, const uint8_t *buf, int64_t pos,
-                            int size)
+static ssize_t stdio_put_buffer(void *opaque, const uint8_t *buf, int64_t pos,
+                                size_t size)
 {
     QEMUFileStdio *s = opaque;
-    int res;
+    size_t res;
 
     res = fwrite(buf, 1, size, s->stdio_file);
 
@@ -51,11 +51,12 @@ static int stdio_put_buffer(void *opaque, const uint8_t *buf, int64_t pos,
     return res;
 }
 
-static int stdio_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+static ssize_t stdio_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
+                                size_t size)
 {
     QEMUFileStdio *s = opaque;
     FILE *fp = s->stdio_file;
-    int bytes;
+    ssize_t bytes;
 
     for (;;) {
         clearerr(fp);
diff --git a/migration/qemu-file-unix.c b/migration/qemu-file-unix.c
index bfbc086..adfe91a 100644
--- a/migration/qemu-file-unix.c
+++ b/migration/qemu-file-unix.c
@@ -54,7 +54,8 @@ static int socket_get_fd(void *opaque)
     return s->fd;
 }
 
-static int socket_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+static ssize_t socket_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
+                                 size_t size)
 {
     QEMUFileSocket *s = opaque;
     ssize_t len;
@@ -138,7 +139,8 @@ static ssize_t unix_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
     return total;
 }
 
-static int unix_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+static ssize_t unix_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
+                              size_t size)
 {
     QEMUFileSocket *s = opaque;
     ssize_t len;
diff --git a/migration/rdma.c b/migration/rdma.c
index 74876fd..fd430c7 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -2519,8 +2519,8 @@ static void *qemu_rdma_data_init(const char *host_port, Error **errp)
  * SEND messages for control only.
  * VM's ram is handled with regular RDMA messages.
  */
-static int qemu_rdma_put_buffer(void *opaque, const uint8_t *buf,
-                                int64_t pos, int size)
+static ssize_t qemu_rdma_put_buffer(void *opaque, const uint8_t *buf,
+                                    int64_t pos, size_t size)
 {
     QEMUFileRDMA *r = opaque;
     QEMUFile *f = r->file;
@@ -2547,7 +2547,8 @@ static int qemu_rdma_put_buffer(void *opaque, const uint8_t *buf,
         r->len = MIN(remaining, RDMA_SEND_INCREMENT);
         remaining -= r->len;
 
-        head.len = r->len;
+        /* Guaranteed to fit due to RDMA_SEND_INCREMENT MIN above */
+        head.len = (uint32_t)r->len;
         head.type = RDMA_CONTROL_QEMU_FILE;
 
         ret = qemu_rdma_exchange_send(rdma, &head, data, NULL, NULL, NULL);
@@ -2564,7 +2565,7 @@ static int qemu_rdma_put_buffer(void *opaque, const uint8_t *buf,
 }
 
 static size_t qemu_rdma_fill(RDMAContext *rdma, uint8_t *buf,
-                             int size, int idx)
+                             size_t size, int idx)
 {
     size_t len = 0;
 
@@ -2585,8 +2586,8 @@ static size_t qemu_rdma_fill(RDMAContext *rdma, uint8_t *buf,
  * RDMA links don't use bytestreams, so we have to
  * return bytes to QEMUFile opportunistically.
  */
-static int qemu_rdma_get_buffer(void *opaque, uint8_t *buf,
-                                int64_t pos, int size)
+static ssize_t qemu_rdma_get_buffer(void *opaque, uint8_t *buf,
+                                    int64_t pos, size_t size)
 {
     QEMUFileRDMA *r = opaque;
     RDMAContext *rdma = r->rdma;
diff --git a/migration/savevm.c b/migration/savevm.c
index 6071215..1bd8827 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -138,14 +138,15 @@ static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
     return qiov.size;
 }
 
-static int block_put_buffer(void *opaque, const uint8_t *buf,
-                           int64_t pos, int size)
+static ssize_t block_put_buffer(void *opaque, const uint8_t *buf,
+                                int64_t pos, size_t size)
 {
     bdrv_save_vmstate(opaque, buf, pos, size);
     return size;
 }
 
-static int block_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+static ssize_t block_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
+                                size_t size)
 {
     return bdrv_load_vmstate(opaque, buf, pos, size);
 }
diff --git a/trace-events b/trace-events
index 1509e5b..1e7d9b6 100644
--- a/trace-events
+++ b/trace-events
@@ -1428,7 +1428,7 @@ qemu_rdma_exchange_get_response_none(const char *desc, int type) "Surprise: got
 qemu_rdma_exchange_send_issue_callback(void) ""
 qemu_rdma_exchange_send_waiting(const char *desc) "Waiting for response %s"
 qemu_rdma_exchange_send_received(const char *desc) "Response %s received."
-qemu_rdma_fill(int64_t control_len, int size) "RDMA %" PRId64 " of %d bytes already in buffer"
+qemu_rdma_fill(size_t control_len, size_t size) "RDMA %zd of %zd bytes already in buffer"
 qemu_rdma_init_ram_blocks(int blocks) "Allocated %d local ram block structures"
 qemu_rdma_poll_recv(const char *compstr, int64_t comp, int64_t id, int sent) "completion %s #%" PRId64 " received (%" PRId64 ") left %d"
 qemu_rdma_poll_write(const char *compstr, int64_t comp, int left, uint64_t block, uint64_t chunk, void *local, void *remote) "completions %s (%" PRId64 ") left %d, block %" PRIu64 ", chunk: %" PRIu64 " %p %p"
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 5/5] migration: qemu-file more size_t'ifying
  2015-08-13 10:51 [Qemu-devel] [PATCH 0/5] Migration cleanups Dr. David Alan Gilbert (git)
                   ` (3 preceding siblings ...)
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 4/5] migration: size_t'ify some of qemu-file Dr. David Alan Gilbert (git)
@ 2015-08-13 10:51 ` Dr. David Alan Gilbert (git)
  2015-08-13 12:15   ` zhanghailiang
  2015-09-15 11:42   ` Amit Shah
  4 siblings, 2 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2015-08-13 10:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: amit.shah, quintela

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

This time convert the external functions:
  qemu_get_buffer, qemu_peek_buffer
  qemu_put_buffer and qemu_put_buffer_async

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/migration/qemu-file.h | 10 +++++-----
 migration/qemu-file.c         | 22 +++++++++++-----------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index e1e2bab..29a338d 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -126,13 +126,13 @@ int qemu_get_fd(QEMUFile *f);
 int qemu_fclose(QEMUFile *f);
 int64_t qemu_ftell(QEMUFile *f);
 int64_t qemu_ftell_fast(QEMUFile *f);
-void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size);
+void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, size_t size);
 void qemu_put_byte(QEMUFile *f, int v);
 /*
  * put_buffer without copying the buffer.
  * The buffer should be available till it is sent asynchronously.
  */
-void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, int size);
+void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, size_t size);
 bool qemu_file_mode_is_not_valid(const char *mode);
 bool qemu_file_is_writable(QEMUFile *f);
 
@@ -161,8 +161,8 @@ static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v)
 void qemu_put_be16(QEMUFile *f, unsigned int v);
 void qemu_put_be32(QEMUFile *f, unsigned int v);
 void qemu_put_be64(QEMUFile *f, uint64_t v);
-int qemu_peek_buffer(QEMUFile *f, uint8_t **buf, int size, size_t offset);
-int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size);
+size_t qemu_peek_buffer(QEMUFile *f, uint8_t **buf, size_t size, size_t offset);
+size_t qemu_get_buffer(QEMUFile *f, uint8_t *buf, size_t size);
 ssize_t qemu_put_compression_data(QEMUFile *f, const uint8_t *p, size_t size,
                                   int level);
 int qemu_put_qemu_file(QEMUFile *f_des, QEMUFile *f_src);
@@ -237,7 +237,7 @@ static inline void qemu_get_8s(QEMUFile *f, uint8_t *pv)
 }
 
 // Signed versions for type safety
-static inline void qemu_put_sbuffer(QEMUFile *f, const int8_t *buf, int size)
+static inline void qemu_put_sbuffer(QEMUFile *f, const int8_t *buf, size_t size)
 {
     qemu_put_buffer(f, (const uint8_t *)buf, size);
 }
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 6bb3dc1..b273b1a 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -270,7 +270,7 @@ int qemu_fclose(QEMUFile *f)
     return ret;
 }
 
-static void add_to_iovec(QEMUFile *f, const uint8_t *buf, int size)
+static void add_to_iovec(QEMUFile *f, const uint8_t *buf, size_t size)
 {
     /* check for adjacent buffer and coalesce them */
     if (f->iovcnt > 0 && buf == f->iov[f->iovcnt - 1].iov_base +
@@ -286,7 +286,7 @@ static void add_to_iovec(QEMUFile *f, const uint8_t *buf, int size)
     }
 }
 
-void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, int size)
+void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, size_t size)
 {
     if (!f->ops->writev_buffer) {
         qemu_put_buffer(f, buf, size);
@@ -301,9 +301,9 @@ void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, int size)
     add_to_iovec(f, buf, size);
 }
 
-void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
+void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, size_t size)
 {
-    int l;
+    size_t l;
 
     if (f->last_error) {
         return;
@@ -363,10 +363,10 @@ void qemu_file_skip(QEMUFile *f, int size)
  * return as many as it managed to read (assuming blocking fd's which
  * all current QEMUFile are)
  */
-int qemu_peek_buffer(QEMUFile *f, uint8_t **buf, int size, size_t offset)
+size_t qemu_peek_buffer(QEMUFile *f, uint8_t **buf, size_t size, size_t offset)
 {
-    int pending;
-    int index;
+    ssize_t pending;
+    size_t index;
 
     assert(!qemu_file_is_writable(f));
     assert(offset < IO_BUF_SIZE);
@@ -411,13 +411,13 @@ int qemu_peek_buffer(QEMUFile *f, uint8_t **buf, int size, size_t offset)
  * return as many as it managed to read (assuming blocking fd's which
  * all current QEMUFile are)
  */
-int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
+size_t qemu_get_buffer(QEMUFile *f, uint8_t *buf, size_t size)
 {
-    int pending = size;
-    int done = 0;
+    size_t pending = size;
+    size_t done = 0;
 
     while (pending > 0) {
-        int res;
+        size_t res;
         uint8_t *src;
 
         res = qemu_peek_buffer(f, &src, MIN(pending, IO_BUF_SIZE), 0);
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 2/5] Split out end of migration code from migration_thread
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 2/5] Split out end of migration code from migration_thread Dr. David Alan Gilbert (git)
@ 2015-08-13 12:14   ` zhanghailiang
  2015-09-15 11:12   ` Amit Shah
  1 sibling, 0 replies; 14+ messages in thread
From: zhanghailiang @ 2015-08-13 12:14 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git), qemu-devel
  Cc: amit.shah, peter.huangpeng, quintela

On 2015/8/13 18:51, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The code that gets run at the end of the migration process
> is getting large, and I'm about to add more for postcopy.
> Split it into a separate function.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---

Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com>

>   migration/migration.c | 75 ++++++++++++++++++++++++++++++++-------------------
>   trace-events          |  2 ++
>   2 files changed, 49 insertions(+), 28 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 662e77e..46bb410 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -913,6 +913,50 @@ int64_t migrate_xbzrle_cache_size(void)
>       return s->xbzrle_cache_size;
>   }
>
> +/**
> + * migration_completion: Used by migration_thread when there's not much left.
> + *   The caller 'breaks' the loop when this returns.
> + *
> + * @s: Current migration state
> + * @*old_vm_running: Pointer to old_vm_running flag
> + * @*start_time: Pointer to time to update
> + */
> +static void migration_completion(MigrationState *s, bool *old_vm_running,
> +                                 int64_t *start_time)
> +{
> +    int ret;
> +
> +    qemu_mutex_lock_iothread();
> +    *start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> +    qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
> +    *old_vm_running = runstate_is_running();
> +
> +    ret = global_state_store();
> +    if (!ret) {
> +        ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
> +        if (ret >= 0) {
> +            qemu_file_set_rate_limit(s->file, INT64_MAX);
> +            qemu_savevm_state_complete(s->file);
> +        }
> +    }
> +    qemu_mutex_unlock_iothread();
> +
> +    if (ret < 0) {
> +        goto fail;
> +    }
> +
> +    if (qemu_file_get_error(s->file)) {
> +        trace_migration_completion_file_err();
> +        goto fail;
> +    }
> +
> +    migrate_set_state(s, MIGRATION_STATUS_ACTIVE, MIGRATION_STATUS_COMPLETED);
> +    return;
> +
> +fail:
> +    migrate_set_state(s, MIGRATION_STATUS_ACTIVE, MIGRATION_STATUS_FAILED);
> +}
> +
>   /* migration thread support */
>
>   static void *migration_thread(void *opaque)
> @@ -943,34 +987,9 @@ static void *migration_thread(void *opaque)
>               if (pending_size && pending_size >= max_size) {
>                   qemu_savevm_state_iterate(s->file);
>               } else {
> -                int ret;
> -
> -                qemu_mutex_lock_iothread();
> -                start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> -                qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
> -                old_vm_running = runstate_is_running();
> -
> -                ret = global_state_store();
> -                if (!ret) {
> -                    ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
> -                    if (ret >= 0) {
> -                        qemu_file_set_rate_limit(s->file, INT64_MAX);
> -                        qemu_savevm_state_complete(s->file);
> -                    }
> -                }
> -                qemu_mutex_unlock_iothread();
> -
> -                if (ret < 0) {
> -                    migrate_set_state(s, MIGRATION_STATUS_ACTIVE,
> -                                      MIGRATION_STATUS_FAILED);
> -                    break;
> -                }
> -
> -                if (!qemu_file_get_error(s->file)) {
> -                    migrate_set_state(s, MIGRATION_STATUS_ACTIVE,
> -                                      MIGRATION_STATUS_COMPLETED);
> -                    break;
> -                }
> +                trace_migration_thread_low_pending(pending_size);
> +                migration_completion(s, &old_vm_running, &start_time);
> +                break;
>               }
>           }
>
> diff --git a/trace-events b/trace-events
> index 94bf3bb..1509e5b 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1406,6 +1406,8 @@ migrate_transferred(uint64_t tranferred, uint64_t time_spent, double bandwidth,
>   migrate_state_too_big(void) ""
>   migrate_global_state_post_load(const char *state) "loaded state: %s"
>   migrate_global_state_pre_save(const char *state) "saved state: %s"
> +migration_completion_file_err(void) ""
> +migration_thread_low_pending(uint64_t pending) "%" PRIu64
>
>   # migration/rdma.c
>   qemu_rdma_accept_incoming_migration(void) ""
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 4/5] migration: size_t'ify some of qemu-file
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 4/5] migration: size_t'ify some of qemu-file Dr. David Alan Gilbert (git)
@ 2015-08-13 12:15   ` zhanghailiang
  2015-09-15 11:39   ` Amit Shah
  1 sibling, 0 replies; 14+ messages in thread
From: zhanghailiang @ 2015-08-13 12:15 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git), qemu-devel
  Cc: amit.shah, peter.huangpeng, quintela

On 2015/8/13 18:51, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> This is a start on using size_t more in qemu-file and friends;
> it fixes up QEMUFilePutBufferFunc and QEMUFileGetBufferFunc
> to take size_t lengths and return ssize_t return values (like read(2))
> and fixes up all the different implementations of them.
>
> Note that I've not yet followed this deeply into bdrv_ implementations.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---

Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com>

>   include/migration/qemu-file.h |  8 ++++----
>   migration/qemu-file-buf.c     |  7 ++++---
>   migration/qemu-file-stdio.c   | 11 ++++++-----
>   migration/qemu-file-unix.c    |  6 ++++--
>   migration/rdma.c              | 13 +++++++------
>   migration/savevm.c            |  7 ++++---
>   trace-events                  |  2 +-
>   7 files changed, 30 insertions(+), 24 deletions(-)
>
> diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
> index ea49f33..e1e2bab 100644
> --- a/include/migration/qemu-file.h
> +++ b/include/migration/qemu-file.h
> @@ -31,15 +31,15 @@
>    * The pos argument can be ignored if the file is only being used for
>    * streaming.  The handler should try to write all of the data it can.
>    */
> -typedef int (QEMUFilePutBufferFunc)(void *opaque, const uint8_t *buf,
> -                                    int64_t pos, int size);
> +typedef ssize_t (QEMUFilePutBufferFunc)(void *opaque, const uint8_t *buf,
> +                                        int64_t pos, size_t size);
>
>   /* Read a chunk of data from a file at the given position.  The pos argument
>    * can be ignored if the file is only be used for streaming.  The number of
>    * bytes actually read should be returned.
>    */
> -typedef int (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
> -                                    int64_t pos, int size);
> +typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
> +                                        int64_t pos, size_t size);
>
>   /* Close a file
>    *
> diff --git a/migration/qemu-file-buf.c b/migration/qemu-file-buf.c
> index 2de9330..1d9528e 100644
> --- a/migration/qemu-file-buf.c
> +++ b/migration/qemu-file-buf.c
> @@ -372,7 +372,8 @@ typedef struct QEMUBuffer {
>       bool qsb_allocated;
>   } QEMUBuffer;
>
> -static int buf_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
> +static ssize_t buf_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
> +                              size_t size)
>   {
>       QEMUBuffer *s = opaque;
>       ssize_t len = qsb_get_length(s->qsb) - pos;
> @@ -387,8 +388,8 @@ static int buf_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
>       return qsb_get_buffer(s->qsb, pos, len, buf);
>   }
>
> -static int buf_put_buffer(void *opaque, const uint8_t *buf,
> -                          int64_t pos, int size)
> +static ssize_t buf_put_buffer(void *opaque, const uint8_t *buf,
> +                              int64_t pos, size_t size)
>   {
>       QEMUBuffer *s = opaque;
>
> diff --git a/migration/qemu-file-stdio.c b/migration/qemu-file-stdio.c
> index 285068b..dc91137 100644
> --- a/migration/qemu-file-stdio.c
> +++ b/migration/qemu-file-stdio.c
> @@ -37,11 +37,11 @@ static int stdio_get_fd(void *opaque)
>       return fileno(s->stdio_file);
>   }
>
> -static int stdio_put_buffer(void *opaque, const uint8_t *buf, int64_t pos,
> -                            int size)
> +static ssize_t stdio_put_buffer(void *opaque, const uint8_t *buf, int64_t pos,
> +                                size_t size)
>   {
>       QEMUFileStdio *s = opaque;
> -    int res;
> +    size_t res;
>
>       res = fwrite(buf, 1, size, s->stdio_file);
>
> @@ -51,11 +51,12 @@ static int stdio_put_buffer(void *opaque, const uint8_t *buf, int64_t pos,
>       return res;
>   }
>
> -static int stdio_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
> +static ssize_t stdio_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
> +                                size_t size)
>   {
>       QEMUFileStdio *s = opaque;
>       FILE *fp = s->stdio_file;
> -    int bytes;
> +    ssize_t bytes;
>
>       for (;;) {
>           clearerr(fp);
> diff --git a/migration/qemu-file-unix.c b/migration/qemu-file-unix.c
> index bfbc086..adfe91a 100644
> --- a/migration/qemu-file-unix.c
> +++ b/migration/qemu-file-unix.c
> @@ -54,7 +54,8 @@ static int socket_get_fd(void *opaque)
>       return s->fd;
>   }
>
> -static int socket_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
> +static ssize_t socket_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
> +                                 size_t size)
>   {
>       QEMUFileSocket *s = opaque;
>       ssize_t len;
> @@ -138,7 +139,8 @@ static ssize_t unix_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
>       return total;
>   }
>
> -static int unix_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
> +static ssize_t unix_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
> +                              size_t size)
>   {
>       QEMUFileSocket *s = opaque;
>       ssize_t len;
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 74876fd..fd430c7 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -2519,8 +2519,8 @@ static void *qemu_rdma_data_init(const char *host_port, Error **errp)
>    * SEND messages for control only.
>    * VM's ram is handled with regular RDMA messages.
>    */
> -static int qemu_rdma_put_buffer(void *opaque, const uint8_t *buf,
> -                                int64_t pos, int size)
> +static ssize_t qemu_rdma_put_buffer(void *opaque, const uint8_t *buf,
> +                                    int64_t pos, size_t size)
>   {
>       QEMUFileRDMA *r = opaque;
>       QEMUFile *f = r->file;
> @@ -2547,7 +2547,8 @@ static int qemu_rdma_put_buffer(void *opaque, const uint8_t *buf,
>           r->len = MIN(remaining, RDMA_SEND_INCREMENT);
>           remaining -= r->len;
>
> -        head.len = r->len;
> +        /* Guaranteed to fit due to RDMA_SEND_INCREMENT MIN above */
> +        head.len = (uint32_t)r->len;
>           head.type = RDMA_CONTROL_QEMU_FILE;
>
>           ret = qemu_rdma_exchange_send(rdma, &head, data, NULL, NULL, NULL);
> @@ -2564,7 +2565,7 @@ static int qemu_rdma_put_buffer(void *opaque, const uint8_t *buf,
>   }
>
>   static size_t qemu_rdma_fill(RDMAContext *rdma, uint8_t *buf,
> -                             int size, int idx)
> +                             size_t size, int idx)
>   {
>       size_t len = 0;
>
> @@ -2585,8 +2586,8 @@ static size_t qemu_rdma_fill(RDMAContext *rdma, uint8_t *buf,
>    * RDMA links don't use bytestreams, so we have to
>    * return bytes to QEMUFile opportunistically.
>    */
> -static int qemu_rdma_get_buffer(void *opaque, uint8_t *buf,
> -                                int64_t pos, int size)
> +static ssize_t qemu_rdma_get_buffer(void *opaque, uint8_t *buf,
> +                                    int64_t pos, size_t size)
>   {
>       QEMUFileRDMA *r = opaque;
>       RDMAContext *rdma = r->rdma;
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 6071215..1bd8827 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -138,14 +138,15 @@ static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
>       return qiov.size;
>   }
>
> -static int block_put_buffer(void *opaque, const uint8_t *buf,
> -                           int64_t pos, int size)
> +static ssize_t block_put_buffer(void *opaque, const uint8_t *buf,
> +                                int64_t pos, size_t size)
>   {
>       bdrv_save_vmstate(opaque, buf, pos, size);
>       return size;
>   }
>
> -static int block_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
> +static ssize_t block_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
> +                                size_t size)
>   {
>       return bdrv_load_vmstate(opaque, buf, pos, size);
>   }
> diff --git a/trace-events b/trace-events
> index 1509e5b..1e7d9b6 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1428,7 +1428,7 @@ qemu_rdma_exchange_get_response_none(const char *desc, int type) "Surprise: got
>   qemu_rdma_exchange_send_issue_callback(void) ""
>   qemu_rdma_exchange_send_waiting(const char *desc) "Waiting for response %s"
>   qemu_rdma_exchange_send_received(const char *desc) "Response %s received."
> -qemu_rdma_fill(int64_t control_len, int size) "RDMA %" PRId64 " of %d bytes already in buffer"
> +qemu_rdma_fill(size_t control_len, size_t size) "RDMA %zd of %zd bytes already in buffer"
>   qemu_rdma_init_ram_blocks(int blocks) "Allocated %d local ram block structures"
>   qemu_rdma_poll_recv(const char *compstr, int64_t comp, int64_t id, int sent) "completion %s #%" PRId64 " received (%" PRId64 ") left %d"
>   qemu_rdma_poll_write(const char *compstr, int64_t comp, int left, uint64_t block, uint64_t chunk, void *local, void *remote) "completions %s (%" PRId64 ") left %d, block %" PRIu64 ", chunk: %" PRIu64 " %p %p"
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 5/5] migration: qemu-file more size_t'ifying
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 5/5] migration: qemu-file more size_t'ifying Dr. David Alan Gilbert (git)
@ 2015-08-13 12:15   ` zhanghailiang
  2015-09-15 11:42   ` Amit Shah
  1 sibling, 0 replies; 14+ messages in thread
From: zhanghailiang @ 2015-08-13 12:15 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git), qemu-devel
  Cc: amit.shah, peter.huangpeng, quintela

On 2015/8/13 18:51, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> This time convert the external functions:
>    qemu_get_buffer, qemu_peek_buffer
>    qemu_put_buffer and qemu_put_buffer_async
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---

Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com>

>   include/migration/qemu-file.h | 10 +++++-----
>   migration/qemu-file.c         | 22 +++++++++++-----------
>   2 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
> index e1e2bab..29a338d 100644
> --- a/include/migration/qemu-file.h
> +++ b/include/migration/qemu-file.h
> @@ -126,13 +126,13 @@ int qemu_get_fd(QEMUFile *f);
>   int qemu_fclose(QEMUFile *f);
>   int64_t qemu_ftell(QEMUFile *f);
>   int64_t qemu_ftell_fast(QEMUFile *f);
> -void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size);
> +void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, size_t size);
>   void qemu_put_byte(QEMUFile *f, int v);
>   /*
>    * put_buffer without copying the buffer.
>    * The buffer should be available till it is sent asynchronously.
>    */
> -void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, int size);
> +void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, size_t size);
>   bool qemu_file_mode_is_not_valid(const char *mode);
>   bool qemu_file_is_writable(QEMUFile *f);
>
> @@ -161,8 +161,8 @@ static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v)
>   void qemu_put_be16(QEMUFile *f, unsigned int v);
>   void qemu_put_be32(QEMUFile *f, unsigned int v);
>   void qemu_put_be64(QEMUFile *f, uint64_t v);
> -int qemu_peek_buffer(QEMUFile *f, uint8_t **buf, int size, size_t offset);
> -int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size);
> +size_t qemu_peek_buffer(QEMUFile *f, uint8_t **buf, size_t size, size_t offset);
> +size_t qemu_get_buffer(QEMUFile *f, uint8_t *buf, size_t size);
>   ssize_t qemu_put_compression_data(QEMUFile *f, const uint8_t *p, size_t size,
>                                     int level);
>   int qemu_put_qemu_file(QEMUFile *f_des, QEMUFile *f_src);
> @@ -237,7 +237,7 @@ static inline void qemu_get_8s(QEMUFile *f, uint8_t *pv)
>   }
>
>   // Signed versions for type safety
> -static inline void qemu_put_sbuffer(QEMUFile *f, const int8_t *buf, int size)
> +static inline void qemu_put_sbuffer(QEMUFile *f, const int8_t *buf, size_t size)
>   {
>       qemu_put_buffer(f, (const uint8_t *)buf, size);
>   }
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 6bb3dc1..b273b1a 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -270,7 +270,7 @@ int qemu_fclose(QEMUFile *f)
>       return ret;
>   }
>
> -static void add_to_iovec(QEMUFile *f, const uint8_t *buf, int size)
> +static void add_to_iovec(QEMUFile *f, const uint8_t *buf, size_t size)
>   {
>       /* check for adjacent buffer and coalesce them */
>       if (f->iovcnt > 0 && buf == f->iov[f->iovcnt - 1].iov_base +
> @@ -286,7 +286,7 @@ static void add_to_iovec(QEMUFile *f, const uint8_t *buf, int size)
>       }
>   }
>
> -void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, int size)
> +void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, size_t size)
>   {
>       if (!f->ops->writev_buffer) {
>           qemu_put_buffer(f, buf, size);
> @@ -301,9 +301,9 @@ void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, int size)
>       add_to_iovec(f, buf, size);
>   }
>
> -void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
> +void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, size_t size)
>   {
> -    int l;
> +    size_t l;
>
>       if (f->last_error) {
>           return;
> @@ -363,10 +363,10 @@ void qemu_file_skip(QEMUFile *f, int size)
>    * return as many as it managed to read (assuming blocking fd's which
>    * all current QEMUFile are)
>    */
> -int qemu_peek_buffer(QEMUFile *f, uint8_t **buf, int size, size_t offset)
> +size_t qemu_peek_buffer(QEMUFile *f, uint8_t **buf, size_t size, size_t offset)
>   {
> -    int pending;
> -    int index;
> +    ssize_t pending;
> +    size_t index;
>
>       assert(!qemu_file_is_writable(f));
>       assert(offset < IO_BUF_SIZE);
> @@ -411,13 +411,13 @@ int qemu_peek_buffer(QEMUFile *f, uint8_t **buf, int size, size_t offset)
>    * return as many as it managed to read (assuming blocking fd's which
>    * all current QEMUFile are)
>    */
> -int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
> +size_t qemu_get_buffer(QEMUFile *f, uint8_t *buf, size_t size)
>   {
> -    int pending = size;
> -    int done = 0;
> +    size_t pending = size;
> +    size_t done = 0;
>
>       while (pending > 0) {
> -        int res;
> +        size_t res;
>           uint8_t *src;
>
>           res = qemu_peek_buffer(f, &src, MIN(pending, IO_BUF_SIZE), 0);
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 1/5] migration/ram.c: Use RAMBlock rather than MemoryRegion
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 1/5] migration/ram.c: Use RAMBlock rather than MemoryRegion Dr. David Alan Gilbert (git)
@ 2015-08-13 12:46   ` Paolo Bonzini
  2015-09-15 10:43   ` Amit Shah
  1 sibling, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2015-08-13 12:46 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git), qemu-devel; +Cc: amit.shah, quintela



On 13/08/2015 12:51, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> RAM migration mainly works on RAMBlocks but in a few places
> uses data from MemoryRegions to access the same information that's
> already held in RAMBlocks; clean it up just to avoid the
> MemoryRegion use.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  migration/ram.c | 26 +++++++++++---------------
>  1 file changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 7f007e6..7df9157 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -497,13 +497,13 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t **current_data,
>  
>  /* Called with rcu_read_lock() to protect migration_bitmap */
>  static inline
> -ram_addr_t migration_bitmap_find_and_reset_dirty(MemoryRegion *mr,
> +ram_addr_t migration_bitmap_find_and_reset_dirty(RAMBlock *rb,
>                                                   ram_addr_t start)
>  {
> -    unsigned long base = mr->ram_addr >> TARGET_PAGE_BITS;
> +    unsigned long base = rb->offset >> TARGET_PAGE_BITS;
>      unsigned long nr = base + (start >> TARGET_PAGE_BITS);
> -    uint64_t mr_size = TARGET_PAGE_ALIGN(memory_region_size(mr));
> -    unsigned long size = base + (mr_size >> TARGET_PAGE_BITS);
> +    uint64_t rb_size = rb->used_length;
> +    unsigned long size = base + (rb_size >> TARGET_PAGE_BITS);
>      unsigned long *bitmap;
>  
>      unsigned long next;
> @@ -573,7 +573,7 @@ static void migration_bitmap_sync(void)
>      qemu_mutex_lock(&migration_bitmap_mutex);
>      rcu_read_lock();
>      QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
> -        migration_bitmap_sync_range(block->mr->ram_addr, block->used_length);
> +        migration_bitmap_sync_range(block->offset, block->used_length);
>      }
>      rcu_read_unlock();
>      qemu_mutex_unlock(&migration_bitmap_mutex);
> @@ -668,12 +668,11 @@ static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset,
>      int pages = -1;
>      uint64_t bytes_xmit;
>      ram_addr_t current_addr;
> -    MemoryRegion *mr = block->mr;
>      uint8_t *p;
>      int ret;
>      bool send_async = true;
>  
> -    p = memory_region_get_ram_ptr(mr) + offset;
> +    p = block->host + offset;
>  
>      /* In doubt sent page as normal */
>      bytes_xmit = 0;
> @@ -744,7 +743,7 @@ static int do_compress_ram_page(CompressParam *param)
>      RAMBlock *block = param->block;
>      ram_addr_t offset = param->offset;
>  
> -    p = memory_region_get_ram_ptr(block->mr) + (offset & TARGET_PAGE_MASK);
> +    p = block->host + (offset & TARGET_PAGE_MASK);
>  
>      bytes_sent = save_page_header(param->file, block, offset |
>                                    RAM_SAVE_FLAG_COMPRESS_PAGE);
> @@ -852,11 +851,10 @@ static int ram_save_compressed_page(QEMUFile *f, RAMBlock *block,
>  {
>      int pages = -1;
>      uint64_t bytes_xmit;
> -    MemoryRegion *mr = block->mr;
>      uint8_t *p;
>      int ret;
>  
> -    p = memory_region_get_ram_ptr(mr) + offset;
> +    p = block->host + offset;
>  
>      bytes_xmit = 0;
>      ret = ram_control_save_page(f, block->offset,
> @@ -929,14 +927,12 @@ static int ram_find_and_save_block(QEMUFile *f, bool last_stage,
>      ram_addr_t offset = last_offset;
>      bool complete_round = false;
>      int pages = 0;
> -    MemoryRegion *mr;
>  
>      if (!block)
>          block = QLIST_FIRST_RCU(&ram_list.blocks);
>  
>      while (true) {
> -        mr = block->mr;
> -        offset = migration_bitmap_find_and_reset_dirty(mr, offset);
> +        offset = migration_bitmap_find_and_reset_dirty(block, offset);
>          if (complete_round && block == last_seen_block &&
>              offset >= last_offset) {
>              break;
> @@ -1344,7 +1340,7 @@ static inline void *host_from_stream_offset(QEMUFile *f,
>              return NULL;
>          }
>  
> -        return memory_region_get_ram_ptr(block->mr) + offset;
> +        return block->host + offset;
>      }
>  
>      len = qemu_get_byte(f);
> @@ -1354,7 +1350,7 @@ static inline void *host_from_stream_offset(QEMUFile *f,
>      QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
>          if (!strncmp(id, block->idstr, sizeof(id)) &&
>              block->max_length > offset) {
> -            return memory_region_get_ram_ptr(block->mr) + offset;
> +            return block->host + offset;
>          }
>      }
>  
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

It would be nice in a follow-up patch to move RAMBlock-related
definitions into include/exec/ram_addr.h.

Paolo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 1/5] migration/ram.c: Use RAMBlock rather than MemoryRegion
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 1/5] migration/ram.c: Use RAMBlock rather than MemoryRegion Dr. David Alan Gilbert (git)
  2015-08-13 12:46   ` Paolo Bonzini
@ 2015-09-15 10:43   ` Amit Shah
  1 sibling, 0 replies; 14+ messages in thread
From: Amit Shah @ 2015-09-15 10:43 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, quintela

On (Thu) 13 Aug 2015 [11:51:30], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> RAM migration mainly works on RAMBlocks but in a few places
> uses data from MemoryRegions to access the same information that's
> already held in RAMBlocks; clean it up just to avoid the
> MemoryRegion use.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Amit Shah <amit.shah@redhat.com>

		Amit

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 2/5] Split out end of migration code from migration_thread
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 2/5] Split out end of migration code from migration_thread Dr. David Alan Gilbert (git)
  2015-08-13 12:14   ` zhanghailiang
@ 2015-09-15 11:12   ` Amit Shah
  1 sibling, 0 replies; 14+ messages in thread
From: Amit Shah @ 2015-09-15 11:12 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, quintela

On (Thu) 13 Aug 2015 [11:51:31], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> The code that gets run at the end of the migration process
> is getting large, and I'm about to add more for postcopy.
> Split it into a separate function.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Amit Shah <amit.shah@redhat.com>

> ---
>  migration/migration.c | 75 ++++++++++++++++++++++++++++++++-------------------
>  trace-events          |  2 ++
>  2 files changed, 49 insertions(+), 28 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 662e77e..46bb410 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -913,6 +913,50 @@ int64_t migrate_xbzrle_cache_size(void)
>      return s->xbzrle_cache_size;
>  }
>  
> +/**
> + * migration_completion: Used by migration_thread when there's not much left.
> + *   The caller 'breaks' the loop when this returns.
> + *
> + * @s: Current migration state
> + * @*old_vm_running: Pointer to old_vm_running flag
> + * @*start_time: Pointer to time to update
> + */
> +static void migration_completion(MigrationState *s, bool *old_vm_running,
> +                                 int64_t *start_time)
> +{
> +    int ret;
> +
> +    qemu_mutex_lock_iothread();
> +    *start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> +    qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
> +    *old_vm_running = runstate_is_running();
> +
> +    ret = global_state_store();
> +    if (!ret) {
> +        ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
> +        if (ret >= 0) {
> +            qemu_file_set_rate_limit(s->file, INT64_MAX);
> +            qemu_savevm_state_complete(s->file);
> +        }
> +    }

The case where ret >= 0 and where s->file has an error didn't set
status to failed nor break'ed in the removed context below.  But it
happened a few lines outside the context anyway.

Now it just happens in this function, which is cleaner and easier to
understand.

		Amit

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 4/5] migration: size_t'ify some of qemu-file
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 4/5] migration: size_t'ify some of qemu-file Dr. David Alan Gilbert (git)
  2015-08-13 12:15   ` zhanghailiang
@ 2015-09-15 11:39   ` Amit Shah
  1 sibling, 0 replies; 14+ messages in thread
From: Amit Shah @ 2015-09-15 11:39 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, quintela

On (Thu) 13 Aug 2015 [11:51:33], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> This is a start on using size_t more in qemu-file and friends;
> it fixes up QEMUFilePutBufferFunc and QEMUFileGetBufferFunc
> to take size_t lengths and return ssize_t return values (like read(2))
> and fixes up all the different implementations of them.
> 
> Note that I've not yet followed this deeply into bdrv_ implementations.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Amit Shah <amit.shah@redhat.com>

This is really nice, thanks!

(Though it got me into thinking what values are fine to be stored in
size_t and ssize_t: and the POSIX spec doesn't specify what the
SSIZE_MIN value is; it only says ssize_t can hold -1..SIZE_MAX.  Hope
we don't have fun due to the -errno return values.)

		Amit

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 5/5] migration: qemu-file more size_t'ifying
  2015-08-13 10:51 ` [Qemu-devel] [PATCH 5/5] migration: qemu-file more size_t'ifying Dr. David Alan Gilbert (git)
  2015-08-13 12:15   ` zhanghailiang
@ 2015-09-15 11:42   ` Amit Shah
  1 sibling, 0 replies; 14+ messages in thread
From: Amit Shah @ 2015-09-15 11:42 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, quintela

On (Thu) 13 Aug 2015 [11:51:34], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> This time convert the external functions:
>   qemu_get_buffer, qemu_peek_buffer
>   qemu_put_buffer and qemu_put_buffer_async
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Amit Shah <amit.shah@redhat.com>

		Amit

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-09-15 11:43 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-13 10:51 [Qemu-devel] [PATCH 0/5] Migration cleanups Dr. David Alan Gilbert (git)
2015-08-13 10:51 ` [Qemu-devel] [PATCH 1/5] migration/ram.c: Use RAMBlock rather than MemoryRegion Dr. David Alan Gilbert (git)
2015-08-13 12:46   ` Paolo Bonzini
2015-09-15 10:43   ` Amit Shah
2015-08-13 10:51 ` [Qemu-devel] [PATCH 2/5] Split out end of migration code from migration_thread Dr. David Alan Gilbert (git)
2015-08-13 12:14   ` zhanghailiang
2015-09-15 11:12   ` Amit Shah
2015-08-13 10:51 ` [Qemu-devel] [PATCH 3/5] Init page sizes in qtest Dr. David Alan Gilbert (git)
2015-08-13 10:51 ` [Qemu-devel] [PATCH 4/5] migration: size_t'ify some of qemu-file Dr. David Alan Gilbert (git)
2015-08-13 12:15   ` zhanghailiang
2015-09-15 11:39   ` Amit Shah
2015-08-13 10:51 ` [Qemu-devel] [PATCH 5/5] migration: qemu-file more size_t'ifying Dr. David Alan Gilbert (git)
2015-08-13 12:15   ` zhanghailiang
2015-09-15 11:42   ` Amit Shah

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).