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

[Brian Jackson <iggy@theiggy.com>]

  On 9/25/2010 3:27 AM, Blue Swirl wrote:
> On Fri, Sep 24, 2010 at 9:10 PM, Brian Jackson<iggy@theiggy.com>  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?


That's why I held off on some of my other patches. They had too many 
ifdef's even for me.


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


Okay, seems reasonable. I will make this change for the future.


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


This is the strategy I was looking at for some of the follow on patches. 
That's a little more involved though. Actual code changes that likely 
need to be tested a little more than my simple compile and see if it 
runs a guest without destroying the world. F.ex. the block drivers all 
do this, so they can be omitted very easily. I'll work on that to get 
rid of the ifdef's.


>>      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()?
>