[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 4/6] qemu: Refactor do{Tunnel, Native}Migrate functions



On Mon, Aug 15, 2011 at 17:48:03 +0800, Daniel Veillard wrote:
> On Mon, Aug 15, 2011 at 09:58:14AM +0200, Jiri Denemark wrote:
> > The core of these two functions is very similar and most of it is even
> > exactly the same. Factor out the core functionality into a separate
> > function to remove code duplication and make further changes easier.
> > ---
> >  src/qemu/qemu_migration.c |  499 ++++++++++++++++++++++-----------------------
> >  1 files changed, 239 insertions(+), 260 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 4d0e062..1cabbe0 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
> > @@ -1265,122 +1265,37 @@ cleanup:
...
> > +struct _qemuMigrationSpec {
> > +    enum qemuMigrationDestinationType destType;
> > +    union {
> > +        struct {
> > +            const char *name;
> > +            int port;
> > +        } host;
> > +
> > +        struct {
> > +            const char *file;
> > +            int sock;
> > +        } unics; /* this sucks but "unix" is a macro defined to 1 */
> 
>   stylistic: I would go for unx, unics is so reminiscent of Multics <sigh/>
> or unix_socket, that one should be clean.

OK, I kind of like this link to Multics :-) but I agree that unix_socket is
cleaner (though unix_socket.sock is not so nice as unix.sock would have been)
and I also removed the comment.

...
> > +qemuMigrationRun(struct qemud_driver *driver,
> > +                 virDomainObjPtr vm,
> > +                 const char *cookiein,
> > +                 int cookieinlen,
> > +                 char **cookieout,
> > +                 int *cookieoutlen,
> > +                 unsigned int flags,
> > +                 unsigned long resource,
> > +                 qemuMigrationSpecPtr spec)
> >  {
> > -    qemuDomainObjPrivatePtr priv = vm->privateData;
> > -    int client_sock = -1;
> > -    int qemu_sock = -1;
> > -    struct sockaddr_un sa_qemu, sa_client;
> > -    socklen_t addrlen;
> > -    int status;
> > -    unsigned long long transferred, remaining, total;
> > -    char *unixfile = NULL;
> > -    unsigned int background_flags = QEMU_MONITOR_MIGRATE_BACKGROUND;
> >      int ret = -1;
> > +    unsigned int migrate_flags = QEMU_MONITOR_MIGRATE_BACKGROUND;
> > +    qemuDomainObjPrivatePtr priv = vm->privateData;
> >      qemuMigrationCookiePtr mig = NULL;
> >      qemuMigrationIOThreadPtr iothread = NULL;
> > -    VIR_DEBUG("driver=%p, vm=%p, st=%p, cookiein=%s, cookieinlen=%d, "
> > -              "cookieout=%p, cookieoutlen=%p, flags=%lx, resource=%lu",
> > -              driver, vm, st, NULLSTR(cookiein), cookieinlen,
> > -              cookieout, cookieoutlen, flags, resource);
> 
>   I would still keep a VIR_DEBUG() even if there is one on the callers,
> because that code is new, not that simple, and I would go for increased
> tracability/verbosity in that case

OK, I added this additional VIR_DEBUG to qemuMigrationRun too. Note that the
existing do{Native,Tunnel}Migrate didn't lose their VIR_DEBUG calls.

Jirka


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]