* [PATCH 01/16] include: move qemu_*_exec_dir() to cutils
2022-05-04 17:30 [PATCH 00/16] Misc cleanups marcandre.lureau
@ 2022-05-04 17:30 ` marcandre.lureau
2022-05-04 17:30 ` [PATCH 02/16] util/win32: simplify qemu_get_local_state_dir() marcandre.lureau
` (15 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: marcandre.lureau @ 2022-05-04 17:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Stefan Weil, Qiuhao Li,
Laurent Vivier, Stefan Hajnoczi, Hanna Reitz, Kevin Wolf,
Markus Armbruster, qemu-block, Konstantin Kostiuk, Bandan Das,
Michael Roth, Darren Kenny, Alexander Bulekov,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The function is required by get_relocated_path() (already in cutils),
and used by qemu-ga and may be generally useful.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/qemu/cutils.h | 7 ++
include/qemu/osdep.h | 8 --
qemu-io.c | 1 +
storage-daemon/qemu-storage-daemon.c | 1 +
tests/qtest/fuzz/fuzz.c | 1 +
util/cutils.c | 108 +++++++++++++++++++++++++++
util/oslib-posix.c | 81 --------------------
util/oslib-win32.c | 36 ---------
8 files changed, 118 insertions(+), 125 deletions(-)
diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index 5c6572d44422..40e10e19a7ed 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -193,6 +193,13 @@ int uleb128_decode_small(const uint8_t *in, uint32_t *n);
*/
int qemu_pstrcmp0(const char **str1, const char **str2);
+/* Find program directory, and save it for later usage with
+ * qemu_get_exec_dir().
+ * Try OS specific API first, if not working, parse from argv0. */
+void qemu_init_exec_dir(const char *argv0);
+
+/* Get the saved exec dir. */
+const char *qemu_get_exec_dir(void);
/**
* get_relocated_path:
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 1c1e7eca9898..67cc4654166b 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -557,14 +557,6 @@ void qemu_set_cloexec(int fd);
*/
char *qemu_get_local_state_dir(void);
-/* Find program directory, and save it for later usage with
- * qemu_get_exec_dir().
- * Try OS specific API first, if not working, parse from argv0. */
-void qemu_init_exec_dir(const char *argv0);
-
-/* Get the saved exec dir. */
-const char *qemu_get_exec_dir(void);
-
/**
* qemu_getauxval:
* @type: the auxiliary vector key to lookup
diff --git a/qemu-io.c b/qemu-io.c
index d70d3dd4fde5..2bd7bfb65073 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -16,6 +16,7 @@
#endif
#include "qemu/help-texts.h"
+#include "qemu/cutils.h"
#include "qapi/error.h"
#include "qemu-io.h"
#include "qemu/error-report.h"
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index 9b8b17f52e48..c104817cdddc 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -44,6 +44,7 @@
#include "qemu/help-texts.h"
#include "qemu-version.h"
+#include "qemu/cutils.h"
#include "qemu/config-file.h"
#include "qemu/error-report.h"
#include "qemu/help_option.h"
diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
index a7a5e14fa3bc..0ad4ba9e94dc 100644
--- a/tests/qtest/fuzz/fuzz.c
+++ b/tests/qtest/fuzz/fuzz.c
@@ -15,6 +15,7 @@
#include <wordexp.h>
+#include "qemu/cutils.h"
#include "qemu/datadir.h"
#include "sysemu/sysemu.h"
#include "sysemu/qtest.h"
diff --git a/util/cutils.c b/util/cutils.c
index b2777210e7da..6cc7cc8cde99 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -931,6 +931,114 @@ static inline const char *next_component(const char *dir, int *p_len)
return dir;
}
+static const char *exec_dir;
+
+void qemu_init_exec_dir(const char *argv0)
+{
+#ifdef G_OS_WIN32
+ char *p;
+ char buf[MAX_PATH];
+ DWORD len;
+
+ if (exec_dir) {
+ return;
+ }
+
+ len = GetModuleFileName(NULL, buf, sizeof(buf) - 1);
+ if (len == 0) {
+ return;
+ }
+
+ buf[len] = 0;
+ p = buf + len - 1;
+ while (p != buf && *p != '\\') {
+ p--;
+ }
+ *p = 0;
+ if (access(buf, R_OK) == 0) {
+ exec_dir = g_strdup(buf);
+ } else {
+ exec_dir = CONFIG_BINDIR;
+ }
+#else
+ char *p = NULL;
+ char buf[PATH_MAX];
+
+ if (exec_dir) {
+ return;
+ }
+
+#if defined(__linux__)
+ {
+ int len;
+ len = readlink("/proc/self/exe", buf, sizeof(buf) - 1);
+ if (len > 0) {
+ buf[len] = 0;
+ p = buf;
+ }
+ }
+#elif defined(__FreeBSD__) \
+ || (defined(__NetBSD__) && defined(KERN_PROC_PATHNAME))
+ {
+#if defined(__FreeBSD__)
+ static int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, -1};
+#else
+ static int mib[4] = {CTL_KERN, KERN_PROC_ARGS, -1, KERN_PROC_PATHNAME};
+#endif
+ size_t len = sizeof(buf) - 1;
+
+ *buf = '\0';
+ if (!sysctl(mib, ARRAY_SIZE(mib), buf, &len, NULL, 0) &&
+ *buf) {
+ buf[sizeof(buf) - 1] = '\0';
+ p = buf;
+ }
+ }
+#elif defined(__APPLE__)
+ {
+ char fpath[PATH_MAX];
+ uint32_t len = sizeof(fpath);
+ if (_NSGetExecutablePath(fpath, &len) == 0) {
+ p = realpath(fpath, buf);
+ if (!p) {
+ return;
+ }
+ }
+ }
+#elif defined(__HAIKU__)
+ {
+ image_info ii;
+ int32_t c = 0;
+
+ *buf = '\0';
+ while (get_next_image_info(0, &c, &ii) == B_OK) {
+ if (ii.type == B_APP_IMAGE) {
+ strncpy(buf, ii.name, sizeof(buf));
+ buf[sizeof(buf) - 1] = 0;
+ p = buf;
+ break;
+ }
+ }
+ }
+#endif
+ /* If we don't have any way of figuring out the actual executable
+ location then try argv[0]. */
+ if (!p && argv0) {
+ p = realpath(argv0, buf);
+ }
+ if (p) {
+ exec_dir = g_path_get_dirname(p);
+ } else {
+ exec_dir = CONFIG_BINDIR;
+ }
+#endif
+}
+
+const char *qemu_get_exec_dir(void)
+{
+ return exec_dir;
+}
+
char *get_relocated_path(const char *dir)
{
size_t prefix_len = strlen(CONFIG_PREFIX);
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 477990f39baf..7ba4472760ba 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -283,87 +283,6 @@ void qemu_set_tty_echo(int fd, bool echo)
tcsetattr(fd, TCSANOW, &tty);
}
-static const char *exec_dir;
-
-void qemu_init_exec_dir(const char *argv0)
-{
- char *p = NULL;
- char buf[PATH_MAX];
-
- if (exec_dir) {
- return;
- }
-
-#if defined(__linux__)
- {
- int len;
- len = readlink("/proc/self/exe", buf, sizeof(buf) - 1);
- if (len > 0) {
- buf[len] = 0;
- p = buf;
- }
- }
-#elif defined(__FreeBSD__) \
- || (defined(__NetBSD__) && defined(KERN_PROC_PATHNAME))
- {
-#if defined(__FreeBSD__)
- static int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, -1};
-#else
- static int mib[4] = {CTL_KERN, KERN_PROC_ARGS, -1, KERN_PROC_PATHNAME};
-#endif
- size_t len = sizeof(buf) - 1;
-
- *buf = '\0';
- if (!sysctl(mib, ARRAY_SIZE(mib), buf, &len, NULL, 0) &&
- *buf) {
- buf[sizeof(buf) - 1] = '\0';
- p = buf;
- }
- }
-#elif defined(__APPLE__)
- {
- char fpath[PATH_MAX];
- uint32_t len = sizeof(fpath);
- if (_NSGetExecutablePath(fpath, &len) == 0) {
- p = realpath(fpath, buf);
- if (!p) {
- return;
- }
- }
- }
-#elif defined(__HAIKU__)
- {
- image_info ii;
- int32_t c = 0;
-
- *buf = '\0';
- while (get_next_image_info(0, &c, &ii) == B_OK) {
- if (ii.type == B_APP_IMAGE) {
- strncpy(buf, ii.name, sizeof(buf));
- buf[sizeof(buf) - 1] = 0;
- p = buf;
- break;
- }
- }
- }
-#endif
- /* If we don't have any way of figuring out the actual executable
- location then try argv[0]. */
- if (!p && argv0) {
- p = realpath(argv0, buf);
- }
- if (p) {
- exec_dir = g_path_get_dirname(p);
- } else {
- exec_dir = CONFIG_BINDIR;
- }
-}
-
-const char *qemu_get_exec_dir(void)
-{
- return exec_dir;
-}
-
#ifdef CONFIG_LINUX
static void sigbus_handler(int signal, siginfo_t *siginfo, void *ctx)
#else /* CONFIG_LINUX */
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index dafef4f15733..6c818749d2b9 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -269,42 +269,6 @@ void qemu_set_tty_echo(int fd, bool echo)
}
}
-static const char *exec_dir;
-
-void qemu_init_exec_dir(const char *argv0)
-{
-
- char *p;
- char buf[MAX_PATH];
- DWORD len;
-
- if (exec_dir) {
- return;
- }
-
- len = GetModuleFileName(NULL, buf, sizeof(buf) - 1);
- if (len == 0) {
- return;
- }
-
- buf[len] = 0;
- p = buf + len - 1;
- while (p != buf && *p != '\\') {
- p--;
- }
- *p = 0;
- if (access(buf, R_OK) == 0) {
- exec_dir = g_strdup(buf);
- } else {
- exec_dir = CONFIG_BINDIR;
- }
-}
-
-const char *qemu_get_exec_dir(void)
-{
- return exec_dir;
-}
-
int getpagesize(void)
{
SYSTEM_INFO system_info;
--
2.36.0.44.g0f828332d5ac
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 02/16] util/win32: simplify qemu_get_local_state_dir()
2022-05-04 17:30 [PATCH 00/16] Misc cleanups marcandre.lureau
2022-05-04 17:30 ` [PATCH 01/16] include: move qemu_*_exec_dir() to cutils marcandre.lureau
@ 2022-05-04 17:30 ` marcandre.lureau
2022-05-04 18:07 ` Stefan Weil via
2022-05-04 17:30 ` [PATCH 03/16] tests: make libqmp buildable for win32 marcandre.lureau
` (14 subsequent siblings)
16 siblings, 1 reply; 24+ messages in thread
From: marcandre.lureau @ 2022-05-04 17:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Stefan Weil, Qiuhao Li,
Laurent Vivier, Stefan Hajnoczi, Hanna Reitz, Kevin Wolf,
Markus Armbruster, qemu-block, Konstantin Kostiuk, Bandan Das,
Michael Roth, Darren Kenny, Alexander Bulekov,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
SHGetFolderPath() is a deprecated API:
https://docs.microsoft.com/en-us/windows/win32/api/shlobj_core/nf-shlobj_core-shgetfolderpatha
It is a wrapper for SHGetKnownFolderPath() and CSIDL_COMMON_PATH is
mapped to FOLDERID_ProgramData:
https://docs.microsoft.com/en-us/windows/win32/shell/csidl
g_get_system_data_dirs() is a suitable replacement, as it will have
FOLDERID_ProgramData in the returned list. However, it follows the XDG
Base Directory Specification, if `XDG_DATA_DIRS` is defined, it will be
returned instead.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
util/oslib-win32.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 6c818749d2b9..5723d3eb4c5a 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -40,9 +40,6 @@
#include "qemu/error-report.h"
#include <malloc.h>
-/* this must come after including "trace.h" */
-#include <shlobj.h>
-
static int get_allocation_granularity(void)
{
SYSTEM_INFO system_info;
@@ -237,17 +234,11 @@ int qemu_get_thread_id(void)
char *
qemu_get_local_state_dir(void)
{
- HRESULT result;
- char base_path[MAX_PATH+1] = "";
+ const char * const *data_dirs = g_get_system_data_dirs();
- result = SHGetFolderPath(NULL, CSIDL_COMMON_APPDATA, NULL,
- /* SHGFP_TYPE_CURRENT */ 0, base_path);
- if (result != S_OK) {
- /* misconfigured environment */
- g_critical("CSIDL_COMMON_APPDATA unavailable: %ld", (long)result);
- abort();
- }
- return g_strdup(base_path);
+ g_assert(data_dirs && data_dirs[0]);
+
+ return g_strdup(data_dirs[0]);
}
void qemu_set_tty_echo(int fd, bool echo)
--
2.36.0.44.g0f828332d5ac
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 02/16] util/win32: simplify qemu_get_local_state_dir()
2022-05-04 17:30 ` [PATCH 02/16] util/win32: simplify qemu_get_local_state_dir() marcandre.lureau
@ 2022-05-04 18:07 ` Stefan Weil via
0 siblings, 0 replies; 24+ messages in thread
From: Stefan Weil via @ 2022-05-04 18:07 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Qiuhao Li, Laurent Vivier,
Stefan Hajnoczi, Hanna Reitz, Kevin Wolf, Markus Armbruster,
qemu-block, Konstantin Kostiuk, Bandan Das, Michael Roth,
Darren Kenny, Alexander Bulekov
[-- Attachment #1.1.1: Type: text/plain, Size: 907 bytes --]
Am 04.05.22 um 19:30 schrieb marcandre.lureau@redhat.com:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> SHGetFolderPath() is a deprecated API:
> https://docs.microsoft.com/en-us/windows/win32/api/shlobj_core/nf-shlobj_core-shgetfolderpatha
>
> It is a wrapper for SHGetKnownFolderPath() and CSIDL_COMMON_PATH is
> mapped to FOLDERID_ProgramData:
> https://docs.microsoft.com/en-us/windows/win32/shell/csidl
>
> g_get_system_data_dirs() is a suitable replacement, as it will have
> FOLDERID_ProgramData in the returned list. However, it follows the XDG
> Base Directory Specification, if `XDG_DATA_DIRS` is defined, it will be
> returned instead.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> util/oslib-win32.c | 17 ++++-------------
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
Reviewed-by: Stefan Weil <sw@weilnetz.de>
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 16043 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 03/16] tests: make libqmp buildable for win32
2022-05-04 17:30 [PATCH 00/16] Misc cleanups marcandre.lureau
2022-05-04 17:30 ` [PATCH 01/16] include: move qemu_*_exec_dir() to cutils marcandre.lureau
2022-05-04 17:30 ` [PATCH 02/16] util/win32: simplify qemu_get_local_state_dir() marcandre.lureau
@ 2022-05-04 17:30 ` marcandre.lureau
2022-05-04 18:18 ` Thomas Huth
2022-05-04 17:30 ` [PATCH 04/16] compiler.h: add QEMU_{BEGIN, END}_IGNORE_INITIALIZER_OVERRIDES marcandre.lureau
` (13 subsequent siblings)
16 siblings, 1 reply; 24+ messages in thread
From: marcandre.lureau @ 2022-05-04 17:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Stefan Weil, Qiuhao Li,
Laurent Vivier, Stefan Hajnoczi, Hanna Reitz, Kevin Wolf,
Markus Armbruster, qemu-block, Konstantin Kostiuk, Bandan Das,
Michael Roth, Darren Kenny, Alexander Bulekov,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/qtest/libqmp.h | 2 ++
tests/qtest/libqmp.c | 35 +++++++++++++++++++++++++++++------
2 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/tests/qtest/libqmp.h b/tests/qtest/libqmp.h
index 94aa97328a17..772f18b73ba3 100644
--- a/tests/qtest/libqmp.h
+++ b/tests/qtest/libqmp.h
@@ -20,8 +20,10 @@
#include "qapi/qmp/qdict.h"
QDict *qmp_fd_receive(int fd);
+#ifndef G_OS_WIN32
void qmp_fd_vsend_fds(int fd, int *fds, size_t fds_num,
const char *fmt, va_list ap) G_GNUC_PRINTF(4, 0);
+#endif
void qmp_fd_vsend(int fd, const char *fmt, va_list ap) G_GNUC_PRINTF(2, 0);
void qmp_fd_send(int fd, const char *fmt, ...) G_GNUC_PRINTF(2, 3);
void qmp_fd_send_raw(int fd, const char *fmt, ...) G_GNUC_PRINTF(2, 3);
diff --git a/tests/qtest/libqmp.c b/tests/qtest/libqmp.c
index 0358b8313dc4..93c9b31cd4ca 100644
--- a/tests/qtest/libqmp.c
+++ b/tests/qtest/libqmp.c
@@ -15,9 +15,13 @@
*/
#include "qemu/osdep.h"
-
#include "libqmp.h"
+#ifndef G_OS_WIN32
+#include <sys/socket.h>
+#endif
+
+#include "qemu/cutils.h"
#include "qapi/error.h"
#include "qapi/qmp/json-parser.h"
#include "qapi/qmp/qjson.h"
@@ -87,6 +91,7 @@ QDict *qmp_fd_receive(int fd)
return qmp.response;
}
+#ifndef G_OS_WIN32
/* Sends a message and file descriptors to the socket.
* It's needed for qmp-commands like getfd/add-fd */
static void socket_send_fds(int socket_fd, int *fds, size_t fds_num,
@@ -120,17 +125,23 @@ static void socket_send_fds(int socket_fd, int *fds, size_t fds_num,
} while (ret < 0 && errno == EINTR);
g_assert_cmpint(ret, >, 0);
}
+#endif
/**
* Allow users to send a message without waiting for the reply,
* in the case that they choose to discard all replies up until
* a particular EVENT is received.
*/
-void qmp_fd_vsend_fds(int fd, int *fds, size_t fds_num,
- const char *fmt, va_list ap)
+static void
+_qmp_fd_vsend_fds(int fd, int *fds, size_t fds_num,
+ const char *fmt, va_list ap)
{
QObject *qobj;
+#ifdef G_OS_WIN32
+ assert(fds_num == 0);
+#endif
+
/* Going through qobject ensures we escape strings properly */
qobj = qobject_from_vjsonf_nofail(fmt, ap);
@@ -148,10 +159,14 @@ void qmp_fd_vsend_fds(int fd, int *fds, size_t fds_num,
if (log) {
fprintf(stderr, "%s", str->str);
}
+
+#ifndef G_OS_WIN32
/* Send QMP request */
if (fds && fds_num > 0) {
socket_send_fds(fd, fds, fds_num, str->str, str->len);
- } else {
+ } else
+#endif
+ {
socket_send(fd, str->str, str->len);
}
@@ -160,15 +175,23 @@ void qmp_fd_vsend_fds(int fd, int *fds, size_t fds_num,
}
}
+#ifndef G_OS_WIN32
+void qmp_fd_vsend_fds(int fd, int *fds, size_t fds_num,
+ const char *fmt, va_list ap)
+{
+ _qmp_fd_vsend_fds(fd, fds, fds_num, fmt, ap);
+}
+#endif
+
void qmp_fd_vsend(int fd, const char *fmt, va_list ap)
{
- qmp_fd_vsend_fds(fd, NULL, 0, fmt, ap);
+ _qmp_fd_vsend_fds(fd, NULL, 0, fmt, ap);
}
QDict *qmp_fdv(int fd, const char *fmt, va_list ap)
{
- qmp_fd_vsend_fds(fd, NULL, 0, fmt, ap);
+ _qmp_fd_vsend_fds(fd, NULL, 0, fmt, ap);
return qmp_fd_receive(fd);
}
--
2.36.0.44.g0f828332d5ac
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 03/16] tests: make libqmp buildable for win32
2022-05-04 17:30 ` [PATCH 03/16] tests: make libqmp buildable for win32 marcandre.lureau
@ 2022-05-04 18:18 ` Thomas Huth
0 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2022-05-04 18:18 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel
Cc: Paolo Bonzini, Stefan Weil, Qiuhao Li, Laurent Vivier,
Stefan Hajnoczi, Hanna Reitz, Kevin Wolf, Markus Armbruster,
qemu-block, Konstantin Kostiuk, Bandan Das, Michael Roth,
Darren Kenny, Alexander Bulekov
On 04/05/2022 19.30, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> tests/qtest/libqmp.h | 2 ++
> tests/qtest/libqmp.c | 35 +++++++++++++++++++++++++++++------
> 2 files changed, 31 insertions(+), 6 deletions(-)
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 04/16] compiler.h: add QEMU_{BEGIN, END}_IGNORE_INITIALIZER_OVERRIDES
2022-05-04 17:30 [PATCH 00/16] Misc cleanups marcandre.lureau
` (2 preceding siblings ...)
2022-05-04 17:30 ` [PATCH 03/16] tests: make libqmp buildable for win32 marcandre.lureau
@ 2022-05-04 17:30 ` marcandre.lureau
2022-05-04 18:41 ` Peter Maydell
2022-05-04 17:30 ` [PATCH 05/16] qobject/json-lexer: disable -Winitializer-overrides warnings marcandre.lureau
` (12 subsequent siblings)
16 siblings, 1 reply; 24+ messages in thread
From: marcandre.lureau @ 2022-05-04 17:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Stefan Weil, Qiuhao Li,
Laurent Vivier, Stefan Hajnoczi, Hanna Reitz, Kevin Wolf,
Markus Armbruster, qemu-block, Konstantin Kostiuk, Bandan Das,
Michael Roth, Darren Kenny, Alexander Bulekov,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
clang has this default warning which QEMU codes triggers in many
situations. However, other projects in general may not want to disable
globally the warning but only in limited specific code blocks.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/qemu/compiler.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index f20a76e4a286..ea0797959641 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -184,4 +184,15 @@
#define QEMU_DISABLE_CFI
#endif
+#if defined (__clang__)
+#define QEMU_BEGIN_IGNORE_INITIALIZER_OVERRIDES \
+ _Pragma("clang diagnostic push") \
+ _Pragma("clang diagnostic ignored \"-Winitializer-overrides\"")
+#define QEMU_END_IGNORE_INITIALIZER_OVERRIDES \
+ _Pragma("clang diagnostic pop")
+#else
+#define QEMU_BEGIN_IGNORE_INITIALIZER_OVERRIDES
+#define QEMU_END_IGNORE_INITIALIZER_OVERRIDES
+#endif
+
#endif /* COMPILER_H */
--
2.36.0.44.g0f828332d5ac
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 04/16] compiler.h: add QEMU_{BEGIN, END}_IGNORE_INITIALIZER_OVERRIDES
2022-05-04 17:30 ` [PATCH 04/16] compiler.h: add QEMU_{BEGIN, END}_IGNORE_INITIALIZER_OVERRIDES marcandre.lureau
@ 2022-05-04 18:41 ` Peter Maydell
2022-05-05 6:55 ` Paolo Bonzini
0 siblings, 1 reply; 24+ messages in thread
From: Peter Maydell @ 2022-05-04 18:41 UTC (permalink / raw)
To: marcandre.lureau
Cc: qemu-devel, Paolo Bonzini, Thomas Huth, Stefan Weil, Qiuhao Li,
Laurent Vivier, Stefan Hajnoczi, Hanna Reitz, Kevin Wolf,
Markus Armbruster, qemu-block, Konstantin Kostiuk, Bandan Das,
Michael Roth, Darren Kenny, Alexander Bulekov
On Wed, 4 May 2022 at 18:36, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> clang has this default warning which QEMU codes triggers in many
> situations. However, other projects in general may not want to disable
> globally the warning but only in limited specific code blocks.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> +#if defined (__clang__)
> +#define QEMU_BEGIN_IGNORE_INITIALIZER_OVERRIDES \
> + _Pragma("clang diagnostic push") \
> + _Pragma("clang diagnostic ignored \"-Winitializer-overrides\"")
> +#define QEMU_END_IGNORE_INITIALIZER_OVERRIDES \
> + _Pragma("clang diagnostic pop")
> +#else
> +#define QEMU_BEGIN_IGNORE_INITIALIZER_OVERRIDES
> +#define QEMU_END_IGNORE_INITIALIZER_OVERRIDES
> +#endif
We use pragma diagnostic controls generally sparingly and
only when we need to briefly disable an otherwise widely
useful warning. In this case the clang warning is just
broken (because it doesn't correctly handle the array
range initializer extension we use), and so we turn off
the warning altogether in configure, so that we get the
behaviour we want everywhere in the source tree, not just
if we mark it up with special macros.
If other projects want to borrow bits of QEMU code then
they need to either (a) abide by our conventions for
what compiler warnings to enable or disable, or else
(b) fork the code and fiddle with their own copy.
I don't really want to see QEMU's source code get littered
with a pile of extra macros hiding diagnostic pragmas.
(If we stop passing -Wno-initializer-overrides to the
compiler then we set a bunch of new "built on gcc on the
developer's machine but fails to build on clang in the
CI jobs" traps for ourselves, and if we don't stop passing
that then the places that should be marked up with the
macros won't reliably be marked up.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 04/16] compiler.h: add QEMU_{BEGIN, END}_IGNORE_INITIALIZER_OVERRIDES
2022-05-04 18:41 ` Peter Maydell
@ 2022-05-05 6:55 ` Paolo Bonzini
0 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2022-05-05 6:55 UTC (permalink / raw)
To: Peter Maydell, marcandre.lureau
Cc: qemu-devel, Thomas Huth, Stefan Weil, Qiuhao Li, Laurent Vivier,
Stefan Hajnoczi, Hanna Reitz, Kevin Wolf, Markus Armbruster,
qemu-block, Konstantin Kostiuk, Bandan Das, Michael Roth,
Darren Kenny, Alexander Bulekov
>If other projects want to borrow bits of QEMU code then
>they need to either (a) abide by our conventions for
>what compiler warnings to enable or disable, or else
>(b) fork the code and fiddle with their own copy.
Agreed, it's not a huge deal if a single add_project_arguments call is duplicated across a couple meson subprojects.
Paolo
>
>I don't really want to see QEMU's source code get littered
>with a pile of extra macros hiding diagnostic pragmas.
>(If we stop passing -Wno-initializer-overrides to the
>compiler then we set a bunch of new "built on gcc on the
>developer's machine but fails to build on clang in the
>CI jobs" traps for ourselves, and if we don't stop passing
>that then the places that should be marked up with the
>macros won't reliably be marked up.)
>
>thanks
>-- PMM
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 05/16] qobject/json-lexer: disable -Winitializer-overrides warnings
2022-05-04 17:30 [PATCH 00/16] Misc cleanups marcandre.lureau
` (3 preceding siblings ...)
2022-05-04 17:30 ` [PATCH 04/16] compiler.h: add QEMU_{BEGIN, END}_IGNORE_INITIALIZER_OVERRIDES marcandre.lureau
@ 2022-05-04 17:30 ` marcandre.lureau
2022-05-04 17:30 ` [PATCH 06/16] include: adjust header guards after renaming marcandre.lureau
` (11 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: marcandre.lureau @ 2022-05-04 17:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Stefan Weil, Qiuhao Li,
Laurent Vivier, Stefan Hajnoczi, Hanna Reitz, Kevin Wolf,
Markus Armbruster, qemu-block, Konstantin Kostiuk, Bandan Das,
Michael Roth, Darren Kenny, Alexander Bulekov,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Silence the clang warning when building the code with default clang,
outside of QEMU.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qobject/json-lexer.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/qobject/json-lexer.c b/qobject/json-lexer.c
index 632320d72d5d..955ce0ff2a53 100644
--- a/qobject/json-lexer.c
+++ b/qobject/json-lexer.c
@@ -128,6 +128,8 @@ QEMU_BUILD_BUG_ON(IN_START_INTERP != IN_START + 1);
#define LOOKAHEAD 0x80
#define TERMINAL(state) [0 ... 0xFF] = ((state) | LOOKAHEAD)
+QEMU_BEGIN_IGNORE_INITIALIZER_OVERRIDES
+
static const uint8_t json_lexer[][256] = {
/* Relies on default initialization to IN_ERROR! */
@@ -261,6 +263,8 @@ static const uint8_t json_lexer[][256] = {
[IN_START_INTERP]['%'] = IN_INTERP,
};
+QEMU_END_IGNORE_INITIALIZER_OVERRIDES
+
static inline uint8_t next_state(JSONLexer *lexer, char ch, bool flush,
bool *char_consumed)
{
--
2.36.0.44.g0f828332d5ac
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 06/16] include: adjust header guards after renaming
2022-05-04 17:30 [PATCH 00/16] Misc cleanups marcandre.lureau
` (4 preceding siblings ...)
2022-05-04 17:30 ` [PATCH 05/16] qobject/json-lexer: disable -Winitializer-overrides warnings marcandre.lureau
@ 2022-05-04 17:30 ` marcandre.lureau
2022-05-04 18:10 ` Stefan Weil via
2022-05-04 17:30 ` [PATCH 07/16] qga: flatten safe_open_or_create() marcandre.lureau
` (10 subsequent siblings)
16 siblings, 1 reply; 24+ messages in thread
From: marcandre.lureau @ 2022-05-04 17:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Stefan Weil, Qiuhao Li,
Laurent Vivier, Stefan Hajnoczi, Hanna Reitz, Kevin Wolf,
Markus Armbruster, qemu-block, Konstantin Kostiuk, Bandan Das,
Michael Roth, Darren Kenny, Alexander Bulekov,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/qemu/help-texts.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/qemu/help-texts.h b/include/qemu/help-texts.h
index ba32cc8b1f39..4f265fed8df1 100644
--- a/include/qemu/help-texts.h
+++ b/include/qemu/help-texts.h
@@ -1,5 +1,5 @@
-#ifndef QEMU_COMMON_H
-#define QEMU_COMMON_H
+#ifndef QEMU_HELP_TEXTS_H
+#define QEMU_HELP_TEXTS_H
/* Copyright string for -version arguments, About dialogs, etc */
#define QEMU_COPYRIGHT "Copyright (c) 2003-2022 " \
--
2.36.0.44.g0f828332d5ac
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 06/16] include: adjust header guards after renaming
2022-05-04 17:30 ` [PATCH 06/16] include: adjust header guards after renaming marcandre.lureau
@ 2022-05-04 18:10 ` Stefan Weil via
0 siblings, 0 replies; 24+ messages in thread
From: Stefan Weil via @ 2022-05-04 18:10 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Qiuhao Li, Laurent Vivier,
Stefan Hajnoczi, Hanna Reitz, Kevin Wolf, Markus Armbruster,
qemu-block, Konstantin Kostiuk, Bandan Das, Michael Roth,
Darren Kenny, Alexander Bulekov
[-- Attachment #1.1.1: Type: text/plain, Size: 791 bytes --]
Am 04.05.22 um 19:30 schrieb marcandre.lureau@redhat.com:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> include/qemu/help-texts.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/qemu/help-texts.h b/include/qemu/help-texts.h
> index ba32cc8b1f39..4f265fed8df1 100644
> --- a/include/qemu/help-texts.h
> +++ b/include/qemu/help-texts.h
> @@ -1,5 +1,5 @@
> -#ifndef QEMU_COMMON_H
> -#define QEMU_COMMON_H
> +#ifndef QEMU_HELP_TEXTS_H
> +#define QEMU_HELP_TEXTS_H
>
> /* Copyright string for -version arguments, About dialogs, etc */
> #define QEMU_COPYRIGHT "Copyright (c) 2003-2022 " \
Reviewed-by: Stefan Weil <sw@weilnetz.de>
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 16043 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 07/16] qga: flatten safe_open_or_create()
2022-05-04 17:30 [PATCH 00/16] Misc cleanups marcandre.lureau
` (5 preceding siblings ...)
2022-05-04 17:30 ` [PATCH 06/16] include: adjust header guards after renaming marcandre.lureau
@ 2022-05-04 17:30 ` marcandre.lureau
2022-05-04 17:30 ` [PATCH 08/16] osdep: export qemu_open_cloexec() marcandre.lureau
` (9 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: marcandre.lureau @ 2022-05-04 17:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Stefan Weil, Qiuhao Li,
Laurent Vivier, Stefan Hajnoczi, Hanna Reitz, Kevin Wolf,
Markus Armbruster, qemu-block, Konstantin Kostiuk, Bandan Das,
Michael Roth, Darren Kenny, Alexander Bulekov,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
There is a bit too much branching in the function, this can be
simplified a bit, and have a common exit point thanks to ERRP_PROPAGATE.
This also helps with the following error handling changes.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qga/commands-posix.c | 126 ++++++++++++++++++++++---------------------
1 file changed, 65 insertions(+), 61 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 69f209af87e6..98d481a58485 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -339,73 +339,77 @@ find_open_flag(const char *mode_str, Error **errp)
static FILE *
safe_open_or_create(const char *path, const char *mode, Error **errp)
{
- Error *local_err = NULL;
- int oflag;
-
- oflag = find_open_flag(mode, &local_err);
- if (local_err == NULL) {
- int fd;
+ ERRP_GUARD();
+ int oflag, fd = -1;
+ FILE *f = NULL;
+
+ oflag = find_open_flag(mode, errp);
+ if (*errp) {
+ goto end;
+ }
+
+ /* If the caller wants / allows creation of a new file, we implement it
+ * with a two step process: open() + (open() / fchmod()).
+ *
+ * First we insist on creating the file exclusively as a new file. If
+ * that succeeds, we're free to set any file-mode bits on it. (The
+ * motivation is that we want to set those file-mode bits independently
+ * of the current umask.)
+ *
+ * If the exclusive creation fails because the file already exists
+ * (EEXIST is not possible for any other reason), we just attempt to
+ * open the file, but in this case we won't be allowed to change the
+ * file-mode bits on the preexistent file.
+ *
+ * The pathname should never disappear between the two open()s in
+ * practice. If it happens, then someone very likely tried to race us.
+ * In this case just go ahead and report the ENOENT from the second
+ * open() to the caller.
+ *
+ * If the caller wants to open a preexistent file, then the first
+ * open() is decisive and its third argument is ignored, and the second
+ * open() and the fchmod() are never called.
+ */
+ fd = open(path, oflag | ((oflag & O_CREAT) ? O_EXCL : 0), 0);
+ if (fd == -1 && errno == EEXIST) {
+ oflag &= ~(unsigned)O_CREAT;
+ fd = open(path, oflag);
+ }
+ if (fd == -1) {
+ error_setg_errno(errp, errno,
+ "failed to open file '%s' "
+ "(mode: '%s')",
+ path, mode);
+ goto end;
+ }
- /* If the caller wants / allows creation of a new file, we implement it
- * with a two step process: open() + (open() / fchmod()).
- *
- * First we insist on creating the file exclusively as a new file. If
- * that succeeds, we're free to set any file-mode bits on it. (The
- * motivation is that we want to set those file-mode bits independently
- * of the current umask.)
- *
- * If the exclusive creation fails because the file already exists
- * (EEXIST is not possible for any other reason), we just attempt to
- * open the file, but in this case we won't be allowed to change the
- * file-mode bits on the preexistent file.
- *
- * The pathname should never disappear between the two open()s in
- * practice. If it happens, then someone very likely tried to race us.
- * In this case just go ahead and report the ENOENT from the second
- * open() to the caller.
- *
- * If the caller wants to open a preexistent file, then the first
- * open() is decisive and its third argument is ignored, and the second
- * open() and the fchmod() are never called.
- */
- fd = open(path, oflag | ((oflag & O_CREAT) ? O_EXCL : 0), 0);
- if (fd == -1 && errno == EEXIST) {
- oflag &= ~(unsigned)O_CREAT;
- fd = open(path, oflag);
- }
+ qemu_set_cloexec(fd);
- if (fd == -1) {
- error_setg_errno(&local_err, errno, "failed to open file '%s' "
- "(mode: '%s')", path, mode);
- } else {
- qemu_set_cloexec(fd);
+ if ((oflag & O_CREAT) && fchmod(fd, DEFAULT_NEW_FILE_MODE) == -1) {
+ error_setg_errno(errp, errno,
+ "failed to set permission "
+ "0%03o on new file '%s' (mode: '%s')",
+ (unsigned)DEFAULT_NEW_FILE_MODE, path, mode);
+ goto end;
+ }
- if ((oflag & O_CREAT) && fchmod(fd, DEFAULT_NEW_FILE_MODE) == -1) {
- error_setg_errno(&local_err, errno, "failed to set permission "
- "0%03o on new file '%s' (mode: '%s')",
- (unsigned)DEFAULT_NEW_FILE_MODE, path, mode);
- } else {
- FILE *f;
-
- f = fdopen(fd, mode);
- if (f == NULL) {
- error_setg_errno(&local_err, errno, "failed to associate "
- "stdio stream with file descriptor %d, "
- "file '%s' (mode: '%s')", fd, path, mode);
- } else {
- return f;
- }
- }
+ f = fdopen(fd, mode);
+ if (f == NULL) {
+ error_setg_errno(errp, errno,
+ "failed to associate "
+ "stdio stream with file descriptor %d, "
+ "file '%s' (mode: '%s')",
+ fd, path, mode);
+ }
- close(fd);
- if (oflag & O_CREAT) {
- unlink(path);
- }
+end:
+ if (f == NULL && fd != -1) {
+ close(fd);
+ if (oflag & O_CREAT) {
+ unlink(path);
}
}
-
- error_propagate(errp, local_err);
- return NULL;
+ return f;
}
int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode,
--
2.36.0.44.g0f828332d5ac
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 08/16] osdep: export qemu_open_cloexec()
2022-05-04 17:30 [PATCH 00/16] Misc cleanups marcandre.lureau
` (6 preceding siblings ...)
2022-05-04 17:30 ` [PATCH 07/16] qga: flatten safe_open_or_create() marcandre.lureau
@ 2022-05-04 17:30 ` marcandre.lureau
2022-05-04 17:30 ` [PATCH 09/16] qga: use qemu_open_cloexec() for safe_open_or_create() marcandre.lureau
` (8 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: marcandre.lureau @ 2022-05-04 17:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Stefan Weil, Qiuhao Li,
Laurent Vivier, Stefan Hajnoczi, Hanna Reitz, Kevin Wolf,
Markus Armbruster, qemu-block, Konstantin Kostiuk, Bandan Das,
Michael Roth, Darren Kenny, Alexander Bulekov,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Used in the next patch, to simplify qga code.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/qemu/osdep.h | 1 +
util/osdep.c | 10 ++++++++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 67cc4654166b..64f51cfb7a62 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -489,6 +489,7 @@ void sigaction_invoke(struct sigaction *action,
*/
int qemu_open_old(const char *name, int flags, ...);
int qemu_open(const char *name, int flags, Error **errp);
+int qemu_open_cloexec(const char *name, int flags, mode_t mode, Error **errp);
int qemu_create(const char *name, int flags, mode_t mode, Error **errp);
int qemu_close(int fd);
int qemu_unlink(const char *name);
diff --git a/util/osdep.c b/util/osdep.c
index 60fcbbaebe72..67541b7654ef 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -279,9 +279,11 @@ int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive)
}
#endif
-static int qemu_open_cloexec(const char *name, int flags, mode_t mode)
+int qemu_open_cloexec(const char *name, int flags, mode_t mode, Error **errp)
{
+ ERRP_GUARD();
int ret;
+
#ifdef O_CLOEXEC
ret = open(name, flags | O_CLOEXEC, mode);
#else
@@ -290,6 +292,10 @@ static int qemu_open_cloexec(const char *name, int flags, mode_t mode)
qemu_set_cloexec(ret);
}
#endif
+ if (ret == -1) {
+ error_setg_errno(errp, errno, "Could not open '%s'", name);
+ }
+
return ret;
}
@@ -327,7 +333,7 @@ qemu_open_internal(const char *name, int flags, mode_t mode, Error **errp)
}
#endif
- ret = qemu_open_cloexec(name, flags, mode);
+ ret = qemu_open_cloexec(name, flags, mode, NULL);
if (ret == -1) {
const char *action = flags & O_CREAT ? "create" : "open";
--
2.36.0.44.g0f828332d5ac
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 09/16] qga: use qemu_open_cloexec() for safe_open_or_create()
2022-05-04 17:30 [PATCH 00/16] Misc cleanups marcandre.lureau
` (7 preceding siblings ...)
2022-05-04 17:30 ` [PATCH 08/16] osdep: export qemu_open_cloexec() marcandre.lureau
@ 2022-05-04 17:30 ` marcandre.lureau
2022-05-04 17:30 ` [PATCH 10/16] qapi/error: add g_autoptr(Error) support marcandre.lureau
` (7 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: marcandre.lureau @ 2022-05-04 17:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Stefan Weil, Qiuhao Li,
Laurent Vivier, Stefan Hajnoczi, Hanna Reitz, Kevin Wolf,
Markus Armbruster, qemu-block, Konstantin Kostiuk, Bandan Das,
Michael Roth, Darren Kenny, Alexander Bulekov,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The function takes care of setting CLOEXEC, and reporting error.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qga/commands-posix.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 98d481a58485..974bcbee7e97 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -370,21 +370,16 @@ safe_open_or_create(const char *path, const char *mode, Error **errp)
* open() is decisive and its third argument is ignored, and the second
* open() and the fchmod() are never called.
*/
- fd = open(path, oflag | ((oflag & O_CREAT) ? O_EXCL : 0), 0);
+ fd = qemu_open_cloexec(path, oflag | ((oflag & O_CREAT) ? O_EXCL : 0), 0, errp);
if (fd == -1 && errno == EEXIST) {
+ g_clear_pointer(errp, error_free);
oflag &= ~(unsigned)O_CREAT;
- fd = open(path, oflag);
+ fd = qemu_open_cloexec(path, oflag, 0, errp);
}
if (fd == -1) {
- error_setg_errno(errp, errno,
- "failed to open file '%s' "
- "(mode: '%s')",
- path, mode);
goto end;
}
- qemu_set_cloexec(fd);
-
if ((oflag & O_CREAT) && fchmod(fd, DEFAULT_NEW_FILE_MODE) == -1) {
error_setg_errno(errp, errno,
"failed to set permission "
--
2.36.0.44.g0f828332d5ac
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 10/16] qapi/error: add g_autoptr(Error) support
2022-05-04 17:30 [PATCH 00/16] Misc cleanups marcandre.lureau
` (8 preceding siblings ...)
2022-05-04 17:30 ` [PATCH 09/16] qga: use qemu_open_cloexec() for safe_open_or_create() marcandre.lureau
@ 2022-05-04 17:30 ` marcandre.lureau
2022-05-05 6:53 ` Paolo Bonzini
2022-05-04 17:30 ` [PATCH 11/16] qga: replace qemu_open_old() with qemu_open_cloexec() marcandre.lureau
` (6 subsequent siblings)
16 siblings, 1 reply; 24+ messages in thread
From: marcandre.lureau @ 2022-05-04 17:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Stefan Weil, Qiuhao Li,
Laurent Vivier, Stefan Hajnoczi, Hanna Reitz, Kevin Wolf,
Markus Armbruster, qemu-block, Konstantin Kostiuk, Bandan Das,
Michael Roth, Darren Kenny, Alexander Bulekov,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Sometime, ERRP_GUARD() isn't what you are looking for, because the
function doesn't throw errors, yet auto-cleaning is nice to have.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/qapi/error.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/qapi/error.h b/include/qapi/error.h
index d798faeec3e9..9482b6a58ae6 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -519,6 +519,8 @@ static inline void error_propagator_cleanup(ErrorPropagator *prop)
G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(ErrorPropagator, error_propagator_cleanup);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(Error, error_free);
+
/*
* Special error destination to abort on error.
* See error_setg() and error_propagate() for details.
--
2.36.0.44.g0f828332d5ac
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 10/16] qapi/error: add g_autoptr(Error) support
2022-05-04 17:30 ` [PATCH 10/16] qapi/error: add g_autoptr(Error) support marcandre.lureau
@ 2022-05-05 6:53 ` Paolo Bonzini
0 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2022-05-05 6:53 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel
Cc: Thomas Huth, Stefan Weil, Qiuhao Li, Laurent Vivier,
Stefan Hajnoczi, Hanna Reitz, Kevin Wolf, Markus Armbruster,
qemu-block, Konstantin Kostiuk, Bandan Das, Michael Roth,
Darren Kenny, Alexander Bulekov, Marc-André Lureau
This was rejected before on the grounds that propagating or printing the error is usually the right thing to do, and neither needs an autoptr:
https://patchew.org/QEMU/20210912124834.503032-1-pbonzini@redhat.com/
So while I do agree with the patch, for it to be accepted some Error* functions would have to be changed to accept Error** instead. This way they can NULL the variable containing their argument, and prevent the auto-error_free.
Paolo
Il 4 maggio 2022 19:30:19 CEST, marcandre.lureau@redhat.com ha scritto:
>From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
>Sometime, ERRP_GUARD() isn't what you are looking for, because the
>function doesn't throw errors, yet auto-cleaning is nice to have.
>
>Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>---
> include/qapi/error.h | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/include/qapi/error.h b/include/qapi/error.h
>index d798faeec3e9..9482b6a58ae6 100644
>--- a/include/qapi/error.h
>+++ b/include/qapi/error.h
>@@ -519,6 +519,8 @@ static inline void error_propagator_cleanup(ErrorPropagator *prop)
>
> G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(ErrorPropagator, error_propagator_cleanup);
>
>+G_DEFINE_AUTOPTR_CLEANUP_FUNC(Error, error_free);
>+
> /*
> * Special error destination to abort on error.
> * See error_setg() and error_propagate() for details.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 11/16] qga: replace qemu_open_old() with qemu_open_cloexec()
2022-05-04 17:30 [PATCH 00/16] Misc cleanups marcandre.lureau
` (9 preceding siblings ...)
2022-05-04 17:30 ` [PATCH 10/16] qapi/error: add g_autoptr(Error) support marcandre.lureau
@ 2022-05-04 17:30 ` marcandre.lureau
2022-05-04 17:30 ` [PATCH 12/16] test/qga: use G_TEST_DIR to locate os-release test file marcandre.lureau
` (5 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: marcandre.lureau @ 2022-05-04 17:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Stefan Weil, Qiuhao Li,
Laurent Vivier, Stefan Hajnoczi, Hanna Reitz, Kevin Wolf,
Markus Armbruster, qemu-block, Konstantin Kostiuk, Bandan Das,
Michael Roth, Darren Kenny, Alexander Bulekov,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
qemu_open_old() uses qemu_open_internal() which handles special
"/dev/fdset/" path for monitor fd sets, set CLOEXEC, and uses Error
reporting (and some O_DIRECT special error casing).
The monitor fdset handling is unnecessary for qga, use
qemu_open_cloexec() instead.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qga/channel-posix.c | 18 ++++++++++++------
qga/commands-posix.c | 23 +++++++++++------------
2 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index a996858e2492..7752114e214d 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -1,4 +1,5 @@
#include "qemu/osdep.h"
+#include "qemu/cutils.h"
#include <termios.h>
#include "qapi/error.h"
#include "qemu/sockets.h"
@@ -121,19 +122,24 @@ static int ga_channel_client_add(GAChannel *c, int fd)
static gboolean ga_channel_open(GAChannel *c, const gchar *path,
GAChannelMethod method, int fd)
{
+ g_autoptr(Error) err = NULL;
int ret;
c->method = method;
switch (c->method) {
case GA_CHANNEL_VIRTIO_SERIAL: {
assert(fd < 0);
- fd = qemu_open_old(path, O_RDWR | O_NONBLOCK
+ fd = qemu_open_cloexec(
+ path,
#ifndef CONFIG_SOLARIS
- | O_ASYNC
+ O_ASYNC |
#endif
- );
+ O_RDWR | O_NONBLOCK,
+ 0,
+ &err
+ );
if (fd == -1) {
- g_critical("error opening channel: %s", strerror(errno));
+ g_critical("error opening channel: %s", error_get_pretty(err));
return false;
}
#ifdef CONFIG_SOLARIS
@@ -157,9 +163,9 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
struct termios tio;
assert(fd < 0);
- fd = qemu_open_old(path, O_RDWR | O_NOCTTY | O_NONBLOCK);
+ fd = qemu_open_cloexec(path, O_RDWR | O_NOCTTY | O_NONBLOCK, 0, &err);
if (fd == -1) {
- g_critical("error opening channel: %s", strerror(errno));
+ g_critical("error opening channel: %s", error_get_pretty(err));
return false;
}
tcgetattr(fd, &tio);
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 974bcbee7e97..70036b789e6f 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1394,6 +1394,7 @@ static GuestDiskInfoList *get_disk_partitions(
static void get_nvme_smart(GuestDiskInfo *disk)
{
+ g_autoptr(Error) err = NULL;
int fd;
GuestNVMeSmart *smart;
NvmeSmartLog log = {0};
@@ -1406,9 +1407,9 @@ static void get_nvme_smart(GuestDiskInfo *disk)
| (((sizeof(log) >> 2) - 1) << 16)
};
- fd = qemu_open_old(disk->name, O_RDONLY);
+ fd = qemu_open_cloexec(disk->name, O_RDONLY, 0, &err);
if (fd == -1) {
- g_debug("Failed to open device: %s: %s", disk->name, g_strerror(errno));
+ g_debug("Failed to open device: %s: %s", disk->name, error_get_pretty(err));
return;
}
@@ -1739,9 +1740,8 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
}
}
- fd = qemu_open_old(mount->dirname, O_RDONLY);
+ fd = qemu_open_cloexec(mount->dirname, O_RDONLY, 0, errp);
if (fd == -1) {
- error_setg_errno(errp, errno, "failed to open %s", mount->dirname);
goto error;
}
@@ -1806,7 +1806,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **errp)
QTAILQ_FOREACH(mount, &mounts, next) {
logged = false;
- fd = qemu_open_old(mount->dirname, O_RDONLY);
+ fd = qemu_open_cloexec(mount->dirname, O_RDONLY, 0, NULL);
if (fd == -1) {
continue;
}
@@ -1866,21 +1866,20 @@ static void guest_fsfreeze_cleanup(void)
GuestFilesystemTrimResponse *
qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp)
{
+ ERRP_GUARD();
GuestFilesystemTrimResponse *response;
GuestFilesystemTrimResult *result;
int ret = 0;
FsMountList mounts;
struct FsMount *mount;
int fd;
- Error *local_err = NULL;
struct fstrim_range r;
slog("guest-fstrim called");
QTAILQ_INIT(&mounts);
- build_fs_mount_list(&mounts, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
+ build_fs_mount_list(&mounts, errp);
+ if (*errp) {
return NULL;
}
@@ -1892,11 +1891,11 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp)
QAPI_LIST_PREPEND(response->paths, result);
- fd = qemu_open_old(mount->dirname, O_RDONLY);
+ fd = qemu_open_cloexec(mount->dirname, O_RDONLY, 0, errp);
if (fd == -1) {
- result->error = g_strdup_printf("failed to open: %s",
- strerror(errno));
+ result->error = g_strdup(error_get_pretty(*errp));
result->has_error = true;
+ g_clear_pointer(errp, error_free);
continue;
}
--
2.36.0.44.g0f828332d5ac
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 12/16] test/qga: use G_TEST_DIR to locate os-release test file
2022-05-04 17:30 [PATCH 00/16] Misc cleanups marcandre.lureau
` (10 preceding siblings ...)
2022-05-04 17:30 ` [PATCH 11/16] qga: replace qemu_open_old() with qemu_open_cloexec() marcandre.lureau
@ 2022-05-04 17:30 ` marcandre.lureau
2022-05-04 17:30 ` [PATCH 13/16] qga/wixl: prefer variables over environment marcandre.lureau
` (4 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: marcandre.lureau @ 2022-05-04 17:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Stefan Weil, Qiuhao Li,
Laurent Vivier, Stefan Hajnoczi, Hanna Reitz, Kevin Wolf,
Markus Armbruster, qemu-block, Konstantin Kostiuk, Bandan Das,
Michael Roth, Darren Kenny, Alexander Bulekov,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
This a more accurate way to lookup the test data, and will allow to move
the test in a subproject.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/unit/test-qga.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c
index d6df1ee92ea1..ab0b12a2dd16 100644
--- a/tests/unit/test-qga.c
+++ b/tests/unit/test-qga.c
@@ -914,15 +914,14 @@ static void test_qga_guest_get_osinfo(gconstpointer data)
{
TestFixture fixture;
const gchar *str;
- gchar *cwd, *env[2];
- QDict *ret, *val;
+ QDict *ret = NULL;
+ char *env[2];
+ QDict *val;
- cwd = g_get_current_dir();
env[0] = g_strdup_printf(
- "QGA_OS_RELEASE=%s%ctests%cdata%ctest-qga-os-release",
- cwd, G_DIR_SEPARATOR, G_DIR_SEPARATOR, G_DIR_SEPARATOR);
+ "QGA_OS_RELEASE=%s%c..%cdata%ctest-qga-os-release",
+ g_test_get_dir(G_TEST_DIST), G_DIR_SEPARATOR, G_DIR_SEPARATOR, G_DIR_SEPARATOR);
env[1] = NULL;
- g_free(cwd);
fixture_setup(&fixture, NULL, env);
ret = qmp_fd(fixture.fd, "{'execute': 'guest-get-osinfo'}");
--
2.36.0.44.g0f828332d5ac
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 13/16] qga/wixl: prefer variables over environment
2022-05-04 17:30 [PATCH 00/16] Misc cleanups marcandre.lureau
` (11 preceding siblings ...)
2022-05-04 17:30 ` [PATCH 12/16] test/qga: use G_TEST_DIR to locate os-release test file marcandre.lureau
@ 2022-05-04 17:30 ` marcandre.lureau
2022-05-04 17:30 ` [PATCH 14/16] qga/wixl: require Mingw_bin marcandre.lureau
` (3 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: marcandre.lureau @ 2022-05-04 17:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Stefan Weil, Qiuhao Li,
Laurent Vivier, Stefan Hajnoczi, Hanna Reitz, Kevin Wolf,
Markus Armbruster, qemu-block, Konstantin Kostiuk, Bandan Das,
Michael Roth, Darren Kenny, Alexander Bulekov,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
No need to setup an environment or to check if the variable is undefined
manually.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qga/installer/qemu-ga.wxs | 30 +++++++++---------------------
qga/meson.build | 9 ++++-----
2 files changed, 13 insertions(+), 26 deletions(-)
diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index 0950e8c6becc..8a19aa165651 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -1,17 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<Wix xmlns="http://schemas.microsoft.com/wix/2006/wi">
- <?ifndef env.QEMU_GA_VERSION ?>
- <?error Environment variable QEMU_GA_VERSION undefined?>
- <?endif?>
-
- <?ifndef env.QEMU_GA_DISTRO ?>
- <?error Environment variable QEMU_GA_DISTRO undefined?>
- <?endif?>
-
- <?ifndef env.QEMU_GA_MANUFACTURER ?>
- <?error Environment variable QEMU_GA_MANUFACTURER undefined?>
- <?endif?>
-
<?ifndef var.Arch?>
<?error Define Arch to 32 or 64?>
<?endif?>
@@ -43,20 +31,20 @@
Name="QEMU guest agent"
Id="*"
UpgradeCode="{EB6B8302-C06E-4BEC-ADAC-932C68A3A98D}"
- Manufacturer="$(env.QEMU_GA_MANUFACTURER)"
- Version="$(env.QEMU_GA_VERSION)"
+ Manufacturer="$(var.QEMU_GA_MANUFACTURER)"
+ Version="$(var.QEMU_GA_VERSION)"
Language="1033">
<?if $(var.Arch) = 32 ?>
<Condition Message="Error: 32-bit version of Qemu GA can not be installed on 64-bit Windows.">NOT VersionNT64</Condition>
<?endif?>
<Package
- Manufacturer="$(env.QEMU_GA_MANUFACTURER)"
+ Manufacturer="$(var.QEMU_GA_MANUFACTURER)"
InstallerVersion="200"
Languages="1033"
Compressed="yes"
InstallScope="perMachine"
/>
- <Media Id="1" Cabinet="qemu_ga.$(env.QEMU_GA_VERSION).cab" EmbedCab="yes" />
+ <Media Id="1" Cabinet="qemu_ga.$(var.QEMU_GA_VERSION).cab" EmbedCab="yes" />
<Property Id="WHSLogo">1</Property>
<MajorUpgrade
DowngradeErrorMessage="Error: A newer version of QEMU guest agent is already installed."
@@ -66,7 +54,7 @@
<Directory Id="$(var.GaProgramFilesFolder)" Name="QEMU Guest Agent">
<Directory Id="qemu_ga_directory" Name="Qemu-ga">
<Component Id="qemu_ga" Guid="{908B7199-DE2A-4DC6-A8D0-27A5AE444FEA}">
- <File Id="qemu_ga.exe" Name="qemu-ga.exe" Source="$(env.BUILD_DIR)/qga/qemu-ga.exe" KeyPath="yes" DiskId="1"/>
+ <File Id="qemu_ga.exe" Name="qemu-ga.exe" Source="$(var.BUILD_DIR)/qga/qemu-ga.exe" KeyPath="yes" DiskId="1"/>
<ServiceInstall
Id="ServiceInstaller"
Type="ownProcess"
@@ -88,10 +76,10 @@
<File Id="libstdc++-6.lib" Name="libstdc++-6.dll" Source="$(var.Mingw_bin)/libstdc++-6.dll" KeyPath="yes" DiskId="1"/>
</Component>
<Component Id="qga_vss_dll" Guid="{CB19C453-FABB-4BB1-ABAB-6B74F687BFBB}">
- <File Id="qga_vss.dll" Name="qga-vss.dll" Source="$(env.BUILD_DIR)/qga/vss-win32/qga-vss.dll" KeyPath="yes" DiskId="1"/>
+ <File Id="qga_vss.dll" Name="qga-vss.dll" Source="$(var.BUILD_DIR)/qga/vss-win32/qga-vss.dll" KeyPath="yes" DiskId="1"/>
</Component>
<Component Id="qga_vss_tlb" Guid="{D8D584B1-59C2-4FB7-A91F-636FF7BFA66E}">
- <File Id="qga_vss.tlb" Name="qga-vss.tlb" Source="$(env.BUILD_DIR)/qga/vss-win32/qga-vss.tlb" KeyPath="yes" DiskId="1"/>
+ <File Id="qga_vss.tlb" Name="qga-vss.tlb" Source="$(var.BUILD_DIR)/qga/vss-win32/qga-vss.tlb" KeyPath="yes" DiskId="1"/>
</Component>
<?endif?>
<?if $(var.Arch) = "32"?>
@@ -133,9 +121,9 @@
</Component>
<Component Id="registry_entries" Guid="{D075D109-51CA-11E3-9F8B-000C29858960}">
<RegistryKey Root="HKLM"
- Key="Software\$(env.QEMU_GA_MANUFACTURER)\$(env.QEMU_GA_DISTRO)\Tools\QemuGA">
+ Key="Software\$(var.QEMU_GA_MANUFACTURER)\$(var.QEMU_GA_DISTRO)\Tools\QemuGA">
<RegistryValue Type="string" Name="ProductID" Value="fb0a0d66-c7fb-4e2e-a16b-c4a3bfe8d13b" />
- <RegistryValue Type="string" Name="Version" Value="$(env.QEMU_GA_VERSION)" />
+ <RegistryValue Type="string" Name="Version" Value="$(var.QEMU_GA_VERSION)" />
</RegistryKey>
</Component>
</Directory>
diff --git a/qga/meson.build b/qga/meson.build
index 6d9f39bb321b..3ad3bc0260cf 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -121,15 +121,14 @@ if targetos == 'windows'
output: 'qemu-ga-@0@.msi'.format(host_arch),
depends: deps,
command: [
- find_program('env'),
- 'QEMU_GA_VERSION=' + config_host['QEMU_GA_VERSION'],
- 'QEMU_GA_MANUFACTURER=' + config_host['QEMU_GA_MANUFACTURER'],
- 'QEMU_GA_DISTRO=' + config_host['QEMU_GA_DISTRO'],
- 'BUILD_DIR=' + meson.build_root(),
wixl, '-o', '@OUTPUT0@', '@INPUT0@',
qemu_ga_msi_arch[cpu],
qemu_ga_msi_vss,
+ '-D', 'BUILD_DIR=' + meson.build_root(),
'-D', 'Mingw_bin=' + config_host['QEMU_GA_MSI_MINGW_BIN_PATH'],
+ '-D', 'QEMU_GA_VERSION=' + config_host['QEMU_GA_VERSION'],
+ '-D', 'QEMU_GA_MANUFACTURER=' + config_host['QEMU_GA_MANUFACTURER'],
+ '-D', 'QEMU_GA_DISTRO=' + config_host['QEMU_GA_DISTRO'],
])
all_qga += [qga_msi]
alias_target('msi', qga_msi)
--
2.36.0.44.g0f828332d5ac
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 14/16] qga/wixl: require Mingw_bin
2022-05-04 17:30 [PATCH 00/16] Misc cleanups marcandre.lureau
` (12 preceding siblings ...)
2022-05-04 17:30 ` [PATCH 13/16] qga/wixl: prefer variables over environment marcandre.lureau
@ 2022-05-04 17:30 ` marcandre.lureau
2022-05-04 17:30 ` [PATCH 15/16] qga/wixl: simplify some pre-processing marcandre.lureau
` (2 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: marcandre.lureau @ 2022-05-04 17:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Stefan Weil, Qiuhao Li,
Laurent Vivier, Stefan Hajnoczi, Hanna Reitz, Kevin Wolf,
Markus Armbruster, qemu-block, Konstantin Kostiuk, Bandan Das,
Michael Roth, Darren Kenny, Alexander Bulekov,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
No clear reason to make guesses here.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qga/installer/qemu-ga.wxs | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index 8a19aa165651..651db6e51cda 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -4,15 +4,6 @@
<?error Define Arch to 32 or 64?>
<?endif?>
- <?ifndef var.Mingw_bin?>
- <?if $(var.Arch) = "64"?>
- <?define Mingw_bin=/usr/x86_64-w64-mingw32/sys-root/mingw/bin ?>
- <?endif?>
- <?if $(var.Arch) = "32"?>
- <?define Mingw_bin=/usr/i686-w64-mingw32/sys-root/mingw/bin ?>
- <?endif?>
- <?endif?>
-
<?if $(var.Arch) = "64"?>
<?define ArchLib=libgcc_s_seh-1.dll?>
<?define GaProgramFilesFolder="ProgramFiles64Folder" ?>
--
2.36.0.44.g0f828332d5ac
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 15/16] qga/wixl: simplify some pre-processing
2022-05-04 17:30 [PATCH 00/16] Misc cleanups marcandre.lureau
` (13 preceding siblings ...)
2022-05-04 17:30 ` [PATCH 14/16] qga/wixl: require Mingw_bin marcandre.lureau
@ 2022-05-04 17:30 ` marcandre.lureau
2022-05-04 17:30 ` [PATCH 16/16] qga/wixl: replace QEMU_GA_MSI_MINGW_BIN_PATH with glib bindir marcandre.lureau
2022-05-05 6:56 ` [PATCH 00/16] Misc cleanups Paolo Bonzini
16 siblings, 0 replies; 24+ messages in thread
From: marcandre.lureau @ 2022-05-04 17:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Stefan Weil, Qiuhao Li,
Laurent Vivier, Stefan Hajnoczi, Hanna Reitz, Kevin Wolf,
Markus Armbruster, qemu-block, Konstantin Kostiuk, Bandan Das,
Michael Roth, Darren Kenny, Alexander Bulekov,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Sadly, wixl doesn't have 'elif'.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qga/installer/qemu-ga.wxs | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index 651db6e51cda..e5b0958e1898 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -1,21 +1,15 @@
<?xml version="1.0" encoding="UTF-8"?>
<Wix xmlns="http://schemas.microsoft.com/wix/2006/wi">
- <?ifndef var.Arch?>
- <?error Define Arch to 32 or 64?>
- <?endif?>
-
<?if $(var.Arch) = "64"?>
<?define ArchLib=libgcc_s_seh-1.dll?>
<?define GaProgramFilesFolder="ProgramFiles64Folder" ?>
- <?endif?>
-
- <?if $(var.Arch) = "32"?>
- <?define ArchLib=libgcc_s_dw2-1.dll?>
- <?define GaProgramFilesFolder="ProgramFilesFolder" ?>
- <?endif?>
-
- <?ifndef var.ArchLib ?>
- <?error Unexpected Arch value $(var.Arch)?>
+ <?else?>
+ <?if $(var.Arch) = "32"?>
+ <?define ArchLib=libgcc_s_dw2-1.dll?>
+ <?define GaProgramFilesFolder="ProgramFilesFolder" ?>
+ <?else?>
+ <?error Unexpected Arch value $(var.Arch)?>
+ <?endif?>
<?endif?>
<Product
--
2.36.0.44.g0f828332d5ac
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 16/16] qga/wixl: replace QEMU_GA_MSI_MINGW_BIN_PATH with glib bindir
2022-05-04 17:30 [PATCH 00/16] Misc cleanups marcandre.lureau
` (14 preceding siblings ...)
2022-05-04 17:30 ` [PATCH 15/16] qga/wixl: simplify some pre-processing marcandre.lureau
@ 2022-05-04 17:30 ` marcandre.lureau
2022-05-05 6:56 ` [PATCH 00/16] Misc cleanups Paolo Bonzini
16 siblings, 0 replies; 24+ messages in thread
From: marcandre.lureau @ 2022-05-04 17:30 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Thomas Huth, Stefan Weil, Qiuhao Li,
Laurent Vivier, Stefan Hajnoczi, Hanna Reitz, Kevin Wolf,
Markus Armbruster, qemu-block, Konstantin Kostiuk, Bandan Das,
Michael Roth, Darren Kenny, Alexander Bulekov,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Use more conventional variables to set the location of pre-built
DLL/bin.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
configure | 9 ++++++---
meson.build | 5 ++++-
qga/installer/qemu-ga.wxs | 24 ++++++++++++------------
qga/meson.build | 2 +-
4 files changed, 23 insertions(+), 17 deletions(-)
diff --git a/configure b/configure
index 59c43bea05eb..616cd2d0e36c 100755
--- a/configure
+++ b/configure
@@ -2023,6 +2023,11 @@ for i in $glib_modules; do
fi
done
+glib_bindir="$($pkg_config --variable=bindir glib-2.0)"
+if test -z "$glib_bindir" ; then
+ glib_bindir="$($pkg_config --variable=prefix glib-2.0)"/bin
+fi
+
# This workaround is required due to a bug in pkg-config file for glib as it
# doesn't define GLIB_STATIC_COMPILATION for pkg-config --static
@@ -2430,8 +2435,6 @@ if test "$QEMU_GA_VERSION" = ""; then
QEMU_GA_VERSION=$(cat $source_path/VERSION)
fi
-QEMU_GA_MSI_MINGW_BIN_PATH="$($pkg_config --variable=prefix glib-2.0)/bin"
-
# Mac OS X ships with a broken assembler
roms=
if { test "$cpu" = "i386" || test "$cpu" = "x86_64"; } && \
@@ -2518,7 +2521,6 @@ if test "$debug_tcg" = "yes" ; then
fi
if test "$mingw32" = "yes" ; then
echo "CONFIG_WIN32=y" >> $config_host_mak
- echo "QEMU_GA_MSI_MINGW_BIN_PATH=${QEMU_GA_MSI_MINGW_BIN_PATH}" >> $config_host_mak
echo "QEMU_GA_MANUFACTURER=${QEMU_GA_MANUFACTURER}" >> $config_host_mak
echo "QEMU_GA_DISTRO=${QEMU_GA_DISTRO}" >> $config_host_mak
echo "QEMU_GA_VERSION=${QEMU_GA_VERSION}" >> $config_host_mak
@@ -2639,6 +2641,7 @@ echo "QEMU_CXXFLAGS=$QEMU_CXXFLAGS" >> $config_host_mak
echo "QEMU_OBJCFLAGS=$QEMU_OBJCFLAGS" >> $config_host_mak
echo "GLIB_CFLAGS=$glib_cflags" >> $config_host_mak
echo "GLIB_LIBS=$glib_libs" >> $config_host_mak
+echo "GLIB_BINDIR=$glib_bindir" >> $config_host_mak
echo "GLIB_VERSION=$(pkg-config --modversion glib-2.0)" >> $config_host_mak
echo "QEMU_LDFLAGS=$QEMU_LDFLAGS" >> $config_host_mak
echo "LD_I386_EMULATION=$ld_i386_emulation" >> $config_host_mak
diff --git a/meson.build b/meson.build
index c26aa442d40e..2f68b6cb8634 100644
--- a/meson.build
+++ b/meson.build
@@ -443,7 +443,10 @@ add_project_arguments(config_host['GLIB_CFLAGS'].split(),
native: false, language: ['c', 'cpp', 'objc'])
glib = declare_dependency(compile_args: config_host['GLIB_CFLAGS'].split(),
link_args: config_host['GLIB_LIBS'].split(),
- version: config_host['GLIB_VERSION'])
+ version: config_host['GLIB_VERSION'],
+ variables: {
+ 'bindir': config_host['GLIB_BINDIR'],
+ })
# override glib dep with the configure results (for subprojects)
meson.override_dependency('glib-2.0', glib)
diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index e5b0958e1898..813d1c6ca6ae 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -58,7 +58,7 @@
</Component>
<?ifdef var.InstallVss?>
<Component Id="libstdc++_6_lib" Guid="{55E737B5-9127-4A11-9FC3-A29367714574}">
- <File Id="libstdc++-6.lib" Name="libstdc++-6.dll" Source="$(var.Mingw_bin)/libstdc++-6.dll" KeyPath="yes" DiskId="1"/>
+ <File Id="libstdc++-6.lib" Name="libstdc++-6.dll" Source="$(var.BIN_DIR)/libstdc++-6.dll" KeyPath="yes" DiskId="1"/>
</Component>
<Component Id="qga_vss_dll" Guid="{CB19C453-FABB-4BB1-ABAB-6B74F687BFBB}">
<File Id="qga_vss.dll" Name="qga-vss.dll" Source="$(var.BUILD_DIR)/qga/vss-win32/qga-vss.dll" KeyPath="yes" DiskId="1"/>
@@ -69,40 +69,40 @@
<?endif?>
<?if $(var.Arch) = "32"?>
<Component Id="gspawn-helper-console" Guid="{446185B3-87BE-43D2-96B8-0FEFD9E8696D}">
- <File Id="gspawn-win32-helper-console.exe" Name="gspawn-win32-helper-console.exe" Source="$(var.Mingw_bin)/gspawn-win32-helper-console.exe" KeyPath="yes" DiskId="1"/>
+ <File Id="gspawn-win32-helper-console.exe" Name="gspawn-win32-helper-console.exe" Source="$(var.BIN_DIR)/gspawn-win32-helper-console.exe" KeyPath="yes" DiskId="1"/>
</Component>
<Component Id="gspawn-helper" Guid="{CD67A5A3-2DB1-4DA1-A67A-8D71E797B466}">
- <File Id="gspawn-win32-helper.exe" Name="gspawn-win32-helper.exe" Source="$(var.Mingw_bin)/gspawn-win32-helper-console.exe" KeyPath="yes" DiskId="1"/>
+ <File Id="gspawn-win32-helper.exe" Name="gspawn-win32-helper.exe" Source="$(var.BIN_DIR)/gspawn-win32-helper-console.exe" KeyPath="yes" DiskId="1"/>
</Component>
<?endif?>
<?if $(var.Arch) = "64"?>
<Component Id="gspawn-helper-console" Guid="{9E615A9F-349A-4992-A5C2-C10BAD173660}">
- <File Id="gspawn-win64-helper-console.exe" Name="gspawn-win64-helper-console.exe" Source="$(var.Mingw_bin)/gspawn-win64-helper-console.exe" KeyPath="yes" DiskId="1"/>
+ <File Id="gspawn-win64-helper-console.exe" Name="gspawn-win64-helper-console.exe" Source="$(var.BIN_DIR)/gspawn-win64-helper-console.exe" KeyPath="yes" DiskId="1"/>
</Component>
<Component Id="gspawn-helper" Guid="{D201AD22-1846-4E4F-B6E1-C7A908ED2457}">
- <File Id="gspawn-win64-helper.exe" Name="gspawn-win64-helper.exe" Source="$(var.Mingw_bin)/gspawn-win64-helper-console.exe" KeyPath="yes" DiskId="1"/>
+ <File Id="gspawn-win64-helper.exe" Name="gspawn-win64-helper.exe" Source="$(var.BIN_DIR)/gspawn-win64-helper-console.exe" KeyPath="yes" DiskId="1"/>
</Component>
<?endif?>
<Component Id="iconv" Guid="{35EE3558-D34B-4F0A-B8BD-430FF0775246}">
- <File Id="iconv.dll" Name="iconv.dll" Source="$(var.Mingw_bin)/iconv.dll" KeyPath="yes" DiskId="1"/>
+ <File Id="iconv.dll" Name="iconv.dll" Source="$(var.BIN_DIR)/iconv.dll" KeyPath="yes" DiskId="1"/>
</Component>
<Component Id="libgcc_arch_lib" Guid="{ADD4D07D-4515-4AB6-AF3E-C904961B4BB0}">
- <File Id="libgcc_arch_lib" Name="$(var.ArchLib)" Source="$(var.Mingw_bin)/$(var.ArchLib)" KeyPath="yes" DiskId="1"/>
+ <File Id="libgcc_arch_lib" Name="$(var.ArchLib)" Source="$(var.BIN_DIR)/$(var.ArchLib)" KeyPath="yes" DiskId="1"/>
</Component>
<Component Id="libglib" Guid="{D31BFD83-2773-4B65-B45A-E0D2ADA58679}">
- <File Id="libglib_2.0_0.dll" Name="libglib-2.0-0.dll" Source="$(var.Mingw_bin)/libglib-2.0-0.dll" KeyPath="yes" DiskId="1"/>
+ <File Id="libglib_2.0_0.dll" Name="libglib-2.0-0.dll" Source="$(var.BIN_DIR)/libglib-2.0-0.dll" KeyPath="yes" DiskId="1"/>
</Component>
<Component Id="libintl" Guid="{A641BC2D-A907-4A94-9149-F30ED430878F}">
- <File Id="libintl_8.dll" Name="libintl-8.dll" Source="$(var.Mingw_bin)/libintl-8.dll" KeyPath="yes" DiskId="1"/>
+ <File Id="libintl_8.dll" Name="libintl-8.dll" Source="$(var.BIN_DIR)/libintl-8.dll" KeyPath="yes" DiskId="1"/>
</Component>
<Component Id="libssp" Guid="{7880087B-02B4-4EF6-A5D3-D18F8E3D90E1}">
- <File Id="libssp_0.dll" Name="libssp-0.dll" Source="$(var.Mingw_bin)/libssp-0.dll" KeyPath="yes" DiskId="1"/>
+ <File Id="libssp_0.dll" Name="libssp-0.dll" Source="$(var.BIN_DIR)/libssp-0.dll" KeyPath="yes" DiskId="1"/>
</Component>
<Component Id="libwinpthread" Guid="{6C117C78-0F47-4B07-8F34-6BEE11643829}">
- <File Id="libwinpthread_1.dll" Name="libwinpthread-1.dll" Source="$(var.Mingw_bin)/libwinpthread-1.dll" KeyPath="yes" DiskId="1"/>
+ <File Id="libwinpthread_1.dll" Name="libwinpthread-1.dll" Source="$(var.BIN_DIR)/libwinpthread-1.dll" KeyPath="yes" DiskId="1"/>
</Component>
<Component Id="libpcre" Guid="{7A86B45E-A009-489A-A849-CE3BACF03CD0}">
- <File Id="libpcre_1.dll" Name="libpcre-1.dll" Source="$(var.Mingw_bin)/libpcre-1.dll" KeyPath="yes" DiskId="1"/>
+ <File Id="libpcre_1.dll" Name="libpcre-1.dll" Source="$(var.BIN_DIR)/libpcre-1.dll" KeyPath="yes" DiskId="1"/>
</Component>
<Component Id="registry_entries" Guid="{D075D109-51CA-11E3-9F8B-000C29858960}">
<RegistryKey Root="HKLM"
diff --git a/qga/meson.build b/qga/meson.build
index 3ad3bc0260cf..51b1e611b194 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -125,7 +125,7 @@ if targetos == 'windows'
qemu_ga_msi_arch[cpu],
qemu_ga_msi_vss,
'-D', 'BUILD_DIR=' + meson.build_root(),
- '-D', 'Mingw_bin=' + config_host['QEMU_GA_MSI_MINGW_BIN_PATH'],
+ '-D', 'BIN_DIR=' + glib.get_variable('bindir'),
'-D', 'QEMU_GA_VERSION=' + config_host['QEMU_GA_VERSION'],
'-D', 'QEMU_GA_MANUFACTURER=' + config_host['QEMU_GA_MANUFACTURER'],
'-D', 'QEMU_GA_DISTRO=' + config_host['QEMU_GA_DISTRO'],
--
2.36.0.44.g0f828332d5ac
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 00/16] Misc cleanups
2022-05-04 17:30 [PATCH 00/16] Misc cleanups marcandre.lureau
` (15 preceding siblings ...)
2022-05-04 17:30 ` [PATCH 16/16] qga/wixl: replace QEMU_GA_MSI_MINGW_BIN_PATH with glib bindir marcandre.lureau
@ 2022-05-05 6:56 ` Paolo Bonzini
16 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2022-05-05 6:56 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel
Cc: Thomas Huth, Stefan Weil, Qiuhao Li, Laurent Vivier,
Stefan Hajnoczi, Hanna Reitz, Kevin Wolf, Markus Armbruster,
qemu-block, Konstantin Kostiuk, Bandan Das, Michael Roth,
Darren Kenny, Alexander Bulekov, Marc-André Lureau
Looks good apart from the two patches I comment on.
Paolo
Il 4 maggio 2022 19:30:09 CEST, marcandre.lureau@redhat.com ha scritto:
>From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
>Hi,
>
>Perhaps the last series of preliminary patches before I propose a longer series
>to add qemu-common & qga meson subprojects. That's why they are mostly
>QGA-related cleanups.
>
>Thanks for the reviews!
>
>Marc-André Lureau (16):
> include: move qemu_*_exec_dir() to cutils
> util/win32: simplify qemu_get_local_state_dir()
> tests: make libqmp buildable for win32
> compiler.h: add QEMU_{BEGIN,END}_IGNORE_INITIALIZER_OVERRIDES
> qobject/json-lexer: disable -Winitializer-overrides warnings
> include: adjust header guards after renaming
> qga: flatten safe_open_or_create()
> osdep: export qemu_open_cloexec()
> qga: use qemu_open_cloexec() for safe_open_or_create()
> qapi/error: add g_autoptr(Error) support
> qga: replace qemu_open_old() with qemu_open_cloexec()
> test/qga: use G_TEST_DIR to locate os-release test file
> qga/wixl: prefer variables over environment
> qga/wixl: require Mingw_bin
> qga/wixl: simplify some pre-processing
> qga/wixl: replace QEMU_GA_MSI_MINGW_BIN_PATH with glib bindir
>
> configure | 9 +-
> meson.build | 5 +-
> include/qapi/error.h | 2 +
> include/qemu/compiler.h | 11 ++
> include/qemu/cutils.h | 7 ++
> include/qemu/help-texts.h | 4 +-
> include/qemu/osdep.h | 9 +-
> tests/qtest/libqmp.h | 2 +
> qemu-io.c | 1 +
> qga/channel-posix.c | 18 ++--
> qga/commands-posix.c | 146 +++++++++++++--------------
> qobject/json-lexer.c | 4 +
> storage-daemon/qemu-storage-daemon.c | 1 +
> tests/qtest/fuzz/fuzz.c | 1 +
> tests/qtest/libqmp.c | 35 +++++--
> tests/unit/test-qga.c | 11 +-
> util/cutils.c | 108 ++++++++++++++++++++
> util/osdep.c | 10 +-
> util/oslib-posix.c | 81 ---------------
> util/oslib-win32.c | 53 +---------
> qga/installer/qemu-ga.wxs | 83 +++++----------
> qga/meson.build | 11 +-
> 22 files changed, 313 insertions(+), 299 deletions(-)
>
^ permalink raw reply [flat|nested] 24+ messages in thread