Hi Paolo, (Please ignore the previous mail that did not include "qemu-devel")
Thanks for your review and suggestions. I'll fix this patch accordingly and please see my replies below. best regards, Houcheng Lin 2015-09-15 17:41 GMT+08:00 Paolo Bonzini <pbonz...@redhat.com>: > This is okay and can be done unconditionally (introduce a new > qemu_getdtablesize function that is defined in util/oslib-posix.c). Will fix it. > > >> - sigtimewait(): call __rt_sigtimewait() instead. >> - lockf(): not see this feature in android, directly return -1. >> - shm_open(): not see this feature in android, directly return -1. > > This is not okay. Please fix your libc instead. I'll modify the bionic C library to support these functions and feedback to google's AOSP project. But the android kernel does not support shmem, so I will prevent compile the ivshmem.c when in android config. The config for ivshmem in pci.mak will be: CONFIG_IVSHMEM=$(call land, $(call lnot,$(CONFIG_ANDROID)),$(CONFIG_KVM)) > For sys/io.h, we can just remove the inclusion. It is not necessary. > > scsi/sg.h should be exported by the Linux kernel, so that we can use > scripts/update-linux-headers.sh to copy it from QEMU. I've sent a Linux > kernel patch and CCed you. It's better to put headers on kernel user headers. Thanks. > > You should instead disable the guest agent on your configure command line. > Okay. > > If you have CONFIG_ANDROID, you do not need -DANDROID. > Okay. >> + LIBS="-lglib-2.0 -lgthread-2.0 -lz -lpixman-1 -lintl -liconv -lc $LIBS" >> + libs_qga="-lglib-2.0 -lgthread-2.0 -lz -lpixman-1 -lintl -liconv -lc" > > This should not be necessary, QEMU uses pkg-config. > >> +fi >> if [ "$bsd" = "yes" ] ; then >> if [ "$darwin" != "yes" ] ; then >> bsd_user="yes" >> @@ -1736,7 +1749,14 @@ fi >> # pkg-config probe >> >> if ! has "$pkg_config_exe"; then >> - error_exit "pkg-config binary '$pkg_config_exe' not found" >> + case $cross_prefix in >> + *android*) >> + pkg_config_exe=scripts/android-pkg-config > > Neither should this. Your cross-compilation environment is not > correctly set up if you do not have a pkg-config executable. If you > want to use a wrapper, you can specify it with the PKG_CONFIG > environment variable. But it need not be maintained in the QEMU > repository, because QEMU assumes a complete cross-compilation environment. I'll use wrapper in next release and specify with environment variable. Later, I may generate pkg-config data file while building library and install it into cross-compilation environment. > >> + ;; >> + *) >> + error_exit "pkg-config binary '$pkg_config_exe' not found" >> + ;; >> + esac >> fi >> >> ########################################## >> @@ -3764,7 +3784,7 @@ elif compile_prog "" "$pthread_lib -lrt" ; then >> fi >> >> if test "$darwin" != "yes" -a "$mingw32" != "yes" -a "$solaris" != yes -a \ >> - "$aix" != "yes" -a "$haiku" != "yes" ; then >> + "$aix" != "yes" -a "$haiku" != "yes" -a "$android" != "yes" ; then >> libs_softmmu="-lutil $libs_softmmu" >> fi >> >> @@ -4709,6 +4729,10 @@ if test "$linux" = "yes" ; then >> echo "CONFIG_LINUX=y" >> $config_host_mak >> fi >> >> +if test "$android" = "yes" ; then >> + echo "CONFIG_ANDROID=y" >> $config_host_mak >> +fi >> + >> if test "$darwin" = "yes" ; then >> echo "CONFIG_DARWIN=y" >> $config_host_mak >> fi >> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h >> index ab3c876..1ba22be 100644 >> --- a/include/qemu/osdep.h >> +++ b/include/qemu/osdep.h >> @@ -59,6 +59,10 @@ >> #define WEXITSTATUS(x) (x) >> #endif >> >> +#ifdef ANDROID >> + #include "sysemu/os-android.h" >> +#endif >> + >> #ifdef _WIN32 >> #include "sysemu/os-win32.h" >> #endif >> diff --git a/include/sysemu/os-android.h b/include/sysemu/os-android.h >> new file mode 100644 >> index 0000000..7f73777 >> --- /dev/null >> +++ b/include/sysemu/os-android.h >> @@ -0,0 +1,35 @@ >> +#ifndef QEMU_OS_ANDROID_H >> +#define QEMU_OS_ANDROID_H >> + >> +#include <sys/statvfs.h> >> + >> +/* >> + * For include the basename prototyping in android. >> + */ >> +#include <libgen.h> > > These two includes can be added to sysemu/os-posix.h. > >> +extern int __rt_sigtimedwait(const sigset_t *uthese, siginfo_t *uinfo, \ >> + const struct timespec *uts, size_t sigsetsize); >> + >> +int getdtablesize(void); >> +int lockf(int fd, int cmd, off_t len); >> +int sigtimedwait(const sigset_t *set, siginfo_t *info, \ >> + const struct timespec *ts); >> +int shm_open(const char *name, int oflag, mode_t mode); >> +char *a_ptsname(int fd); >> + >> +#define F_TLOCK 0 >> +#define IOV_MAX 256 > > libc should really be fixed for all of these (except getdtablesize and > a_ptsname). The way you're working around it is introducing subtle > bugs, for example the pidfile is _not_ locked. Okay, I will fix it. >> +#define I_LOOK (__SID | 4) /* Retrieve the name of the module just >> below >> + the STREAM head and place it in a >> character >> + string. */ > > These are not necessary. Okay. >> +#ifdef ANDROID >> +#undef PAGE_SIZE >> +#endif > > What is the definition of PAGE_SIZE on Android? Can we do > > #ifdef PAGE_SIZE > QEMU_BUILD_BUG_ON(PAGE_SIZE != TARGET_PAGE_SIZE); > #else > #define PAGE_SIZE TARGET_PAGE_SIZE > #endif > > ? Yes, we can use this approach. In android, PAGE_SIZE is defined to 4096 and this code can detect definition mismatch. > See above, this should not be part of QEMU. The reason is that QEMU > developers do not test Android builds, and the script would rot. > Instead, you should document how to prepare a complete environment to > cross-compile QEMU for Android. I'll write a document to describe steps to build QEMU for Android. But some library need minor modification to build for Android. Maybe I shall also put my modified library onto github. > > The ioctl(sfd, I_PUSH, "ptem") is not needed on Android. > > In addition, you can add a > > if (name) > strcpy(name, slave); > > and inside qemu_openpty_row add Android to the "#if defined(__OpenBSD__) > || defined(__DragonFly__)" case. This avoids the need to introduce > a_openpty. Okay, I'll use the strcpy approach to avoid a_openpty. > > The biggest problem is the workarounds you have added for pkg-config. > Everything else is easy to fix. I look forward to reviewing the next > version! Many thanks for your comments and I'll do the next version. > > Paolo > -- Best regards, Houcheng Lin -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
