[libvirt] [PATCH 1/2] qemu: Split out various utility functions to qemu_util.c

[Peter Krempa]

On 01/03/13 18:37, Daniel P. Berrange wrote:
> On Thu, Jan 03, 2013 at 05:45:49PM +0100, Peter Krempa wrote:
>> ---
>>   po/POTFILES.in         |   1 +
>>   src/Makefile.am        |   1 +
>>   src/qemu/qemu_driver.c | 492 ++-----------------------------------------------
>>   src/qemu/qemu_util.c   | 486 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   src/qemu/qemu_util.h   | 111 +++++++++++
>>   5 files changed, 611 insertions(+), 480 deletions(-)
>>   create mode 100644 src/qemu/qemu_util.c
>>   create mode 100644 src/qemu/qemu_util.h
>

...

>
> All these should go in qemu_domain.{c,h}
>

Fair enough.

>
>> +int qemuOpenFile(virQEMUDriverPtr driver,
>> +                 const char *path,
>> +                 int oflags,
>> +                 bool *needUnlink,
>> +                 bool *bypassSecurityDriver);
>
> qemu_conf.c

Hm, qemu_conf.c, okay it's used to open files honoring the config ...

>
>> +int qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver,
>> +                                      virDomainObjPtr vm,
>> +                                      virCgroupPtr cgroup,
>> +                                      virDomainDiskDefPtr disk,
>> +                                      const char *file,
>> +                                      qemuDomainDiskChainMode mode);
>
> qemu_domain.h
>
>
>> +int qemuDomainSaveMemory(virQEMUDriverPtr driver,
>> +                         virDomainObjPtr vm,
>> +                         const char *path,
>> +                         const char *domXML,
>> +                         int compressed,
>> +                         bool was_running,
>> +                         unsigned int flags,
>> +                         enum qemuDomainAsyncJob asyncJob);
>
> Not convinced this needs to move at all.

This one needs to be included into qemu_snapshot.c that is created in 
2/2 of this series. All functions split out in this patch were chosen to 
allow that.

>
>> +const char *qemuCompressProgramName(int compress);
>
> qemu_domain.h
>
>
> In summary, I don't think we should create a qemu_util.{c,h} file - any
> file named util just ends up as a garbage dumping ground for code that
> should be better placed elsewhere. See also util/util.h which should be
> split up.

Well, as you can see it will ultimately end up somewhere. Unfortunately 
it this case everything tends to end up in qemu_driver.c.

>
> Daniel
>

Peter