| Message ID | 1297330258-20494-3-git-send-email-tamura.yoshiaki@lab.ntt.co.jp |
|---|---|
| State | New |
| Headers | show |
On 02/10/2011 10:30 AM, Yoshiaki Tamura wrote: > Currently FdMigrationState doesn't support read(), and this patch > introduces it to get response from the other side. > > Signed-off-by: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp> > Migration is unidirectional. Changing this is fundamental and not something to be done lightly. I thought we previously discussed using a protocol wrapper around the existing migration protocol? Regards, Anthony Liguori > --- > migration-tcp.c | 15 +++++++++++++++ > migration.c | 13 +++++++++++++ > migration.h | 3 +++ > 3 files changed, 31 insertions(+), 0 deletions(-) > > diff --git a/migration-tcp.c b/migration-tcp.c > index b55f419..55777c8 100644 > --- a/migration-tcp.c > +++ b/migration-tcp.c > @@ -39,6 +39,20 @@ static int socket_write(FdMigrationState *s, const void * buf, size_t size) > return send(s->fd, buf, size, 0); > } > > +static int socket_read(FdMigrationState *s, const void * buf, size_t size) > +{ > + ssize_t len; > + > + do { > + len = recv(s->fd, (void *)buf, size, 0); > + } while (len == -1&& socket_error() == EINTR); > + if (len == -1) { > + len = -socket_error(); > + } > + > + return len; > +} > + > static int tcp_close(FdMigrationState *s) > { > DPRINTF("tcp_close\n"); > @@ -94,6 +108,7 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon, > > s->get_error = socket_errno; > s->write = socket_write; > + s->read = socket_read; > s->close = tcp_close; > s->mig_state.cancel = migrate_fd_cancel; > s->mig_state.get_status = migrate_fd_get_status; > diff --git a/migration.c b/migration.c > index 3612572..f0df5fc 100644 > --- a/migration.c > +++ b/migration.c > @@ -340,6 +340,19 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size) > return ret; > } > > +int migrate_fd_get_buffer(void *opaque, uint8_t *data, int64_t pos, size_t size) > +{ > + FdMigrationState *s = opaque; > + int ret; > + > + ret = s->read(s, data, size); > + if (ret == -1) { > + ret = -(s->get_error(s)); > + } > + > + return ret; > +} > + > void migrate_fd_connect(FdMigrationState *s) > { > int ret; > diff --git a/migration.h b/migration.h > index 2170792..88a6987 100644 > --- a/migration.h > +++ b/migration.h > @@ -48,6 +48,7 @@ struct FdMigrationState > int (*get_error)(struct FdMigrationState*); > int (*close)(struct FdMigrationState*); > int (*write)(struct FdMigrationState*, const void *, size_t); > + int (*read)(struct FdMigrationState *, const void *, size_t); > void *opaque; > }; > > @@ -116,6 +117,8 @@ void migrate_fd_put_notify(void *opaque); > > ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size); > > +int migrate_fd_get_buffer(void *opaque, uint8_t *data, int64_t pos, size_t size); > + > void migrate_fd_connect(FdMigrationState *s); > > void migrate_fd_put_ready(void *opaque); >
2011/2/10 Anthony Liguori <anthony@codemonkey.ws>: > On 02/10/2011 10:30 AM, Yoshiaki Tamura wrote: >> >> Currently FdMigrationState doesn't support read(), and this patch >> introduces it to get response from the other side. >> >> Signed-off-by: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp> >> > > Migration is unidirectional. Changing this is fundamental and not something > to be done lightly. > > I thought we previously discussed using a protocol wrapper around the > existing migration protocol? AFAIR, I don't think we had that discussion before. I applied comments from Stefan though. If I missed the discussion, could you please give me the link? Thanks, Yoshi > > Regards, > > Anthony Liguori > >> --- >> migration-tcp.c | 15 +++++++++++++++ >> migration.c | 13 +++++++++++++ >> migration.h | 3 +++ >> 3 files changed, 31 insertions(+), 0 deletions(-) >> >> diff --git a/migration-tcp.c b/migration-tcp.c >> index b55f419..55777c8 100644 >> --- a/migration-tcp.c >> +++ b/migration-tcp.c >> @@ -39,6 +39,20 @@ static int socket_write(FdMigrationState *s, const void >> * buf, size_t size) >> return send(s->fd, buf, size, 0); >> } >> >> +static int socket_read(FdMigrationState *s, const void * buf, size_t >> size) >> +{ >> + ssize_t len; >> + >> + do { >> + len = recv(s->fd, (void *)buf, size, 0); >> + } while (len == -1&& socket_error() == EINTR); >> + if (len == -1) { >> + len = -socket_error(); >> + } >> + >> + return len; >> +} >> + >> static int tcp_close(FdMigrationState *s) >> { >> DPRINTF("tcp_close\n"); >> @@ -94,6 +108,7 @@ MigrationState *tcp_start_outgoing_migration(Monitor >> *mon, >> >> s->get_error = socket_errno; >> s->write = socket_write; >> + s->read = socket_read; >> s->close = tcp_close; >> s->mig_state.cancel = migrate_fd_cancel; >> s->mig_state.get_status = migrate_fd_get_status; >> diff --git a/migration.c b/migration.c >> index 3612572..f0df5fc 100644 >> --- a/migration.c >> +++ b/migration.c >> @@ -340,6 +340,19 @@ ssize_t migrate_fd_put_buffer(void *opaque, const >> void *data, size_t size) >> return ret; >> } >> >> +int migrate_fd_get_buffer(void *opaque, uint8_t *data, int64_t pos, >> size_t size) >> +{ >> + FdMigrationState *s = opaque; >> + int ret; >> + >> + ret = s->read(s, data, size); >> + if (ret == -1) { >> + ret = -(s->get_error(s)); >> + } >> + >> + return ret; >> +} >> + >> void migrate_fd_connect(FdMigrationState *s) >> { >> int ret; >> diff --git a/migration.h b/migration.h >> index 2170792..88a6987 100644 >> --- a/migration.h >> +++ b/migration.h >> @@ -48,6 +48,7 @@ struct FdMigrationState >> int (*get_error)(struct FdMigrationState*); >> int (*close)(struct FdMigrationState*); >> int (*write)(struct FdMigrationState*, const void *, size_t); >> + int (*read)(struct FdMigrationState *, const void *, size_t); >> void *opaque; >> }; >> >> @@ -116,6 +117,8 @@ void migrate_fd_put_notify(void *opaque); >> >> ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t >> size); >> >> +int migrate_fd_get_buffer(void *opaque, uint8_t *data, int64_t pos, >> size_t size); >> + >> void migrate_fd_connect(FdMigrationState *s); >> >> void migrate_fd_put_ready(void *opaque); >> > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On Thu, Feb 10, 2011 at 10:54:01AM +0100, Anthony Liguori wrote: > On 02/10/2011 10:30 AM, Yoshiaki Tamura wrote: > >Currently FdMigrationState doesn't support read(), and this patch > >introduces it to get response from the other side. > > > >Signed-off-by: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp> > > Migration is unidirectional. Changing this is fundamental and not > something to be done lightly. Making it bi-directional might break libvirt's save/restore to file support which uses migration, passing a unidirectional FD for the file. It could also break libvirt's secure tunnelled migration support which is currently only expecting to have data sent in one direction on the socket. Daniel
2011/2/10 Daniel P. Berrange <berrange@redhat.com>: > On Thu, Feb 10, 2011 at 10:54:01AM +0100, Anthony Liguori wrote: >> On 02/10/2011 10:30 AM, Yoshiaki Tamura wrote: >> >Currently FdMigrationState doesn't support read(), and this patch >> >introduces it to get response from the other side. >> > >> >Signed-off-by: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp> >> >> Migration is unidirectional. Changing this is fundamental and not >> something to be done lightly. > > Making it bi-directional might break libvirt's save/restore > to file support which uses migration, passing a unidirectional > FD for the file. It could also break libvirt's secure tunnelled > migration support which is currently only expecting to have > data sent in one direction on the socket. Hi Daniel, IIUC, this patch isn't something to make existing live migration bi-directional. Just opens up a way for Kemari to use it. Do you think it's dangerous for libvirt still? Thanks, Yoshi > > Daniel > -- > |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On Thu, Feb 10, 2011 at 07:23:33PM +0900, Yoshiaki Tamura wrote: > 2011/2/10 Daniel P. Berrange <berrange@redhat.com>: > > On Thu, Feb 10, 2011 at 10:54:01AM +0100, Anthony Liguori wrote: > >> On 02/10/2011 10:30 AM, Yoshiaki Tamura wrote: > >> >Currently FdMigrationState doesn't support read(), and this patch > >> >introduces it to get response from the other side. > >> > > >> >Signed-off-by: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp> > >> > >> Migration is unidirectional. Changing this is fundamental and not > >> something to be done lightly. > > > > Making it bi-directional might break libvirt's save/restore > > to file support which uses migration, passing a unidirectional > > FD for the file. It could also break libvirt's secure tunnelled > > migration support which is currently only expecting to have > > data sent in one direction on the socket. > > Hi Daniel, > > IIUC, this patch isn't something to make existing live migration > bi-directional. Just opens up a way for Kemari to use it. Do > you think it's dangerous for libvirt still? The key is for it to be a no-op for any usage of the existing 'migrate' command. I had thought this was wiring up read into the event loop too, so it would be poll()ing for reads, but after re-reading I see this isn't the case here. Regards, Daniel
2011/2/10 Daniel P. Berrange <berrange@redhat.com>: > On Thu, Feb 10, 2011 at 07:23:33PM +0900, Yoshiaki Tamura wrote: >> 2011/2/10 Daniel P. Berrange <berrange@redhat.com>: >> > On Thu, Feb 10, 2011 at 10:54:01AM +0100, Anthony Liguori wrote: >> >> On 02/10/2011 10:30 AM, Yoshiaki Tamura wrote: >> >> >Currently FdMigrationState doesn't support read(), and this patch >> >> >introduces it to get response from the other side. >> >> > >> >> >Signed-off-by: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp> >> >> >> >> Migration is unidirectional. Changing this is fundamental and not >> >> something to be done lightly. >> > >> > Making it bi-directional might break libvirt's save/restore >> > to file support which uses migration, passing a unidirectional >> > FD for the file. It could also break libvirt's secure tunnelled >> > migration support which is currently only expecting to have >> > data sent in one direction on the socket. >> >> Hi Daniel, >> >> IIUC, this patch isn't something to make existing live migration >> bi-directional. Just opens up a way for Kemari to use it. Do >> you think it's dangerous for libvirt still? > > The key is for it to be a no-op for any usage of the existing > 'migrate' command. I had thought this was wiring up read into > the event loop too, so it would be poll()ing for reads, but > after re-reading I see this isn't the case here. It's a no-op for existing migration related code. Anthony, did you have the same concern? Yoshi > > Regards, > Daniel > -- > |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
diff --git a/migration-tcp.c b/migration-tcp.c index b55f419..55777c8 100644 --- a/migration-tcp.c +++ b/migration-tcp.c @@ -39,6 +39,20 @@ static int socket_write(FdMigrationState *s, const void * buf, size_t size) return send(s->fd, buf, size, 0); } +static int socket_read(FdMigrationState *s, const void * buf, size_t size) +{ + ssize_t len; + + do { + len = recv(s->fd, (void *)buf, size, 0); + } while (len == -1 && socket_error() == EINTR); + if (len == -1) { + len = -socket_error(); + } + + return len; +} + static int tcp_close(FdMigrationState *s) { DPRINTF("tcp_close\n"); @@ -94,6 +108,7 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon, s->get_error = socket_errno; s->write = socket_write; + s->read = socket_read; s->close = tcp_close; s->mig_state.cancel = migrate_fd_cancel; s->mig_state.get_status = migrate_fd_get_status; diff --git a/migration.c b/migration.c index 3612572..f0df5fc 100644 --- a/migration.c +++ b/migration.c @@ -340,6 +340,19 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size) return ret; } +int migrate_fd_get_buffer(void *opaque, uint8_t *data, int64_t pos, size_t size) +{ + FdMigrationState *s = opaque; + int ret; + + ret = s->read(s, data, size); + if (ret == -1) { + ret = -(s->get_error(s)); + } + + return ret; +} + void migrate_fd_connect(FdMigrationState *s) { int ret; diff --git a/migration.h b/migration.h index 2170792..88a6987 100644 --- a/migration.h +++ b/migration.h @@ -48,6 +48,7 @@ struct FdMigrationState int (*get_error)(struct FdMigrationState*); int (*close)(struct FdMigrationState*); int (*write)(struct FdMigrationState*, const void *, size_t); + int (*read)(struct FdMigrationState *, const void *, size_t); void *opaque; }; @@ -116,6 +117,8 @@ void migrate_fd_put_notify(void *opaque); ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size); +int migrate_fd_get_buffer(void *opaque, uint8_t *data, int64_t pos, size_t size); + void migrate_fd_connect(FdMigrationState *s); void migrate_fd_put_ready(void *opaque);
Currently FdMigrationState doesn't support read(), and this patch introduces it to get response from the other side. Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> --- migration-tcp.c | 15 +++++++++++++++ migration.c | 13 +++++++++++++ migration.h | 3 +++ 3 files changed, 31 insertions(+), 0 deletions(-)