Re: [Qemu-devel] [RFC 03/13] migration: Allow -incoming to work on file: urls

[Hailiang Zhang]

On 2016/1/12 4:02, Dr. David Alan Gilbert wrote:
> * zhanghailiang (address@hidden) wrote:
> > Usage:
> > -incoming file:/path/to/vm_statefile
> >
> > Signed-off-by: zhanghailiang <address@hidden>
> > Signed-off-by: Benoit Canet <address@hidden>
>
> This could again be split out of this series; however I have some comments.
>
> > ---
> > - Rebase on qemu 2.5
> > - Use qemu_strtol instead of strtol
> > ---
> >   include/migration/migration.h |  4 +++-
> >   migration/fd.c                | 28 +++++++++++++++++++++++++---
> >   migration/migration.c         |  4 +++-
> >   3 files changed, 31 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/migration/migration.h b/include/migration/migration.h
> > index bf4f8e9..3f372a5 100644
> > --- a/include/migration/migration.h
> > +++ b/include/migration/migration.h
> > @@ -191,7 +191,9 @@ void unix_start_incoming_migration(const char *path, Error 
> > **errp);
> >
> >   void unix_start_outgoing_migration(MigrationState *s, const char *path, Error 
> > **errp);
> >
> > -void fd_start_incoming_migration(const char *path, Error **errp);
> > +void fd_start_incoming_migration(const char *path, int fd, Error **errp);
> > +
> > +void file_start_incoming_migration(const char *filename, Error **errp);
> >
> >   void fd_start_outgoing_migration(MigrationState *s, const char *fdname,
> >                                    int outfd, Error **errp);
> > diff --git a/migration/fd.c b/migration/fd.c
> > index b62161f..ac38256 100644
> > --- a/migration/fd.c
> > +++ b/migration/fd.c
> > @@ -81,14 +81,24 @@ static void fd_accept_incoming_migration(void *opaque)
> >       process_incoming_migration(f);
> >   }
> >
> > -void fd_start_incoming_migration(const char *infd, Error **errp)
> > +void fd_start_incoming_migration(const char *infd,  int fd, Error **errp)
> >   {
> > -    int fd;
> >       QEMUFile *f;
> > +    int err;
> > +    long in_fd;
> >
> >       DPRINTF("Attempting to start an incoming migration via fd\n");
> >
> > -    fd = strtol(infd, NULL, 0);
> > +    if (infd) {
> > +        err = qemu_strtol(infd, NULL, 0, &in_fd);
> > +        if (err < 0) {
> > +            error_setg_errno(errp, -err, "Failed to convert string '%s'"
> > +                            " to number", infd);
> > +            return;
> > +        }
> > +        fd = (int)in_fd;
> > +    }
> > +
> >       if (fd_is_socket(fd)) {
> >           f = qemu_fopen_socket(fd, "rb");
> >       } else {
>
> I think I'd prefer to see something like:
> void fd_start_incoming_migration_core(int fd, Error **errp)
>
> void fd_start_incoming_migration(const char *infd, Error **errp)
> {
>    qemu_strtol
>    fd_start_incoming_migration_core
> ...
> }

Hmm, good idea, it avoids changing the define of this function. I will fix it.

Thanks,
Hailiang

> (I've always done -incoming "exec:cat file"  but this is neater)
>
> Dave
>
> > @@ -101,3 +111,15 @@ void fd_start_incoming_migration(const char *infd, Error 
> > **errp)
> >
> >       qemu_set_fd_handler(fd, fd_accept_incoming_migration, NULL, f);
> >   }
> > +
> > +void file_start_incoming_migration(const char *filename, Error **errp)
> > +{
> > +    int fd;
> > +
> > +    fd = qemu_open(filename, O_RDONLY);
> > +    if (fd < 0) {
> > +        error_setg_errno(errp, errno, "Failed to open file:%s", filename);
> > +        return;
> > +    }
> > +    fd_start_incoming_migration(NULL, fd, NULL);
> > +}
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 3ec3b85..e54910d 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -314,7 +314,9 @@ void qemu_start_incoming_migration(const char *uri, Error 
> > **errp)
> >       } else if (strstart(uri, "unix:", &p)) {
> >           unix_start_incoming_migration(p, errp);
> >       } else if (strstart(uri, "fd:", &p)) {
> > -        fd_start_incoming_migration(p, errp);
> > +        fd_start_incoming_migration(p, -1, errp);
> > +    } else if (strstart(uri, "file:", &p)) {
> > +        file_start_incoming_migration(p, errp);
> >   #endif
> >       } else {
> >           error_setg(errp, "unknown migration protocol: %s", uri);
> > --
> > 1.8.3.1
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK
>
> .