[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