| Message ID | 1485207141-1941-16-git-send-email-quintela@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* Juan Quintela (quintela@redhat.com) wrote: > We just send the address through the alternate channels and test that it > is ok. > > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > migration/ram.c | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/migration/ram.c b/migration/ram.c > index 4e530ea..95af694 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -432,8 +432,22 @@ static void *multifd_send_thread(void *opaque) > qemu_mutex_lock(¶ms->mutex); > while (!params->quit){ > if (params->pages.num) { > + int i; > + int num; > + > + num = params->pages.num; > params->pages.num = 0; > qemu_mutex_unlock(¶ms->mutex); > + > + for(i=0; i < num; i++) { > + if (qio_channel_write(params->c, > + (const char *)¶ms->pages.address[i], > + sizeof(uint8_t *), &error_abort) > + != sizeof(uint8_t*)) { > + /* Shuoudn't ever happen */ > + exit(-1); > + } Nope, need to find a way to cleanly find the migration; that might actually be tricky from one of these threads? > + } > qemu_mutex_lock(&multifd_send_mutex); > params->done = true; > qemu_cond_signal(&multifd_send_cond); > @@ -594,6 +608,7 @@ QemuCond multifd_recv_cond; > static void *multifd_recv_thread(void *opaque) > { > MultiFDRecvParams *params = opaque; > + uint8_t *recv_address; > char start; > > qio_channel_read(params->c, &start, 1, &error_abort); > @@ -605,8 +620,29 @@ static void *multifd_recv_thread(void *opaque) > qemu_mutex_lock(¶ms->mutex); > while (!params->quit){ > if (params->pages.num) { > + int i; > + int num; > + > + num = params->pages.num; > params->pages.num = 0; > qemu_mutex_unlock(¶ms->mutex); > + > + for(i = 0; i < num; i++) { > + if (qio_channel_read(params->c, > + (char *)&recv_address, > + sizeof(uint8_t*), &error_abort) > + != sizeof(uint8_t *)) { > + /* shouldn't ever happen */ > + exit(-1); > + } > + if (recv_address != params->pages.address[i]) { > + printf("We received %p what we were expecting %p (%d)\n", > + recv_address, > + params->pages.address[i], i); > + exit(-1); > + } > + } > + > qemu_mutex_lock(&multifd_recv_mutex); > params->done = true; > qemu_cond_signal(&multifd_recv_cond); > -- > 2.9.3 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > * Juan Quintela (quintela@redhat.com) wrote: >> We just send the address through the alternate channels and test that it >> is ok. >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> --- >> migration/ram.c | 36 ++++++++++++++++++++++++++++++++++++ >> 1 file changed, 36 insertions(+) >> >> diff --git a/migration/ram.c b/migration/ram.c >> index 4e530ea..95af694 100644 >> --- a/migration/ram.c >> +++ b/migration/ram.c >> @@ -432,8 +432,22 @@ static void *multifd_send_thread(void *opaque) >> qemu_mutex_lock(¶ms->mutex); >> while (!params->quit){ >> if (params->pages.num) { >> + int i; >> + int num; >> + >> + num = params->pages.num; >> params->pages.num = 0; >> qemu_mutex_unlock(¶ms->mutex); >> + >> + for(i=0; i < num; i++) { >> + if (qio_channel_write(params->c, >> + (const char *)¶ms->pages.address[i], >> + sizeof(uint8_t *), &error_abort) >> + != sizeof(uint8_t*)) { >> + /* Shuoudn't ever happen */ >> + exit(-1); >> + } > > Nope, need to find a way to cleanly find the migration; that > might actually be tricky from one of these threads? It is tricky, but the error handling is wrong in the callers of this already. Will try to improve it on next series, but the problem is already there.
* Juan Quintela (quintela@redhat.com) wrote: > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > > * Juan Quintela (quintela@redhat.com) wrote: > >> We just send the address through the alternate channels and test that it > >> is ok. > >> > >> Signed-off-by: Juan Quintela <quintela@redhat.com> > >> --- > >> migration/ram.c | 36 ++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 36 insertions(+) > >> > >> diff --git a/migration/ram.c b/migration/ram.c > >> index 4e530ea..95af694 100644 > >> --- a/migration/ram.c > >> +++ b/migration/ram.c > >> @@ -432,8 +432,22 @@ static void *multifd_send_thread(void *opaque) > >> qemu_mutex_lock(¶ms->mutex); > >> while (!params->quit){ > >> if (params->pages.num) { > >> + int i; > >> + int num; > >> + > >> + num = params->pages.num; > >> params->pages.num = 0; > >> qemu_mutex_unlock(¶ms->mutex); > >> + > >> + for(i=0; i < num; i++) { > >> + if (qio_channel_write(params->c, > >> + (const char *)¶ms->pages.address[i], > >> + sizeof(uint8_t *), &error_abort) > >> + != sizeof(uint8_t*)) { > >> + /* Shuoudn't ever happen */ > >> + exit(-1); > >> + } > > > > Nope, need to find a way to cleanly find the migration; that > > might actually be tricky from one of these threads? > > It is tricky, but the error handling is wrong in the callers of this > already. Will try to improve it on next series, but the problem is > already there. Well we should never kill the source because of a failed migration; especially just due to a fairly boring IO error. You could just exit the thread and return a marker value that gets delivered to the pthread_join. There is qemu_file_set_error() that you could call - although you'd have to figure out thread safety. (Which might be as simple as cmpxchg the error values and only change the error value if it was previously 0). Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Mon, Jan 23, 2017 at 10:32:19PM +0100, Juan Quintela wrote: > We just send the address through the alternate channels and test that it > is ok. > > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > migration/ram.c | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/migration/ram.c b/migration/ram.c > index 4e530ea..95af694 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -432,8 +432,22 @@ static void *multifd_send_thread(void *opaque) > qemu_mutex_lock(¶ms->mutex); > while (!params->quit){ > if (params->pages.num) { > + int i; > + int num; > + > + num = params->pages.num; Is this likely to be a small or a large value ? ..... > params->pages.num = 0; > qemu_mutex_unlock(¶ms->mutex); > + > + for(i=0; i < num; i++) { > + if (qio_channel_write(params->c, > + (const char *)¶ms->pages.address[i], > + sizeof(uint8_t *), &error_abort) > + != sizeof(uint8_t*)) { > + /* Shuoudn't ever happen */ > + exit(-1); > + } > + } If 'num' is large,then you would be better populating an iovec and using qio_channel_writev() rather than sending one uint8_t * at a time. > @@ -605,8 +620,29 @@ static void *multifd_recv_thread(void *opaque) > qemu_mutex_lock(¶ms->mutex); > while (!params->quit){ > if (params->pages.num) { > + int i; > + int num; > + > + num = params->pages.num; > params->pages.num = 0; > qemu_mutex_unlock(¶ms->mutex); > + > + for(i = 0; i < num; i++) { > + if (qio_channel_read(params->c, > + (char *)&recv_address, > + sizeof(uint8_t*), &error_abort) > + != sizeof(uint8_t *)) { > + /* shouldn't ever happen */ > + exit(-1); > + } > + if (recv_address != params->pages.address[i]) { > + printf("We received %p what we were expecting %p (%d)\n", > + recv_address, > + params->pages.address[i], i); > + exit(-1); > + } > + } Same comment as above. Regards, Daniel
diff --git a/migration/ram.c b/migration/ram.c index 4e530ea..95af694 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -432,8 +432,22 @@ static void *multifd_send_thread(void *opaque) qemu_mutex_lock(¶ms->mutex); while (!params->quit){ if (params->pages.num) { + int i; + int num; + + num = params->pages.num; params->pages.num = 0; qemu_mutex_unlock(¶ms->mutex); + + for(i=0; i < num; i++) { + if (qio_channel_write(params->c, + (const char *)¶ms->pages.address[i], + sizeof(uint8_t *), &error_abort) + != sizeof(uint8_t*)) { + /* Shuoudn't ever happen */ + exit(-1); + } + } qemu_mutex_lock(&multifd_send_mutex); params->done = true; qemu_cond_signal(&multifd_send_cond); @@ -594,6 +608,7 @@ QemuCond multifd_recv_cond; static void *multifd_recv_thread(void *opaque) { MultiFDRecvParams *params = opaque; + uint8_t *recv_address; char start; qio_channel_read(params->c, &start, 1, &error_abort); @@ -605,8 +620,29 @@ static void *multifd_recv_thread(void *opaque) qemu_mutex_lock(¶ms->mutex); while (!params->quit){ if (params->pages.num) { + int i; + int num; + + num = params->pages.num; params->pages.num = 0; qemu_mutex_unlock(¶ms->mutex); + + for(i = 0; i < num; i++) { + if (qio_channel_read(params->c, + (char *)&recv_address, + sizeof(uint8_t*), &error_abort) + != sizeof(uint8_t *)) { + /* shouldn't ever happen */ + exit(-1); + } + if (recv_address != params->pages.address[i]) { + printf("We received %p what we were expecting %p (%d)\n", + recv_address, + params->pages.address[i], i); + exit(-1); + } + } + qemu_mutex_lock(&multifd_recv_mutex); params->done = true; qemu_cond_signal(&multifd_recv_cond);
We just send the address through the alternate channels and test that it is ok. Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/ram.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)