[Qemu-devel] [PATCH v4 36/47] Page request: Process incoming page request
From: "Dr. David Alan Gilbert"
On receiving MIG_RPCOMM_REQPAGES look up the address and
queue the page.
Signed-off-by: Dr. David Alan Gilbert
---
arch_init.c | 52 +++
include/migration/migration.h | 21 +
include/qemu/typedefs.h | 3 ++-
migration.c | 34 +++-
4 files changed, 108 insertions(+), 2 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 4a03171..72f9e17 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -660,6 +660,58 @@ static int ram_save_page(QEMUFile *f, RAMBlock* block,
ram_addr_t offset,
}
/*
+ * Queue the pages for transmission, e.g. a request from postcopy destination
+ * ms: MigrationStatus in which the queue is held
+ * rbname: The RAMBlock the request is for - may be NULL (to mean reuse last)
+ * start: Offset from the start of the RAMBlock
+ * len: Length (in bytes) to send
+ * Return: 0 on success
+ */
+int ram_save_queue_pages(MigrationState *ms, const char *rbname,
+ ram_addr_t start, ram_addr_t len)
+{
+RAMBlock *ramblock;
+
+if (!rbname) {
+/* Reuse last RAMBlock */
+ramblock = ms->last_req_rb;
+
+if (!ramblock) {
+error_report("ram_save_queue_pages no previous block");
+return -1;
+}
+} else {
+ramblock = ram_find_block(rbname);
+
+if (!ramblock) {
+error_report("ram_save_queue_pages no block '%s'", rbname);
+return -1;
+}
+}
+DPRINTF("ram_save_queue_pages: Block %s start %zx len %zx",
+ramblock->idstr, start, len);
+
+if (start+len > ramblock->length) {
+error_report("%s request overrun start=%zx len=%zx blocklen=%zx",
+ __func__, start, len, ramblock->length);
+return -1;
+}
+
+struct MigrationSrcPageRequest *new_entry =
+g_malloc0(sizeof(struct MigrationSrcPageRequest));
+new_entry->rb = ramblock;
+new_entry->offset = start;
+new_entry->len = len;
+ms->last_req_rb = ramblock;
+
+qemu_mutex_lock(&ms->src_page_req_mutex);
+QSIMPLEQ_INSERT_TAIL(&ms->src_page_requests, new_entry, next_req);
+qemu_mutex_unlock(&ms->src_page_req_mutex);
+
+return 0;
+}
+
+/*
* ram_find_and_save_block: Finds a page to send and sends it to f
*
* Returns: The number of bytes written.
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 5e0d30d..5bc01d5 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -102,6 +102,18 @@ MigrationIncomingState
*migration_incoming_get_current(void);
MigrationIncomingState *migration_incoming_state_init(QEMUFile *f);
void migration_incoming_state_destroy(void);
+/*
+ * An outstanding page request, on the source, having been received
+ * and queued
+ */
+struct MigrationSrcPageRequest {
+RAMBlock *rb;
+hwaddroffset;
+hwaddrlen;
+
+QSIMPLEQ_ENTRY(MigrationSrcPageRequest) next_req;
+};
+
struct MigrationState
{
int64_t bandwidth_limit;
@@ -138,6 +150,12 @@ struct MigrationState
* of the postcopy phase
*/
unsigned long *sentmap;
+
+/* Queue of outstanding page requests from the destination */
+QemuMutex src_page_req_mutex;
+QSIMPLEQ_HEAD(src_page_requests, MigrationSrcPageRequest)
src_page_requests;
+/* The RAMBlock used in the last src_page_request */
+RAMBlock *last_req_rb;
};
void process_incoming_migration(QEMUFile *f);
@@ -273,4 +291,7 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t
block_offset,
ram_addr_t offset, size_t size,
int *bytes_sent);
+int ram_save_queue_pages(MigrationState *ms, const char *rbname,
+ ram_addr_t start, ram_addr_t len);
+
#endif
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 79f57c0..24c2207 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -8,6 +8,7 @@ typedef struct QEMUTimerListGroup QEMUTimerListGroup;
typedef struct QEMUFile QEMUFile;
typedef struct QEMUBH QEMUBH;
+typedef struct AdapterInfo AdapterInfo;
typedef struct AioContext AioContext;
typedef struct Visitor Visitor;
@@ -80,6 +81,6 @@ typedef struct FWCfgState FWCfgState;
typedef struct PcGuestInfo PcGuestInfo;
typedef struct PostcopyPMI PostcopyPMI;
typedef struct Range Range;
-typedef struct AdapterInfo AdapterInfo;
+typedef struct RAMBlock RAMBlock;
#endif /* QEMU_TYPEDEFS_H */
diff --git a/migration.c b/migration.c
index cfdaa52..63d7699 100644
--- a/migration.c
+++ b/migration.c
@@ -26,6 +26,8 @@
#include "qemu/thread.h"
#include "qmp-commands.h"
#include "trace.h"
+#include "exec/memory.h"
+#include "exec/address-spaces.h"
//#define DEBUG_MIGRATION
@@ -504,6 +506,15 @@ static void migrate_fd_cleanup(void *opaque)
migrate_fd_cleanup_src_rp(s);
+/* This queue generally sh
Re: [Qemu-devel] [PATCH v4 36/47] Page request: Process incoming page request
On 2014/10/4 1:47, Dr. David Alan Gilbert (git) wrote:
From: "Dr. David Alan Gilbert"
On receiving MIG_RPCOMM_REQPAGES look up the address and
queue the page.
Signed-off-by: Dr. David Alan Gilbert
---
arch_init.c | 52 +++
include/migration/migration.h | 21 +
include/qemu/typedefs.h | 3 ++-
migration.c | 34 +++-
4 files changed, 108 insertions(+), 2 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 4a03171..72f9e17 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -660,6 +660,58 @@ static int ram_save_page(QEMUFile *f, RAMBlock* block,
ram_addr_t offset,
}
/*
+ * Queue the pages for transmission, e.g. a request from postcopy destination
+ * ms: MigrationStatus in which the queue is held
+ * rbname: The RAMBlock the request is for - may be NULL (to mean reuse last)
+ * start: Offset from the start of the RAMBlock
+ * len: Length (in bytes) to send
+ * Return: 0 on success
+ */
+int ram_save_queue_pages(MigrationState *ms, const char *rbname,
+ ram_addr_t start, ram_addr_t len)
+{
+RAMBlock *ramblock;
+
+if (!rbname) {
+/* Reuse last RAMBlock */
+ramblock = ms->last_req_rb;
+
+if (!ramblock) {
+error_report("ram_save_queue_pages no previous block");
+return -1;
+}
+} else {
+ramblock = ram_find_block(rbname);
+
+if (!ramblock) {
+error_report("ram_save_queue_pages no block '%s'", rbname);
+return -1;
+}
+}
+DPRINTF("ram_save_queue_pages: Block %s start %zx len %zx",
+ramblock->idstr, start, len);
+
+if (start+len > ramblock->length) {
+error_report("%s request overrun start=%zx len=%zx blocklen=%zx",
+ __func__, start, len, ramblock->length);
+return -1;
+}
+
+struct MigrationSrcPageRequest *new_entry =
+g_malloc0(sizeof(struct MigrationSrcPageRequest));
+new_entry->rb = ramblock;
+new_entry->offset = start;
+new_entry->len = len;
+ms->last_req_rb = ramblock;
+
+qemu_mutex_lock(&ms->src_page_req_mutex);
+QSIMPLEQ_INSERT_TAIL(&ms->src_page_requests, new_entry, next_req);
+qemu_mutex_unlock(&ms->src_page_req_mutex);
+
+return 0;
+}
+
+/*
* ram_find_and_save_block: Finds a page to send and sends it to f
*
* Returns: The number of bytes written.
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 5e0d30d..5bc01d5 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -102,6 +102,18 @@ MigrationIncomingState
*migration_incoming_get_current(void);
MigrationIncomingState *migration_incoming_state_init(QEMUFile *f);
void migration_incoming_state_destroy(void);
+/*
+ * An outstanding page request, on the source, having been received
+ * and queued
+ */
+struct MigrationSrcPageRequest {
+RAMBlock *rb;
+hwaddroffset;
+hwaddrlen;
+
+QSIMPLEQ_ENTRY(MigrationSrcPageRequest) next_req;
+};
+
struct MigrationState
{
int64_t bandwidth_limit;
@@ -138,6 +150,12 @@ struct MigrationState
* of the postcopy phase
*/
unsigned long *sentmap;
+
+/* Queue of outstanding page requests from the destination */
+QemuMutex src_page_req_mutex;
+QSIMPLEQ_HEAD(src_page_requests, MigrationSrcPageRequest)
src_page_requests;
+/* The RAMBlock used in the last src_page_request */
+RAMBlock *last_req_rb;
};
void process_incoming_migration(QEMUFile *f);
@@ -273,4 +291,7 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t
block_offset,
ram_addr_t offset, size_t size,
int *bytes_sent);
+int ram_save_queue_pages(MigrationState *ms, const char *rbname,
+ ram_addr_t start, ram_addr_t len);
+
#endif
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 79f57c0..24c2207 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -8,6 +8,7 @@ typedef struct QEMUTimerListGroup QEMUTimerListGroup;
typedef struct QEMUFile QEMUFile;
typedef struct QEMUBH QEMUBH;
+typedef struct AdapterInfo AdapterInfo;
typedef struct AioContext AioContext;
typedef struct Visitor Visitor;
@@ -80,6 +81,6 @@ typedef struct FWCfgState FWCfgState;
typedef struct PcGuestInfo PcGuestInfo;
typedef struct PostcopyPMI PostcopyPMI;
typedef struct Range Range;
-typedef struct AdapterInfo AdapterInfo;
+typedef struct RAMBlock RAMBlock;
:(, another redefinition, 'RAMBlock' also defined in
'include/exec/cpu-all.h:314',
Am i miss something when compile qemu?
#endif /* QEMU_TYPEDEFS_H */
diff --git a/migration.c b/migration.c
index cfdaa52..63d7699 100644
--- a/migration.c
+++ b/migration.c
@@ -26,6 +26,8 @@
#include "qemu/thread.h"
#include "qmp-commands.h"
#include "trace.h"
+#include "exec/
Re: [Qemu-devel] [PATCH v4 36/47] Page request: Process incoming page request
* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> > typedef struct Visitor Visitor;
> >@@ -80,6 +81,6 @@ typedef struct FWCfgState FWCfgState;
> > typedef struct PcGuestInfo PcGuestInfo;
> > typedef struct PostcopyPMI PostcopyPMI;
> > typedef struct Range Range;
> >-typedef struct AdapterInfo AdapterInfo;
> >+typedef struct RAMBlock RAMBlock;
> >
>
> :(, another redefinition, 'RAMBlock' also defined in
> 'include/exec/cpu-all.h:314',
> Am i miss something when compile qemu?
Interesting; I'm not seeing that problem at all (gcc 4.8.3-7)
What compiler and flags are you using?
Dave
>
> > #endif /* QEMU_TYPEDEFS_H */
> >diff --git a/migration.c b/migration.c
> >index cfdaa52..63d7699 100644
> >--- a/migration.c
> >+++ b/migration.c
> >@@ -26,6 +26,8 @@
> > #include "qemu/thread.h"
> > #include "qmp-commands.h"
> > #include "trace.h"
> >+#include "exec/memory.h"
> >+#include "exec/address-spaces.h"
> >
> > //#define DEBUG_MIGRATION
> >
> >@@ -504,6 +506,15 @@ static void migrate_fd_cleanup(void *opaque)
> >
> > migrate_fd_cleanup_src_rp(s);
> >
> >+/* This queue generally should be empty - but in the case of a failed
> >+ * migration might have some droppings in.
> >+ */
> >+struct MigrationSrcPageRequest *mspr, *next_mspr;
> >+QSIMPLEQ_FOREACH_SAFE(mspr, &s->src_page_requests, next_req, next_mspr)
> >{
> >+QSIMPLEQ_REMOVE_HEAD(&s->src_page_requests, next_req);
> >+g_free(mspr);
> >+}
> >+
> > if (s->file) {
> > trace_migrate_fd_cleanup();
> > qemu_mutex_unlock_iothread();
> >@@ -610,6 +621,9 @@ MigrationState *migrate_init(const MigrationParams
> >*params)
> > s->state = MIG_STATE_SETUP;
> > trace_migrate_set_state(MIG_STATE_SETUP);
> >
> >+qemu_mutex_init(&s->src_page_req_mutex);
> >+QSIMPLEQ_INIT(&s->src_page_requests);
> >+
> > s->total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> > return s;
> > }
> >@@ -823,7 +837,25 @@ static void source_return_path_bad(MigrationState *s)
> > static void migrate_handle_rp_reqpages(MigrationState *ms, const char*
> > rbname,
> > ram_addr_t start, ram_addr_t len)
> > {
> >-DPRINTF("migrate_handle_rp_reqpages: at %zx for len %zx", start, len);
> >+DPRINTF("migrate_handle_rp_reqpages: in %s start %zx len %zx",
> >+rbname, start, len);
> >+
> >+/* Round everything up to our host page size */
> >+long our_host_ps = sysconf(_SC_PAGESIZE);
> >+if (start & (our_host_ps-1)) {
> >+long roundings = start & (our_host_ps-1);
> >+start -= roundings;
> >+len += roundings;
> >+}
> >+if (len & (our_host_ps-1)) {
> >+long roundings = len & (our_host_ps-1);
> >+len -= roundings;
> >+len += our_host_ps;
> >+}
> >+
> >+if (ram_save_queue_pages(ms, rbname, start, len)) {
> >+source_return_path_bad(ms);
> >+}
> > }
> >
> > /*
> >
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v4 36/47] Page request: Process incoming page request
Il 08/10/2014 09:49, Dr. David Alan Gilbert ha scritto: >> > :(, another redefinition, 'RAMBlock' also defined in >> > 'include/exec/cpu-all.h:314', >> > Am i miss something when compile qemu? > Interesting; I'm not seeing that problem at all (gcc 4.8.3-7) > > What compiler and flags are you using? I think it is visible with CentOS 6. Paolo
Re: [Qemu-devel] [PATCH v4 36/47] Page request: Process incoming page request
On 2014/10/8 15:49, Dr. David Alan Gilbert wrote:
* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
typedef struct Visitor Visitor;
@@ -80,6 +81,6 @@ typedef struct FWCfgState FWCfgState;
typedef struct PcGuestInfo PcGuestInfo;
typedef struct PostcopyPMI PostcopyPMI;
typedef struct Range Range;
-typedef struct AdapterInfo AdapterInfo;
+typedef struct RAMBlock RAMBlock;
:(, another redefinition, 'RAMBlock' also defined in
'include/exec/cpu-all.h:314',
Am i miss something when compile qemu?
Interesting; I'm not seeing that problem at all (gcc 4.8.3-7)
What compiler and flags are you using?
Dave
Hi Dave,
My compiler info:
gcc (SUSE Linux) 4.3.4
The configure info is:
#./configure --target-list=x86_64-softmmu --enable-debug --disable-gtk
...
CFLAGS-pthread -I/usr/include/glib-2.0
-I/usr/lib64/glib-2.0/include -g
QEMU_CFLAGS -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef
-Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common
-Wendif-labels -Wmissing-include-dirs -Wempty-body -Wnested-externs
-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers
-Wold-style-declaration -Wold-style-definition -Wtype-limits
-fstack-protector-all-I/usr/include/libpng12 -I/usr/include/pixman-1
LDFLAGS -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g
...
Maybe its gcc's limitation, but why this redefinition need? After i remove one,
it compiles successfully;)
Thanks,
zhanghailiang
#endif /* QEMU_TYPEDEFS_H */
diff --git a/migration.c b/migration.c
index cfdaa52..63d7699 100644
--- a/migration.c
+++ b/migration.c
@@ -26,6 +26,8 @@
#include "qemu/thread.h"
#include "qmp-commands.h"
#include "trace.h"
+#include "exec/memory.h"
+#include "exec/address-spaces.h"
//#define DEBUG_MIGRATION
@@ -504,6 +506,15 @@ static void migrate_fd_cleanup(void *opaque)
migrate_fd_cleanup_src_rp(s);
+/* This queue generally should be empty - but in the case of a failed
+ * migration might have some droppings in.
+ */
+struct MigrationSrcPageRequest *mspr, *next_mspr;
+QSIMPLEQ_FOREACH_SAFE(mspr, &s->src_page_requests, next_req, next_mspr) {
+QSIMPLEQ_REMOVE_HEAD(&s->src_page_requests, next_req);
+g_free(mspr);
+}
+
if (s->file) {
trace_migrate_fd_cleanup();
qemu_mutex_unlock_iothread();
@@ -610,6 +621,9 @@ MigrationState *migrate_init(const MigrationParams *params)
s->state = MIG_STATE_SETUP;
trace_migrate_set_state(MIG_STATE_SETUP);
+qemu_mutex_init(&s->src_page_req_mutex);
+QSIMPLEQ_INIT(&s->src_page_requests);
+
s->total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
return s;
}
@@ -823,7 +837,25 @@ static void source_return_path_bad(MigrationState *s)
static void migrate_handle_rp_reqpages(MigrationState *ms, const char* rbname,
ram_addr_t start, ram_addr_t len)
{
-DPRINTF("migrate_handle_rp_reqpages: at %zx for len %zx", start, len);
+DPRINTF("migrate_handle_rp_reqpages: in %s start %zx len %zx",
+rbname, start, len);
+
+/* Round everything up to our host page size */
+long our_host_ps = sysconf(_SC_PAGESIZE);
+if (start & (our_host_ps-1)) {
+long roundings = start & (our_host_ps-1);
+start -= roundings;
+len += roundings;
+}
+if (len & (our_host_ps-1)) {
+long roundings = len & (our_host_ps-1);
+len -= roundings;
+len += our_host_ps;
+}
+
+if (ram_save_queue_pages(ms, rbname, start, len)) {
+source_return_path_bad(ms);
+}
}
/*
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
.
Re: [Qemu-devel] [PATCH v4 36/47] Page request: Process incoming page request
* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> On 2014/10/8 15:49, Dr. David Alan Gilbert wrote:
> >* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> >
> >>> typedef struct Visitor Visitor;
> >>>@@ -80,6 +81,6 @@ typedef struct FWCfgState FWCfgState;
> >>> typedef struct PcGuestInfo PcGuestInfo;
> >>> typedef struct PostcopyPMI PostcopyPMI;
> >>> typedef struct Range Range;
> >>>-typedef struct AdapterInfo AdapterInfo;
> >>>+typedef struct RAMBlock RAMBlock;
> >>>
> >>
> >>:(, another redefinition, 'RAMBlock' also defined in
> >>'include/exec/cpu-all.h:314',
> >>Am i miss something when compile qemu?
> >
> >Interesting; I'm not seeing that problem at all (gcc 4.8.3-7)
> >
> >What compiler and flags are you using?
> >
> >Dave
> >
>
> Hi Dave,
>
> My compiler info:
> gcc (SUSE Linux) 4.3.4
>
> The configure info is:
> #./configure --target-list=x86_64-softmmu --enable-debug --disable-gtk
> ...
> CFLAGS-pthread -I/usr/include/glib-2.0
> -I/usr/lib64/glib-2.0/include -g
> QEMU_CFLAGS -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
> -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef
> -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common
> -Wendif-labels -Wmissing-include-dirs -Wempty-body -Wnested-externs
> -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers
> -Wold-style-declaration -Wold-style-definition -Wtype-limits
> -fstack-protector-all-I/usr/include/libpng12 -I/usr/include/pixman-1
> LDFLAGS -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g
> ...
>
> Maybe its gcc's limitation, but why this redefinition need? After i remove
> one,
> it compiles successfully;)
OK, thanks. I'll clean them up.
Dave
>
> Thanks,
> zhanghailiang
>
> >
> >>
> >>> #endif /* QEMU_TYPEDEFS_H */
> >>>diff --git a/migration.c b/migration.c
> >>>index cfdaa52..63d7699 100644
> >>>--- a/migration.c
> >>>+++ b/migration.c
> >>>@@ -26,6 +26,8 @@
> >>> #include "qemu/thread.h"
> >>> #include "qmp-commands.h"
> >>> #include "trace.h"
> >>>+#include "exec/memory.h"
> >>>+#include "exec/address-spaces.h"
> >>>
> >>> //#define DEBUG_MIGRATION
> >>>
> >>>@@ -504,6 +506,15 @@ static void migrate_fd_cleanup(void *opaque)
> >>>
> >>> migrate_fd_cleanup_src_rp(s);
> >>>
> >>>+/* This queue generally should be empty - but in the case of a failed
> >>>+ * migration might have some droppings in.
> >>>+ */
> >>>+struct MigrationSrcPageRequest *mspr, *next_mspr;
> >>>+QSIMPLEQ_FOREACH_SAFE(mspr, &s->src_page_requests, next_req,
> >>>next_mspr) {
> >>>+QSIMPLEQ_REMOVE_HEAD(&s->src_page_requests, next_req);
> >>>+g_free(mspr);
> >>>+}
> >>>+
> >>> if (s->file) {
> >>> trace_migrate_fd_cleanup();
> >>> qemu_mutex_unlock_iothread();
> >>>@@ -610,6 +621,9 @@ MigrationState *migrate_init(const MigrationParams
> >>>*params)
> >>> s->state = MIG_STATE_SETUP;
> >>> trace_migrate_set_state(MIG_STATE_SETUP);
> >>>
> >>>+qemu_mutex_init(&s->src_page_req_mutex);
> >>>+QSIMPLEQ_INIT(&s->src_page_requests);
> >>>+
> >>> s->total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> >>> return s;
> >>> }
> >>>@@ -823,7 +837,25 @@ static void source_return_path_bad(MigrationState *s)
> >>> static void migrate_handle_rp_reqpages(MigrationState *ms, const char*
> >>> rbname,
> >>> ram_addr_t start, ram_addr_t len)
> >>> {
> >>>-DPRINTF("migrate_handle_rp_reqpages: at %zx for len %zx", start, len);
> >>>+DPRINTF("migrate_handle_rp_reqpages: in %s start %zx len %zx",
> >>>+rbname, start, len);
> >>>+
> >>>+/* Round everything up to our host page size */
> >>>+long our_host_ps = sysconf(_SC_PAGESIZE);
> >>>+if (start & (our_host_ps-1)) {
> >>>+long roundings = start & (our_host_ps-1);
> >>>+start -= roundings;
> >>>+len += roundings;
> >>>+}
> >>>+if (len & (our_host_ps-1)) {
> >>>+long roundings = len & (our_host_ps-1);
> >>>+len -= roundings;
> >>>+len += our_host_ps;
> >>>+}
> >>>+
> >>>+if (ram_save_queue_pages(ms, rbname, start, len)) {
> >>>+source_return_path_bad(ms);
> >>>+}
> >>> }
> >>>
> >>> /*
> >>>
> >>
> >>
> >--
> >Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> >
> >.
> >
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v4 36/47] Page request: Process incoming page request
On Fri, Oct 03, 2014 at 06:47:42PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert"
>
> On receiving MIG_RPCOMM_REQPAGES look up the address and
> queue the page.
>
> Signed-off-by: Dr. David Alan Gilbert
> ---
> arch_init.c | 52
> +++
> include/migration/migration.h | 21 +
> include/qemu/typedefs.h | 3 ++-
> migration.c | 34 +++-
> 4 files changed, 108 insertions(+), 2 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 4a03171..72f9e17 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -660,6 +660,58 @@ static int ram_save_page(QEMUFile *f, RAMBlock* block,
> ram_addr_t offset,
> }
>
> /*
> + * Queue the pages for transmission, e.g. a request from postcopy destination
> + * ms: MigrationStatus in which the queue is held
> + * rbname: The RAMBlock the request is for - may be NULL (to mean reuse
> last)
> + * start: Offset from the start of the RAMBlock
> + * len: Length (in bytes) to send
> + * Return: 0 on success
> + */
> +int ram_save_queue_pages(MigrationState *ms, const char *rbname,
> + ram_addr_t start, ram_addr_t len)
> +{
> +RAMBlock *ramblock;
> +
> +if (!rbname) {
> +/* Reuse last RAMBlock */
> +ramblock = ms->last_req_rb;
> +
> +if (!ramblock) {
> +error_report("ram_save_queue_pages no previous block");
> +return -1;
This should be an assert() shouldn't it?
> +}
> +} else {
> +ramblock = ram_find_block(rbname);
> +
> +if (!ramblock) {
> +error_report("ram_save_queue_pages no block '%s'", rbname);
> +return -1;
> +}
And maybe this one too - I would have expected the rb names to have
already been validated on the source machine at this stage.
> +}
> +DPRINTF("ram_save_queue_pages: Block %s start %zx len %zx",
> +ramblock->idstr, start, len);
> +
> +if (start+len > ramblock->length) {
> +error_report("%s request overrun start=%zx len=%zx blocklen=%zx",
> + __func__, start, len, ramblock->length);
> +return -1;
> +}
> +
> +struct MigrationSrcPageRequest *new_entry =
> +g_malloc0(sizeof(struct MigrationSrcPageRequest));
> +new_entry->rb = ramblock;
> +new_entry->offset = start;
> +new_entry->len = len;
> +ms->last_req_rb = ramblock;
> +
> +qemu_mutex_lock(&ms->src_page_req_mutex);
> +QSIMPLEQ_INSERT_TAIL(&ms->src_page_requests, new_entry, next_req);
> +qemu_mutex_unlock(&ms->src_page_req_mutex);
> +
> +return 0;
> +}
> +
> +/*
> * ram_find_and_save_block: Finds a page to send and sends it to f
> *
> * Returns: The number of bytes written.
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 5e0d30d..5bc01d5 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -102,6 +102,18 @@ MigrationIncomingState
> *migration_incoming_get_current(void);
> MigrationIncomingState *migration_incoming_state_init(QEMUFile *f);
> void migration_incoming_state_destroy(void);
>
> +/*
> + * An outstanding page request, on the source, having been received
> + * and queued
> + */
> +struct MigrationSrcPageRequest {
> +RAMBlock *rb;
> +hwaddroffset;
> +hwaddrlen;
> +
> +QSIMPLEQ_ENTRY(MigrationSrcPageRequest) next_req;
> +};
> +
> struct MigrationState
> {
> int64_t bandwidth_limit;
> @@ -138,6 +150,12 @@ struct MigrationState
> * of the postcopy phase
> */
> unsigned long *sentmap;
> +
> +/* Queue of outstanding page requests from the destination */
> +QemuMutex src_page_req_mutex;
> +QSIMPLEQ_HEAD(src_page_requests, MigrationSrcPageRequest)
> src_page_requests;
> +/* The RAMBlock used in the last src_page_request */
> +RAMBlock *last_req_rb;
> };
>
> void process_incoming_migration(QEMUFile *f);
> @@ -273,4 +291,7 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t
> block_offset,
> ram_addr_t offset, size_t size,
> int *bytes_sent);
>
> +int ram_save_queue_pages(MigrationState *ms, const char *rbname,
> + ram_addr_t start, ram_addr_t len);
> +
> #endif
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index 79f57c0..24c2207 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -8,6 +8,7 @@ typedef struct QEMUTimerListGroup QEMUTimerListGroup;
> typedef struct QEMUFile QEMUFile;
> typedef struct QEMUBH QEMUBH;
>
> +typedef struct AdapterInfo AdapterInfo;
> typedef struct AioContext AioContext;
>
> typedef struct Visitor Visitor;
> @@ -80,6 +81,6 @@ typedef struct FWCfgState FWCfgState;
> typedef struct PcGuestInfo PcGuestInfo;
> typedef struct PostcopyPMI PostcopyPMI;
> typedef struct Range R
Re: [Qemu-devel] [PATCH v4 36/47] Page request: Process incoming page request
* David Gibson (da...@gibson.dropbear.id.au) wrote:
> On Fri, Oct 03, 2014 at 06:47:42PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert"
> >
> > On receiving MIG_RPCOMM_REQPAGES look up the address and
> > queue the page.
> >
> > Signed-off-by: Dr. David Alan Gilbert
> > ---
> > arch_init.c | 52
> > +++
> > include/migration/migration.h | 21 +
> > include/qemu/typedefs.h | 3 ++-
> > migration.c | 34 +++-
> > 4 files changed, 108 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch_init.c b/arch_init.c
> > index 4a03171..72f9e17 100644
> > --- a/arch_init.c
> > +++ b/arch_init.c
> > @@ -660,6 +660,58 @@ static int ram_save_page(QEMUFile *f, RAMBlock* block,
> > ram_addr_t offset,
> > }
> >
> > /*
> > + * Queue the pages for transmission, e.g. a request from postcopy
> > destination
> > + * ms: MigrationStatus in which the queue is held
> > + * rbname: The RAMBlock the request is for - may be NULL (to mean reuse
> > last)
> > + * start: Offset from the start of the RAMBlock
> > + * len: Length (in bytes) to send
> > + * Return: 0 on success
> > + */
> > +int ram_save_queue_pages(MigrationState *ms, const char *rbname,
> > + ram_addr_t start, ram_addr_t len)
> > +{
> > +RAMBlock *ramblock;
> > +
> > +if (!rbname) {
> > +/* Reuse last RAMBlock */
> > +ramblock = ms->last_req_rb;
> > +
> > +if (!ramblock) {
> > +error_report("ram_save_queue_pages no previous block");
> > +return -1;
>
> This should be an assert() shouldn't it?
>
> > +}
> > +} else {
> > +ramblock = ram_find_block(rbname);
> > +
> > +if (!ramblock) {
> > +error_report("ram_save_queue_pages no block '%s'", rbname);
> > +return -1;
> > +}
>
> And maybe this one too - I would have expected the rb names to have
> already been validated on the source machine at this stage.
No to both:
I've been trying to avoid asserts in migration outgoing code, because
they shouldn't affect the state of your guest, so there's no reason
to kill off what might still be a viable running guest just because
migration failed.
(On the incoming side it's a bit more OK since if you've not got
a full running VM anyway yet then there's not much to lose).
Dave
>
> > +}
> > +DPRINTF("ram_save_queue_pages: Block %s start %zx len %zx",
> > +ramblock->idstr, start, len);
> > +
> > +if (start+len > ramblock->length) {
> > +error_report("%s request overrun start=%zx len=%zx blocklen=%zx",
> > + __func__, start, len, ramblock->length);
> > +return -1;
> > +}
> > +
> > +struct MigrationSrcPageRequest *new_entry =
> > +g_malloc0(sizeof(struct MigrationSrcPageRequest));
> > +new_entry->rb = ramblock;
> > +new_entry->offset = start;
> > +new_entry->len = len;
> > +ms->last_req_rb = ramblock;
> > +
> > +qemu_mutex_lock(&ms->src_page_req_mutex);
> > +QSIMPLEQ_INSERT_TAIL(&ms->src_page_requests, new_entry, next_req);
> > +qemu_mutex_unlock(&ms->src_page_req_mutex);
> > +
> > +return 0;
> > +}
> > +
> > +/*
> > * ram_find_and_save_block: Finds a page to send and sends it to f
> > *
> > * Returns: The number of bytes written.
> > diff --git a/include/migration/migration.h b/include/migration/migration.h
> > index 5e0d30d..5bc01d5 100644
> > --- a/include/migration/migration.h
> > +++ b/include/migration/migration.h
> > @@ -102,6 +102,18 @@ MigrationIncomingState
> > *migration_incoming_get_current(void);
> > MigrationIncomingState *migration_incoming_state_init(QEMUFile *f);
> > void migration_incoming_state_destroy(void);
> >
> > +/*
> > + * An outstanding page request, on the source, having been received
> > + * and queued
> > + */
> > +struct MigrationSrcPageRequest {
> > +RAMBlock *rb;
> > +hwaddroffset;
> > +hwaddrlen;
> > +
> > +QSIMPLEQ_ENTRY(MigrationSrcPageRequest) next_req;
> > +};
> > +
> > struct MigrationState
> > {
> > int64_t bandwidth_limit;
> > @@ -138,6 +150,12 @@ struct MigrationState
> > * of the postcopy phase
> > */
> > unsigned long *sentmap;
> > +
> > +/* Queue of outstanding page requests from the destination */
> > +QemuMutex src_page_req_mutex;
> > +QSIMPLEQ_HEAD(src_page_requests, MigrationSrcPageRequest)
> > src_page_requests;
> > +/* The RAMBlock used in the last src_page_request */
> > +RAMBlock *last_req_rb;
> > };
> >
> > void process_incoming_migration(QEMUFile *f);
> > @@ -273,4 +291,7 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t
> > block_offset,
> > ram_addr_t offset, size_t size,
> > int *bytes_sent);
> >
> > +int ram_save_queue_pages(MigrationState *ms, const
Re: [Qemu-devel] [PATCH v4 36/47] Page request: Process incoming page request
On Mon, Nov 17, 2014 at 07:07:33PM +, Dr. David Alan Gilbert wrote:
> * David Gibson (da...@gibson.dropbear.id.au) wrote:
> > On Fri, Oct 03, 2014 at 06:47:42PM +0100, Dr. David Alan Gilbert (git)
> > wrote:
> > > From: "Dr. David Alan Gilbert"
> > >
> > > On receiving MIG_RPCOMM_REQPAGES look up the address and
> > > queue the page.
> > >
> > > Signed-off-by: Dr. David Alan Gilbert
> > > ---
> > > arch_init.c | 52
> > > +++
> > > include/migration/migration.h | 21 +
> > > include/qemu/typedefs.h | 3 ++-
> > > migration.c | 34 +++-
> > > 4 files changed, 108 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch_init.c b/arch_init.c
> > > index 4a03171..72f9e17 100644
> > > --- a/arch_init.c
> > > +++ b/arch_init.c
> > > @@ -660,6 +660,58 @@ static int ram_save_page(QEMUFile *f, RAMBlock*
> > > block, ram_addr_t offset,
> > > }
> > >
> > > /*
> > > + * Queue the pages for transmission, e.g. a request from postcopy
> > > destination
> > > + * ms: MigrationStatus in which the queue is held
> > > + * rbname: The RAMBlock the request is for - may be NULL (to mean
> > > reuse last)
> > > + * start: Offset from the start of the RAMBlock
> > > + * len: Length (in bytes) to send
> > > + * Return: 0 on success
> > > + */
> > > +int ram_save_queue_pages(MigrationState *ms, const char *rbname,
> > > + ram_addr_t start, ram_addr_t len)
> > > +{
> > > +RAMBlock *ramblock;
> > > +
> > > +if (!rbname) {
> > > +/* Reuse last RAMBlock */
> > > +ramblock = ms->last_req_rb;
> > > +
> > > +if (!ramblock) {
> > > +error_report("ram_save_queue_pages no previous block");
> > > +return -1;
> >
> > This should be an assert() shouldn't it?
> >
> > > +}
> > > +} else {
> > > +ramblock = ram_find_block(rbname);
> > > +
> > > +if (!ramblock) {
> > > +error_report("ram_save_queue_pages no block '%s'", rbname);
> > > +return -1;
> > > +}
> >
> > And maybe this one too - I would have expected the rb names to have
> > already been validated on the source machine at this stage.
>
> No to both:
> I've been trying to avoid asserts in migration outgoing code, because
> they shouldn't affect the state of your guest, so there's no reason
> to kill off what might still be a viable running guest just because
> migration failed.
Ah, ok, that makes sense. Maybe adding something to the error message
or a nearby comment indicating that if these happen it's certainly a
bug, not the result of some external problem?
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
pgpHMIr0c9EFO.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH v4 36/47] Page request: Process incoming page request
* David Gibson (da...@gibson.dropbear.id.au) wrote: > On Mon, Nov 17, 2014 at 07:07:33PM +, Dr. David Alan Gilbert wrote: > > > And maybe this one too - I would have expected the rb names to have > > > already been validated on the source machine at this stage. > > > > No to both: > > I've been trying to avoid asserts in migration outgoing code, because > > they shouldn't affect the state of your guest, so there's no reason > > to kill off what might still be a viable running guest just because > > migration failed. > > Ah, ok, that makes sense. Maybe adding something to the error message > or a nearby comment indicating that if these happen it's certainly a > bug, not the result of some external problem? Done. Dave > > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au| minimalist, thank you. NOT _the_ > _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
