Re: [Qemu-devel] [PATCH 1/9] Move QEMU OS dependant library functions to OS specific files

[Blue Swirl]

On Fri, Oct 15, 2010 at 2:05 PM,  <address@hidden> wrote:
> From: Jes Sorensen <address@hidden>
>
> This moves library functions used by both QEMU and the QEMU tools,
> such as qemu-img, qemu-nbd etc. from osdep.c to os-{posix,win32}-lib.c

$ find . \( -name '*posix*.c' -o -name '*win32*.c' \) -print
./os-win32.c
./block/raw-posix.c
./block/raw-win32.c
./posix-aio-compat.c
./os-posix.c
./net/tap-win32.c

Please use something like osdep-{posix,win32}.c or oslib-{posix,win32}.c.

>
> In addition it introduces oslib-obj.y to the Makefile set to be
> included by the various targets, instead of relying on these library
> functions magically getting included via block-obj-y.
>
> Signed-off-by: Jes Sorensen <address@hidden>
> ---
>  Makefile       |    6 ++--
>  Makefile.objs  |    9 ++++++-
>  os-posix-lib.c |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  os-win32-lib.c |   71 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  osdep.c        |   70 -------------------------------------------------------
>  5 files changed, 147 insertions(+), 74 deletions(-)
>  create mode 100644 os-posix-lib.c
>  create mode 100644 os-win32-lib.c
>
> diff --git a/Makefile b/Makefile
> index 252c817..0b3751d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -129,11 +129,11 @@ version-obj-$(CONFIG_WIN32) += version.o
>  qemu-img.o: qemu-img-cmds.h
>  qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o: $(GENERATED_HEADERS)
>
> -qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(trace-obj-y) 
> $(block-obj-y) $(qobject-obj-y) $(version-obj-y)
> +qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(oslib-obj-y) 
> $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y)
>
> -qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(trace-obj-y) 
> $(block-obj-y) $(qobject-obj-y) $(version-obj-y)
> +qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(oslib-obj-y) 
> $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y)
>
> -qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(trace-obj-y) 
> $(block-obj-y) $(qobject-obj-y) $(version-obj-y)
> +qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(oslib-obj-y) 
> $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y)
>
>  qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
>        $(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@,"  GEN   $@")
> diff --git a/Makefile.objs b/Makefile.objs
> index 816194a..4281ae8 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -5,10 +5,16 @@ qobject-obj-y += qjson.o json-lexer.o json-streamer.o 
> json-parser.o
>  qobject-obj-y += qerror.o
>
>  #######################################################################
> +# os-obj-y is code depending on the OS (win32 vs posix)

The comment, variable name and file names are also all different.

> +oslib-obj-y = osdep.o
> +oslib-obj-$(CONFIG_WIN32) += os-win32-lib.o
> +oslib-obj-$(CONFIG_POSIX) += os-posix-lib.o
> +
> +#######################################################################
>  # block-obj-y is code used by both qemu system emulation and qemu-img
>
>  block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
> -block-obj-y += nbd.o block.o aio.o aes.o osdep.o qemu-config.o
> +block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o
>  block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
>  block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
>
> @@ -50,6 +56,7 @@ common-obj-y += $(net-obj-y)
>  common-obj-y += $(qobject-obj-y)
>  common-obj-$(CONFIG_LINUX) += $(fsdev-obj-$(CONFIG_LINUX))
>  common-obj-y += readline.o console.o cursor.o async.o qemu-error.o
> +common-obj-y += $(oslib-obj-y)
>  common-obj-$(CONFIG_WIN32) += os-win32.o
>  common-obj-$(CONFIG_POSIX) += os-posix.o
>
> diff --git a/os-posix-lib.c b/os-posix-lib.c
> new file mode 100644
> index 0000000..83e5101
> --- /dev/null
> +++ b/os-posix-lib.c
> @@ -0,0 +1,65 @@
> +/*
> + * os-posix-lib.c
> + *
> + * Copyright (c) 2003-2008 Fabrice Bellard
> + * Copyright (c) 2010 Red Hat, Inc.
> + *
> + * QEMU library functions on POSIX which are shared between QEMU and
> + * the QEMU tools.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "config-host.h"
> +#include "sysemu.h"
> +#include "trace.h"
> +#include "net/slirp.h"
> +#include "qemu-options.h"

I don't think all of these are really needed, please trim.

> +
> +void *qemu_memalign(size_t alignment, size_t size)
> +{
> +    void *ptr;
> +#if defined(_POSIX_C_SOURCE) && !defined(__sun__)
> +    int ret;
> +    ret = posix_memalign(&ptr, alignment, size);
> +    if (ret != 0) {
> +        fprintf(stderr, "Failed to allocate %zu B: %s\n",
> +                size, strerror(ret));
> +        abort();
> +    }
> +#elif defined(CONFIG_BSD)
> +    ptr = oom_check(valloc(size));
> +#else
> +    ptr = oom_check(memalign(alignment, size));
> +#endif
> +    trace_qemu_memalign(alignment, size, ptr);
> +    return ptr;
> +}
> +
> +/* alloc shared memory pages */
> +void *qemu_vmalloc(size_t size)
> +{
> +    return qemu_memalign(getpagesize(), size);
> +}
> +
> +void qemu_vfree(void *ptr)
> +{
> +    trace_qemu_vfree(ptr);
> +    free(ptr);
> +}
> diff --git a/os-win32-lib.c b/os-win32-lib.c
> new file mode 100644
> index 0000000..80e713a
> --- /dev/null
> +++ b/os-win32-lib.c
> @@ -0,0 +1,71 @@
> +/*
> + * os-win32.c
> + *
> + * Copyright (c) 2003-2008 Fabrice Bellard
> + * Copyright (c) 2010 Red Hat, Inc.
> + *
> + * QEMU library functions for win32 which are shared between QEMU and
> + * the QEMU tools.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +#include <windows.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <signal.h>
> +#include <time.h>
> +#include <errno.h>
> +#include <sys/time.h>
> +#include "config-host.h"
> +#include "sysemu.h"
> +#include "trace.h"
> +#include "qemu-options.h"

Ditto.