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

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.

> +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