Re: [Qemu-devel] [PATCH] Some tweaks to make some features only built-in when necessary

[Blue Swirl]

On Fri, Sep 24, 2010 at 9:10 PM, Brian Jackson <address@hidden> wrote:
> In trying to make the qemu binary size smaller, I've come across some things
> that can be left out of the binary without affecting the binary working. I've
> got more patches in the pipeline but the more I try to take out, the more
> invasive the patch. These are pretty simple to get started.
>
> Binary savings total for these patches is 606K.

I've mixed feelings about this, don't we want less #ifdeffery instead of more?

> ---
>  Makefile.objs |   31 +++++++++++++++++++++++++------
>  configure     |   20 ++++++++++++++++++++
>  migration.c   |   12 ++++++++++++
>  net.c         |    4 ++++
>  4 files changed, 61 insertions(+), 6 deletions(-)
>
> diff --git a/Makefile.objs b/Makefile.objs
> index dad4593..0c74477 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -12,9 +12,24 @@ block-obj-y += nbd.o block.o aio.o aes.o osdep.o 
> qemu-config.o
>  block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
>  block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
>
> -block-nested-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o 
> vpc.o vvfat.o
> -block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
> -block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
> +block-nested-$(CONFIG_BLK_RAW) += raw.o
> +block-nested-$(CONFIG_BLK_QCOW) += cow.o qcow.o
> +block-nested-$(CONFIG_BLK_VDI) += vdi.o
> +block-nested-$(CONFIG_BLK_VMDK) += vmdk.o
> +block-nested-$(CONFIG_BLK_CLOOP) += cloop.o
> +block-nested-$(CONFIG_BLK_DMG) += dmg.o
> +blcok-nested-$(CONFIG_BLK_BOCHS) += bochs.o
> +block-nested-$(CONFIG_BLK_VPC) += vpc.o
> +block-nested-$(CONFIG_BLK_VVFAT) += vvfat.o
> +block-nested-$(CONFIG_BLK_QCOW2) += qcow2.o qcow2-refcount.o qcow2-cluster.o 
> qcow2-snapshot.o
> +block-nested-$(CONFIG_BLK_PARALLELS) += parallels.o
> +block-nested-$(CONFIG_BLK_NBD) += nbd.o
> +block-nested-$(CONFIG_BLK_BLKDEBUG) += blkdebug.o
> +block-nested-$(CONFIG_BLK_SHEEPDOG) += sheepdog.o
> +block-nested-$(CONFIG_BLK_VERIFY) += blkverify.o
> +block-nested-$(CONFIG_WIN32) += raw-win32.o
> +block-nested-$(CONFIG_POSIX) += raw-posix.o
> +block-nested-$(CONFIG_CURL) += curl.o
>  block-nested-$(CONFIG_WIN32) += raw-win32.o
>  block-nested-$(CONFIG_POSIX) += raw-posix.o
>  block-nested-$(CONFIG_CURL) += curl.o
> @@ -23,8 +38,8 @@ block-obj-y +=  $(addprefix block/, $(block-nested-y))
>
>  net-obj-y = net.o
>  net-nested-y = queue.o checksum.o util.o
> -net-nested-y += socket.o
> -net-nested-y += dump.o
> +net-nested-$(CONFIG_NET_SOCKET) += socket.o
> +net-nested-$(CONFIG_NET_DUMP) += dump.o
>  net-nested-$(CONFIG_POSIX) += tap.o
>  net-nested-$(CONFIG_LINUX) += tap-linux.o
>  net-nested-$(CONFIG_WIN32) += tap-win32.o
> @@ -85,7 +100,11 @@ common-obj-y += qdev.o qdev-properties.o
>  common-obj-y += block-migration.o
>
>  common-obj-$(CONFIG_BRLAPI) += baum.o
> -common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o 
> migration-fd.o
> +ifdef CONFIG_POSIX

At least this can be avoided by moving the decision to configure...

> +common-obj-$(CONFIG_MIG_EXEC) += migration-exec.o
> +common-obj-$(CONFIG_MIG_UNIX) += migration-unix.o
> +common-obj-$(CONFIG_MIG_FD) += migration-fd.o
> +endif
>
>  audio-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
>  audio-obj-$(CONFIG_SDL) += sdlaudio.o
> diff --git a/configure b/configure
> index 3bfc5e9..681b678 100755
> --- a/configure
> +++ b/configure
> @@ -2439,6 +2439,26 @@ if test "$bluez" = "yes" ; then
>   echo "CONFIG_BLUEZ=y" >> $config_host_mak
>   echo "BLUEZ_CFLAGS=$bluez_cflags" >> $config_host_mak
>  fi
> +echo "CONFIG_BLK_RAW=y" >> $config_host_mak
> +echo "CONFIG_BLK_QCOW2=y" >> $config_host_mak
> +echo "CONFIG_BLK_VVFAT=y" >> $config_host_mak
> +echo "CONFIG_BLK_VDI=y" >> $config_host_mak
> +echo "CONFIG_BLK_DMG=y" >> $config_host_mak
> +echo "CONFIG_BLK_QCOW=y" >> $config_host_mak
> +echo "CONFIG_BLK_VMDK=y" >> $config_host_mak
> +echo "CONFIG_BLK_CLOOP=y" >> $config_host_mak
> +echo "CONFIG_BLK_BOCHS=y" >> $config_host_mak
> +echo "CONFIG_BLK_VPC=y" >> $config_host_mak
> +echo "CONFIG_BLK_PARALLELS=y" >> $config_host_mak
> +echo "CONFIG_BLK_NBD=y" >> $config_host_mak
> +echo "CONFIG_BLK_BLKDEBUG=y" >> $config_host_mak
> +echo "CONFIG_BLK_SHEEPDOG=y" >> $config_host_mak
> +echo "CONFIG_BLK_BLKDEBUG=y" >> $config_host_mak
> +echo "CONFIG_NET_DUMP=y" >> $config_host_mak
> +echo "CONFIG_NET_SOCKET=y" >> $config_host_mak

... by adding something like
if test "$posix" = "yes" ; then

> +echo "CONFIG_MIG_EXEC=y" >> $config_host_mak
> +echo "CONFIG_MIG_UNIX=y" >> $config_host_mak
> +echo "CONFIG_MIG_FD=y" >> $config_host_mak
>  if test "$xen" = "yes" ; then
>   echo "CONFIG_XEN=y" >> $config_host_mak
>  fi
> diff --git a/migration.c b/migration.c
> index 468d517..a22fa5f 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -44,13 +44,19 @@ int qemu_start_incoming_migration(const char *uri)
>     if (strstart(uri, "tcp:", &p))
>         ret = tcp_start_incoming_migration(p);
>  #if !defined(WIN32)
> +#ifdef CONFIG_MIG_EXEC
>     else if (strstart(uri, "exec:", &p))
>         ret =  exec_start_incoming_migration(p);
> +#endif
> +#ifdef CONFIG_MIG_UNIX
>     else if (strstart(uri, "unix:", &p))
>         ret = unix_start_incoming_migration(p);
> +#endif
> +#ifdef CONFIG_MIG_FD
>     else if (strstart(uri, "fd:", &p))
>         ret = fd_start_incoming_migration(p);
>  #endif
> +#endif

Perhaps there should be a way for protocols to register themselves and
their prefixes, so this part would not need to know about protocols.

>     else {
>         fprintf(stderr, "unknown migration protocol: %s\n", uri);
>         ret = -EPROTONOSUPPORT;
> @@ -92,16 +98,22 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject 
> **ret_data)
>         s = tcp_start_outgoing_migration(mon, p, max_throttle, detach,
>                                          blk, inc);
>  #if !defined(WIN32)
> +#ifdef CONFIG_MIG_EXEC
>     } else if (strstart(uri, "exec:", &p)) {
>         s = exec_start_outgoing_migration(mon, p, max_throttle, detach,
>                                           blk, inc);
> +#endif
> +#ifdef CONFIG_MIG_UNIX
>     } else if (strstart(uri, "unix:", &p)) {
>         s = unix_start_outgoing_migration(mon, p, max_throttle, detach,
>                                           blk, inc);
> +#endif
> +#ifdef CONFIG_MIG_FD
>     } else if (strstart(uri, "fd:", &p)) {
>         s = fd_start_outgoing_migration(mon, p, max_throttle, detach,
>                                         blk, inc);
>  #endif
> +#endif
>     } else {
>         monitor_printf(mon, "unknown migration protocol: %s\n", uri);
>         return -1;
> diff --git a/net.c b/net.c
> index 3d0fde7..a8612af 100644
> --- a/net.c
> +++ b/net.c
> @@ -988,6 +988,7 @@ static const struct {
>  #endif /* _WIN32 */
>             { /* end of list */ }
>         },
> +#ifdef CONFIG_NET_SOCKET

Could we use a similar approach as used for hw devices, device_init()?