* [PATCH v5 00/15] Misc cleanups
@ 2022-05-25 14:41 marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 01/15] include: move qemu_*_exec_dir() to cutils marcandre.lureau
` (14 more replies)
0 siblings, 15 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-05-25 14:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Konstantin Kostiuk, Markus Armbruster, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Hi,
More preliminary cleanups before a series to make qemu-ga a meson subproject().
v5:
- drop Error argument from qga_open_cloexec()
- add r-b tags
v4:
- remove some unnecessary ERRP_GUARD()
- open-code some g_clear_pointer()
- don't export qemu_open_cloexec, introduce qga_open_cloexec instead
- some formatting improvements
- add new r-b tags
v3:
- changed error_report_err() back to g_critical()
- added "qga: make build_fs_mount_list() return a bool"
- replaced g_clear_pointer() usage by open-coded version
- dropped needless g_autoptr(GError) in tests
- rebased, (dropped "include: adjust header guards after renaming")
- some commit message rewording
- added r-b tags
v2:
- drop "compiler.h: add QEMU_{BEGIN,END}_IGNORE_INITIALIZER_OVERRIDES",
"qobject/json-lexer: disable -Winitializer-overrides warnings" &
"qapi/error: add g_autoptr(Error) support" and adjust related code.
- add "test/qga: use g_auto wherever sensible"
- add r-b tags
Marc-André Lureau (15):
include: move qemu_*_exec_dir() to cutils
util/win32: simplify qemu_get_local_state_dir()
tests: make libqmp buildable for win32
qga: flatten safe_open_or_create()
qga: add qga_open_cloexec() helper
qga: use qga_open_cloexec() for safe_open_or_create()
qga: throw an Error in ga_channel_open()
qga: replace qemu_open_old() with qga_open_cloexec()
qga: make build_fs_mount_list() return a bool
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
test/qga: use g_auto wherever sensible
configure | 9 +-
meson.build | 5 +-
include/qemu/cutils.h | 7 ++
include/qemu/osdep.h | 8 --
qga/cutils.h | 8 ++
tests/qtest/libqmp.h | 2 +
qemu-io.c | 1 +
qga/channel-posix.c | 55 +++++-----
qga/commands-posix.c | 154 +++++++++++++--------------
qga/cutils.c | 33 ++++++
storage-daemon/qemu-storage-daemon.c | 1 +
tests/qtest/fuzz/fuzz.c | 1 +
tests/qtest/libqmp.c | 34 +++++-
tests/unit/test-qga.c | 130 ++++++++--------------
util/cutils.c | 108 +++++++++++++++++++
util/oslib-posix.c | 81 --------------
util/oslib-win32.c | 53 +--------
qga/installer/qemu-ga.wxs | 83 +++++----------
qga/meson.build | 12 +--
19 files changed, 385 insertions(+), 400 deletions(-)
create mode 100644 qga/cutils.h
create mode 100644 qga/cutils.c
--
2.36.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 01/15] include: move qemu_*_exec_dir() to cutils
2022-05-25 14:41 [PATCH v5 00/15] Misc cleanups marcandre.lureau
@ 2022-05-25 14:41 ` marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 02/15] util/win32: simplify qemu_get_local_state_dir() marcandre.lureau
` (13 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-05-25 14:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Konstantin Kostiuk, Markus Armbruster, 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>
Reviewed-by: Markus Armbruster <armbru@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 5c6572d444..40e10e19a7 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 a72e99db85..b1c161c035 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 d70d3dd4fd..2bd7bfb650 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 9b8b17f52e..c104817cdd 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 a7a5e14fa3..0ad4ba9e94 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 b2777210e7..6cc7cc8cde 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 477990f39b..7ba4472760 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 dafef4f157..6c818749d2 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.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v5 02/15] util/win32: simplify qemu_get_local_state_dir()
2022-05-25 14:41 [PATCH v5 00/15] Misc cleanups marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 01/15] include: move qemu_*_exec_dir() to cutils marcandre.lureau
@ 2022-05-25 14:41 ` marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 03/15] tests: make libqmp buildable for win32 marcandre.lureau
` (12 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-05-25 14:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Konstantin Kostiuk, Markus Armbruster, 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>
Reviewed-by: Stefan Weil <sw@weilnetz.de>
---
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 6c818749d2..5723d3eb4c 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.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v5 03/15] tests: make libqmp buildable for win32
2022-05-25 14:41 [PATCH v5 00/15] Misc cleanups marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 01/15] include: move qemu_*_exec_dir() to cutils marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 02/15] util/win32: simplify qemu_get_local_state_dir() marcandre.lureau
@ 2022-05-25 14:41 ` marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 04/15] qga: flatten safe_open_or_create() marcandre.lureau
` (11 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-05-25 14:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Konstantin Kostiuk, Markus Armbruster, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
tests/qtest/libqmp.h | 2 ++
tests/qtest/libqmp.c | 34 +++++++++++++++++++++++++++++-----
2 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/tests/qtest/libqmp.h b/tests/qtest/libqmp.h
index 5cb7eeaa18..3445b753ff 100644
--- a/tests/qtest/libqmp.h
+++ b/tests/qtest/libqmp.h
@@ -21,8 +21,10 @@
#include "qapi/qmp/qdict.h"
QDict *qmp_fd_receive(int fd);
+#ifndef _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 0358b8313d..ade26c15f0 100644
--- a/tests/qtest/libqmp.c
+++ b/tests/qtest/libqmp.c
@@ -18,6 +18,11 @@
#include "libqmp.h"
+#ifndef _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 +92,7 @@ QDict *qmp_fd_receive(int fd)
return qmp.response;
}
+#ifndef _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 +126,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 _WIN32
+ assert(fds_num == 0);
+#endif
+
/* Going through qobject ensures we escape strings properly */
qobj = qobject_from_vjsonf_nofail(fmt, ap);
@@ -148,10 +160,14 @@ void qmp_fd_vsend_fds(int fd, int *fds, size_t fds_num,
if (log) {
fprintf(stderr, "%s", str->str);
}
+
+#ifndef _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 +176,23 @@ void qmp_fd_vsend_fds(int fd, int *fds, size_t fds_num,
}
}
+#ifndef _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.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v5 04/15] qga: flatten safe_open_or_create()
2022-05-25 14:41 [PATCH v5 00/15] Misc cleanups marcandre.lureau
` (2 preceding siblings ...)
2022-05-25 14:41 ` [PATCH v5 03/15] tests: make libqmp buildable for win32 marcandre.lureau
@ 2022-05-25 14:41 ` marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 05/15] qga: add qga_open_cloexec() helper marcandre.lureau
` (10 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-05-25 14:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Konstantin Kostiuk, Markus Armbruster, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
There is a bit too much nesting in the function, this can be simplified
a bit to improve readability.
This also helps with the following error handling changes.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
qga/commands-posix.c | 120 +++++++++++++++++++++----------------------
1 file changed, 60 insertions(+), 60 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 69f209af87..8ee149e550 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -339,73 +339,73 @@ 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;
+ int fd = -1;
+ FILE *f = NULL;
+
+ oflag = find_open_flag(mode, errp);
+ if (oflag < 0) {
+ 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;
+ }
- oflag = find_open_flag(mode, &local_err);
- if (local_err == NULL) {
- int fd;
-
- /* 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.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v5 05/15] qga: add qga_open_cloexec() helper
2022-05-25 14:41 [PATCH v5 00/15] Misc cleanups marcandre.lureau
` (3 preceding siblings ...)
2022-05-25 14:41 ` [PATCH v5 04/15] qga: flatten safe_open_or_create() marcandre.lureau
@ 2022-05-25 14:41 ` marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 06/15] qga: use qga_open_cloexec() for safe_open_or_create() marcandre.lureau
` (9 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-05-25 14:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Konstantin Kostiuk, Markus Armbruster, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
QGA calls qemu_open_old() in various places. Calling qemu_open() instead
isn't a great alternative, as it has special "/dev/fdset" handling and
depends on QEMU internal monitor data structures.
Instead, provide a simple helper for QGA needs, with Error* support. The
following patches will make use of it.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
qga/cutils.h | 8 ++++++++
qga/cutils.c | 33 +++++++++++++++++++++++++++++++++
qga/meson.build | 1 +
3 files changed, 42 insertions(+)
create mode 100644 qga/cutils.h
create mode 100644 qga/cutils.c
diff --git a/qga/cutils.h b/qga/cutils.h
new file mode 100644
index 0000000000..f0f30a7d28
--- /dev/null
+++ b/qga/cutils.h
@@ -0,0 +1,8 @@
+#ifndef CUTILS_H_
+#define CUTILS_H_
+
+#include "qemu/osdep.h"
+
+int qga_open_cloexec(const char *name, int flags, mode_t mode);
+
+#endif /* CUTILS_H_ */
diff --git a/qga/cutils.c b/qga/cutils.c
new file mode 100644
index 0000000000..b8e142ef64
--- /dev/null
+++ b/qga/cutils.c
@@ -0,0 +1,33 @@
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "cutils.h"
+
+#include "qapi/error.h"
+
+/**
+ * qga_open_cloexec:
+ * @name: the pathname to open
+ * @flags: as in open()
+ * @mode: as in open()
+ *
+ * A wrapper for open() function which sets O_CLOEXEC.
+ *
+ * On error, -1 is returned.
+ */
+int qga_open_cloexec(const char *name, int flags, mode_t mode)
+{
+ int ret;
+
+#ifdef O_CLOEXEC
+ ret = open(name, flags | O_CLOEXEC, mode);
+#else
+ ret = open(name, flags, mode);
+ if (ret >= 0) {
+ qemu_set_cloexec(ret);
+ }
+#endif
+
+ return ret;
+}
diff --git a/qga/meson.build b/qga/meson.build
index 6d9f39bb32..35fe2229e9 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -65,6 +65,7 @@ qga_ss.add(files(
'commands.c',
'guest-agent-command-state.c',
'main.c',
+ 'cutils.c',
))
qga_ss.add(when: 'CONFIG_POSIX', if_true: files(
'channel-posix.c',
--
2.36.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v5 06/15] qga: use qga_open_cloexec() for safe_open_or_create()
2022-05-25 14:41 [PATCH v5 00/15] Misc cleanups marcandre.lureau
` (4 preceding siblings ...)
2022-05-25 14:41 ` [PATCH v5 05/15] qga: add qga_open_cloexec() helper marcandre.lureau
@ 2022-05-25 14:41 ` marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 07/15] qga: throw an Error in ga_channel_open() marcandre.lureau
` (8 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-05-25 14:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Konstantin Kostiuk, Markus Armbruster, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The function takes care of setting CLOEXEC.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
qga/commands-posix.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 8ee149e550..88ccbf2fdf 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -27,6 +27,7 @@
#include "qemu/cutils.h"
#include "commands-common.h"
#include "block/nvme.h"
+#include "cutils.h"
#ifdef HAVE_UTMPX
#include <utmpx.h>
@@ -370,10 +371,10 @@ 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 = qga_open_cloexec(path, oflag | ((oflag & O_CREAT) ? O_EXCL : 0), 0);
if (fd == -1 && errno == EEXIST) {
oflag &= ~(unsigned)O_CREAT;
- fd = open(path, oflag);
+ fd = qga_open_cloexec(path, oflag, 0);
}
if (fd == -1) {
error_setg_errno(errp, errno,
@@ -382,8 +383,6 @@ safe_open_or_create(const char *path, const char *mode, Error **errp)
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 "
"0%03o on new file '%s' (mode: '%s')",
--
2.36.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v5 07/15] qga: throw an Error in ga_channel_open()
2022-05-25 14:41 [PATCH v5 00/15] Misc cleanups marcandre.lureau
` (5 preceding siblings ...)
2022-05-25 14:41 ` [PATCH v5 06/15] qga: use qga_open_cloexec() for safe_open_or_create() marcandre.lureau
@ 2022-05-25 14:41 ` marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 08/15] qga: replace qemu_open_old() with qga_open_cloexec() marcandre.lureau
` (7 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-05-25 14:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Konstantin Kostiuk, Markus Armbruster, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Allow for a single point of error reporting, and further refactoring.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
qga/channel-posix.c | 42 +++++++++++++++++-------------------------
1 file changed, 17 insertions(+), 25 deletions(-)
diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index a996858e24..25fcc5cb1a 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -119,7 +119,7 @@ static int ga_channel_client_add(GAChannel *c, int fd)
}
static gboolean ga_channel_open(GAChannel *c, const gchar *path,
- GAChannelMethod method, int fd)
+ GAChannelMethod method, int fd, Error **errp)
{
int ret;
c->method = method;
@@ -133,21 +133,20 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
#endif
);
if (fd == -1) {
- g_critical("error opening channel: %s", strerror(errno));
+ error_setg_errno(errp, errno, "error opening channel");
return false;
}
#ifdef CONFIG_SOLARIS
ret = ioctl(fd, I_SETSIG, S_OUTPUT | S_INPUT | S_HIPRI);
if (ret == -1) {
- g_critical("error setting event mask for channel: %s",
- strerror(errno));
+ error_setg_errno(errp, errno, "error setting event mask for channel");
close(fd);
return false;
}
#endif
ret = ga_channel_client_add(c, fd);
if (ret) {
- g_critical("error adding channel to main loop");
+ error_setg(errp, "error adding channel to main loop");
close(fd);
return false;
}
@@ -159,7 +158,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
assert(fd < 0);
fd = qemu_open_old(path, O_RDWR | O_NOCTTY | O_NONBLOCK);
if (fd == -1) {
- g_critical("error opening channel: %s", strerror(errno));
+ error_setg_errno(errp, errno, "error opening channel");
return false;
}
tcgetattr(fd, &tio);
@@ -180,7 +179,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
tcsetattr(fd, TCSANOW, &tio);
ret = ga_channel_client_add(c, fd);
if (ret) {
- g_critical("error adding channel to main loop");
+ error_setg(errp, "error adding channel to main loop");
close(fd);
return false;
}
@@ -188,12 +187,8 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
}
case GA_CHANNEL_UNIX_LISTEN: {
if (fd < 0) {
- Error *local_err = NULL;
-
- fd = unix_listen(path, &local_err);
- if (local_err != NULL) {
- g_critical("%s", error_get_pretty(local_err));
- error_free(local_err);
+ fd = unix_listen(path, errp);
+ if (fd < 0) {
return false;
}
}
@@ -202,24 +197,19 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
}
case GA_CHANNEL_VSOCK_LISTEN: {
if (fd < 0) {
- Error *local_err = NULL;
SocketAddress *addr;
char *addr_str;
addr_str = g_strdup_printf("vsock:%s", path);
- addr = socket_parse(addr_str, &local_err);
+ addr = socket_parse(addr_str, errp);
g_free(addr_str);
- if (local_err != NULL) {
- g_critical("%s", error_get_pretty(local_err));
- error_free(local_err);
+ if (!addr) {
return false;
}
- fd = socket_listen(addr, 1, &local_err);
+ fd = socket_listen(addr, 1, errp);
qapi_free_SocketAddress(addr);
- if (local_err != NULL) {
- g_critical("%s", error_get_pretty(local_err));
- error_free(local_err);
+ if (fd < 0) {
return false;
}
}
@@ -227,7 +217,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
break;
}
default:
- g_critical("error binding/listening to specified socket");
+ error_setg(errp, "error binding/listening to specified socket");
return false;
}
@@ -272,12 +262,14 @@ GIOStatus ga_channel_read(GAChannel *c, gchar *buf, gsize size, gsize *count)
GAChannel *ga_channel_new(GAChannelMethod method, const gchar *path,
int listen_fd, GAChannelCallback cb, gpointer opaque)
{
+ Error *err = NULL;
GAChannel *c = g_new0(GAChannel, 1);
c->event_cb = cb;
c->user_data = opaque;
- if (!ga_channel_open(c, path, method, listen_fd)) {
- g_critical("error opening channel");
+ if (!ga_channel_open(c, path, method, listen_fd, &err)) {
+ g_critical("%s", error_get_pretty(err));
+ error_free(err);
ga_channel_free(c);
return NULL;
}
--
2.36.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v5 08/15] qga: replace qemu_open_old() with qga_open_cloexec()
2022-05-25 14:41 [PATCH v5 00/15] Misc cleanups marcandre.lureau
` (6 preceding siblings ...)
2022-05-25 14:41 ` [PATCH v5 07/15] qga: throw an Error in ga_channel_open() marcandre.lureau
@ 2022-05-25 14:41 ` marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 09/15] qga: make build_fs_mount_list() return a bool marcandre.lureau
` (6 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-05-25 14:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Konstantin Kostiuk, Markus Armbruster, 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
qga_open_cloexec() instead.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
---
qga/channel-posix.c | 13 +++++++++----
qga/commands-posix.c | 8 ++++----
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index 25fcc5cb1a..6796a02cff 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -1,8 +1,10 @@
#include "qemu/osdep.h"
+#include "qemu/cutils.h"
#include <termios.h>
#include "qapi/error.h"
#include "qemu/sockets.h"
#include "channel.h"
+#include "cutils.h"
#ifdef CONFIG_SOLARIS
#include <stropts.h>
@@ -127,11 +129,14 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
switch (c->method) {
case GA_CHANNEL_VIRTIO_SERIAL: {
assert(fd < 0);
- fd = qemu_open_old(path, O_RDWR | O_NONBLOCK
+ fd = qga_open_cloexec(
+ path,
#ifndef CONFIG_SOLARIS
- | O_ASYNC
+ O_ASYNC |
#endif
- );
+ O_RDWR | O_NONBLOCK,
+ 0
+ );
if (fd == -1) {
error_setg_errno(errp, errno, "error opening channel");
return false;
@@ -156,7 +161,7 @@ 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 = qga_open_cloexec(path, O_RDWR | O_NOCTTY | O_NONBLOCK, 0);
if (fd == -1) {
error_setg_errno(errp, errno, "error opening channel");
return false;
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 88ccbf2fdf..ff2d21d394 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1406,7 +1406,7 @@ static void get_nvme_smart(GuestDiskInfo *disk)
| (((sizeof(log) >> 2) - 1) << 16)
};
- fd = qemu_open_old(disk->name, O_RDONLY);
+ fd = qga_open_cloexec(disk->name, O_RDONLY, 0);
if (fd == -1) {
g_debug("Failed to open device: %s: %s", disk->name, g_strerror(errno));
return;
@@ -1739,7 +1739,7 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
}
}
- fd = qemu_open_old(mount->dirname, O_RDONLY);
+ fd = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
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 = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
if (fd == -1) {
continue;
}
@@ -1892,7 +1892,7 @@ 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 = qga_open_cloexec(mount->dirname, O_RDONLY, 0);
if (fd == -1) {
result->error = g_strdup_printf("failed to open: %s",
strerror(errno));
--
2.36.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v5 09/15] qga: make build_fs_mount_list() return a bool
2022-05-25 14:41 [PATCH v5 00/15] Misc cleanups marcandre.lureau
` (7 preceding siblings ...)
2022-05-25 14:41 ` [PATCH v5 08/15] qga: replace qemu_open_old() with qga_open_cloexec() marcandre.lureau
@ 2022-05-25 14:41 ` marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 10/15] test/qga: use G_TEST_DIR to locate os-release test file marcandre.lureau
` (5 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-05-25 14:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Konstantin Kostiuk, Markus Armbruster, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Change build_fs_mount_list() to return bool, in accordance
with the guidance under = Rules = in include/qapi/error.h
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Suggested-by: Markus Armbruster <armbru@redhat.com>
---
qga/commands-posix.c | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index ff2d21d394..af93faae97 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -673,7 +673,7 @@ static int dev_major_minor(const char *devpath,
/*
* Walk the mount table and build a list of local file systems
*/
-static void build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
+static bool build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
{
struct mntent *ment;
FsMount *mount;
@@ -684,7 +684,7 @@ static void build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
fp = setmntent(mtab, "r");
if (!fp) {
error_setg(errp, "failed to open mtab file: '%s'", mtab);
- return;
+ return false;
}
while ((ment = getmntent(fp))) {
@@ -714,6 +714,7 @@ static void build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
}
endmntent(fp);
+ return true;
}
static void decode_mntname(char *name, int len)
@@ -738,7 +739,7 @@ static void decode_mntname(char *name, int len)
}
}
-static void build_fs_mount_list(FsMountList *mounts, Error **errp)
+static bool build_fs_mount_list(FsMountList *mounts, Error **errp)
{
FsMount *mount;
char const *mountinfo = "/proc/self/mountinfo";
@@ -751,8 +752,7 @@ static void build_fs_mount_list(FsMountList *mounts, Error **errp)
fp = fopen(mountinfo, "r");
if (!fp) {
- build_fs_mount_list_from_mtab(mounts, errp);
- return;
+ return build_fs_mount_list_from_mtab(mounts, errp);
}
while (getline(&line, &n, fp) != -1) {
@@ -794,6 +794,7 @@ static void build_fs_mount_list(FsMountList *mounts, Error **errp)
free(line);
fclose(fp);
+ return true;
}
#endif
@@ -1592,8 +1593,7 @@ GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp)
Error *local_err = NULL;
QTAILQ_INIT(&mounts);
- build_fs_mount_list(&mounts, &local_err);
- if (local_err) {
+ if (!build_fs_mount_list(&mounts, &local_err)) {
error_propagate(errp, local_err);
return NULL;
}
@@ -1716,8 +1716,7 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
}
QTAILQ_INIT(&mounts);
- build_fs_mount_list(&mounts, &local_err);
- if (local_err) {
+ if (!build_fs_mount_list(&mounts, &local_err)) {
error_propagate(errp, local_err);
return -1;
}
@@ -1798,8 +1797,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **errp)
Error *local_err = NULL;
QTAILQ_INIT(&mounts);
- build_fs_mount_list(&mounts, &local_err);
- if (local_err) {
+ if (!build_fs_mount_list(&mounts, &local_err)) {
error_propagate(errp, local_err);
return 0;
}
@@ -1872,15 +1870,12 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp)
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);
+ if (!build_fs_mount_list(&mounts, errp)) {
return NULL;
}
--
2.36.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v5 10/15] test/qga: use G_TEST_DIR to locate os-release test file
2022-05-25 14:41 [PATCH v5 00/15] Misc cleanups marcandre.lureau
` (8 preceding siblings ...)
2022-05-25 14:41 ` [PATCH v5 09/15] qga: make build_fs_mount_list() return a bool marcandre.lureau
@ 2022-05-25 14:41 ` marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 11/15] qga/wixl: prefer variables over environment marcandre.lureau
` (4 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-05-25 14:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Konstantin Kostiuk, Markus Armbruster, 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>
Reviewed-by: Konstantin Kostiuk <kkostiuk@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 d6df1ee92e..ab0b12a2dd 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.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v5 11/15] qga/wixl: prefer variables over environment
2022-05-25 14:41 [PATCH v5 00/15] Misc cleanups marcandre.lureau
` (9 preceding siblings ...)
2022-05-25 14:41 ` [PATCH v5 10/15] test/qga: use G_TEST_DIR to locate os-release test file marcandre.lureau
@ 2022-05-25 14:41 ` marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 12/15] qga/wixl: require Mingw_bin marcandre.lureau
` (3 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-05-25 14:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Konstantin Kostiuk, Markus Armbruster, 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>
Reviewed-by: Konstantin Kostiuk <kkostiuk@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 0950e8c6be..8a19aa1656 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 35fe2229e9..31370405f9 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -122,15 +122,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.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v5 12/15] qga/wixl: require Mingw_bin
2022-05-25 14:41 [PATCH v5 00/15] Misc cleanups marcandre.lureau
` (10 preceding siblings ...)
2022-05-25 14:41 ` [PATCH v5 11/15] qga/wixl: prefer variables over environment marcandre.lureau
@ 2022-05-25 14:41 ` marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 13/15] qga/wixl: simplify some pre-processing marcandre.lureau
` (2 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-05-25 14:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Konstantin Kostiuk, Markus Armbruster, 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>
Reviewed-by: Konstantin Kostiuk <kkostiuk@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 8a19aa1656..651db6e51c 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.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v5 13/15] qga/wixl: simplify some pre-processing
2022-05-25 14:41 [PATCH v5 00/15] Misc cleanups marcandre.lureau
` (11 preceding siblings ...)
2022-05-25 14:41 ` [PATCH v5 12/15] qga/wixl: require Mingw_bin marcandre.lureau
@ 2022-05-25 14:41 ` marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 14/15] qga/wixl: replace QEMU_GA_MSI_MINGW_BIN_PATH with glib bindir marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 15/15] test/qga: use g_auto wherever sensible marcandre.lureau
14 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-05-25 14:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Konstantin Kostiuk, Markus Armbruster, 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>
Reviewed-by: Konstantin Kostiuk <kkostiuk@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 651db6e51c..e5b0958e18 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.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v5 14/15] qga/wixl: replace QEMU_GA_MSI_MINGW_BIN_PATH with glib bindir
2022-05-25 14:41 [PATCH v5 00/15] Misc cleanups marcandre.lureau
` (12 preceding siblings ...)
2022-05-25 14:41 ` [PATCH v5 13/15] qga/wixl: simplify some pre-processing marcandre.lureau
@ 2022-05-25 14:41 ` marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 15/15] test/qga: use g_auto wherever sensible marcandre.lureau
14 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-05-25 14:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Konstantin Kostiuk, Markus Armbruster, 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>
Reviewed-by: Konstantin Kostiuk <kkostiuk@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 180ee688dc..f2baf2f526 100755
--- a/configure
+++ b/configure
@@ -1495,6 +1495,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
@@ -1860,8 +1865,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"; } && \
@@ -1948,7 +1951,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
@@ -2020,6 +2022,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 9ebc00f032..8830b4dc1a 100644
--- a/meson.build
+++ b/meson.build
@@ -466,7 +466,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 e5b0958e18..813d1c6ca6 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 31370405f9..619ff095bc 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -126,7 +126,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.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v5 15/15] test/qga: use g_auto wherever sensible
2022-05-25 14:41 [PATCH v5 00/15] Misc cleanups marcandre.lureau
` (13 preceding siblings ...)
2022-05-25 14:41 ` [PATCH v5 14/15] qga/wixl: replace QEMU_GA_MSI_MINGW_BIN_PATH with glib bindir marcandre.lureau
@ 2022-05-25 14:41 ` marcandre.lureau
14 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-05-25 14:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Konstantin Kostiuk, Markus Armbruster, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
---
tests/unit/test-qga.c | 121 +++++++++++++++---------------------------
1 file changed, 43 insertions(+), 78 deletions(-)
diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c
index ab0b12a2dd..530317044b 100644
--- a/tests/unit/test-qga.c
+++ b/tests/unit/test-qga.c
@@ -52,7 +52,10 @@ fixture_setup(TestFixture *fixture, gconstpointer data, gchar **envp)
{
const gchar *extra_arg = data;
GError *error = NULL;
- gchar *cwd, *path, *cmd, **argv = NULL;
+ g_autofree char *cwd = NULL;
+ g_autofree char *path = NULL;
+ g_autofree char *cmd = NULL;
+ g_auto(GStrv) argv = NULL;
fixture->loop = g_main_loop_new(NULL, FALSE);
@@ -78,17 +81,12 @@ fixture_setup(TestFixture *fixture, gconstpointer data, gchar **envp)
fixture->fd = connect_qga(path);
g_assert_cmpint(fixture->fd, !=, -1);
-
- g_strfreev(argv);
- g_free(cmd);
- g_free(cwd);
- g_free(path);
}
static void
fixture_tear_down(TestFixture *fixture, gconstpointer data)
{
- gchar *tmp;
+ g_autofree char *tmp = NULL;
kill(fixture->pid, SIGTERM);
@@ -107,7 +105,6 @@ fixture_tear_down(TestFixture *fixture, gconstpointer data)
tmp = g_build_filename(fixture->test_dir, "sock", NULL);
g_unlink(tmp);
- g_free(tmp);
g_rmdir(fixture->test_dir);
g_free(fixture->test_dir);
@@ -122,7 +119,7 @@ static void qmp_assertion_message_error(const char *domain,
QDict *dict)
{
const char *class, *desc;
- char *s;
+ g_autofree char *s = NULL;
QDict *error;
error = qdict_get_qdict(dict, "error");
@@ -131,7 +128,6 @@ static void qmp_assertion_message_error(const char *domain,
s = g_strdup_printf("assertion failed %s: %s %s", expr, class, desc);
g_assertion_message(domain, file, line, func, s);
- g_free(s);
}
#define qmp_assert_no_error(err) do { \
@@ -146,7 +142,7 @@ static void test_qga_sync_delimited(gconstpointer fix)
const TestFixture *fixture = fix;
guint32 v, r = g_test_rand_int();
unsigned char c;
- QDict *ret;
+ g_autoptr(QDict) ret = NULL;
qmp_fd_send_raw(fixture->fd, "\xff");
qmp_fd_send(fixture->fd,
@@ -180,15 +176,13 @@ static void test_qga_sync_delimited(gconstpointer fix)
v = qdict_get_int(ret, "return");
g_assert_cmpint(r, ==, v);
-
- qobject_unref(ret);
}
static void test_qga_sync(gconstpointer fix)
{
const TestFixture *fixture = fix;
guint32 v, r = g_test_rand_int();
- QDict *ret;
+ g_autoptr(QDict) ret = NULL;
/*
* TODO guest-sync is inherently limited: we cannot distinguish
@@ -210,33 +204,27 @@ static void test_qga_sync(gconstpointer fix)
v = qdict_get_int(ret, "return");
g_assert_cmpint(r, ==, v);
-
- qobject_unref(ret);
}
static void test_qga_ping(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret;
+ g_autoptr(QDict) ret = NULL;
ret = qmp_fd(fixture->fd, "{'execute': 'guest-ping'}");
g_assert_nonnull(ret);
qmp_assert_no_error(ret);
-
- qobject_unref(ret);
}
static void test_qga_id(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret;
+ g_autoptr(QDict) ret = NULL;
ret = qmp_fd(fixture->fd, "{'execute': 'guest-ping', 'id': 1}");
g_assert_nonnull(ret);
qmp_assert_no_error(ret);
g_assert_cmpint(qdict_get_int(ret, "id"), ==, 1);
-
- qobject_unref(ret);
}
static void test_qga_invalid_oob(gconstpointer fix)
@@ -253,7 +241,8 @@ static void test_qga_invalid_oob(gconstpointer fix)
static void test_qga_invalid_args(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret, *error;
+ g_autoptr(QDict) ret = NULL;
+ QDict *error;
const gchar *class, *desc;
ret = qmp_fd(fixture->fd, "{'execute': 'guest-ping', "
@@ -266,14 +255,13 @@ static void test_qga_invalid_args(gconstpointer fix)
g_assert_cmpstr(class, ==, "GenericError");
g_assert_cmpstr(desc, ==, "Parameter 'foo' is unexpected");
-
- qobject_unref(ret);
}
static void test_qga_invalid_cmd(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret, *error;
+ g_autoptr(QDict) ret = NULL;
+ QDict *error;
const gchar *class, *desc;
ret = qmp_fd(fixture->fd, "{'execute': 'guest-invalid-cmd'}");
@@ -285,14 +273,13 @@ static void test_qga_invalid_cmd(gconstpointer fix)
g_assert_cmpstr(class, ==, "CommandNotFound");
g_assert_cmpint(strlen(desc), >, 0);
-
- qobject_unref(ret);
}
static void test_qga_info(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret, *val;
+ g_autoptr(QDict) ret = NULL;
+ QDict *val;
const gchar *version;
ret = qmp_fd(fixture->fd, "{'execute': 'guest-info'}");
@@ -302,14 +289,12 @@ static void test_qga_info(gconstpointer fix)
val = qdict_get_qdict(ret, "return");
version = qdict_get_try_str(val, "version");
g_assert_cmpstr(version, ==, QEMU_VERSION);
-
- qobject_unref(ret);
}
static void test_qga_get_vcpus(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret;
+ g_autoptr(QDict) ret = NULL;
QList *list;
const QListEntry *entry;
@@ -322,14 +307,12 @@ static void test_qga_get_vcpus(gconstpointer fix)
entry = qlist_first(list);
g_assert(qdict_haskey(qobject_to(QDict, entry->value), "online"));
g_assert(qdict_haskey(qobject_to(QDict, entry->value), "logical-id"));
-
- qobject_unref(ret);
}
static void test_qga_get_fsinfo(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret;
+ g_autoptr(QDict) ret = NULL;
QList *list;
const QListEntry *entry;
@@ -346,14 +329,13 @@ static void test_qga_get_fsinfo(gconstpointer fix)
g_assert(qdict_haskey(qobject_to(QDict, entry->value), "type"));
g_assert(qdict_haskey(qobject_to(QDict, entry->value), "disk"));
}
-
- qobject_unref(ret);
}
static void test_qga_get_memory_block_info(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret, *val;
+ g_autoptr(QDict) ret = NULL;
+ QDict *val;
int64_t size;
ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-memory-block-info'}");
@@ -366,14 +348,12 @@ static void test_qga_get_memory_block_info(gconstpointer fix)
size = qdict_get_int(val, "size");
g_assert_cmpint(size, >, 0);
}
-
- qobject_unref(ret);
}
static void test_qga_get_memory_blocks(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret;
+ g_autoptr(QDict) ret = NULL;
QList *list;
const QListEntry *entry;
@@ -391,14 +371,12 @@ static void test_qga_get_memory_blocks(gconstpointer fix)
g_assert(qdict_haskey(qobject_to(QDict, entry->value), "online"));
}
}
-
- qobject_unref(ret);
}
static void test_qga_network_get_interfaces(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret;
+ g_autoptr(QDict) ret = NULL;
QList *list;
const QListEntry *entry;
@@ -410,8 +388,6 @@ static void test_qga_network_get_interfaces(gconstpointer fix)
list = qdict_get_qlist(ret, "return");
entry = qlist_first(list);
g_assert(qdict_haskey(qobject_to(QDict, entry->value), "name"));
-
- qobject_unref(ret);
}
static void test_qga_file_ops(gconstpointer fix)
@@ -642,7 +618,7 @@ static void test_qga_file_write_read(gconstpointer fix)
static void test_qga_get_time(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret;
+ g_autoptr(QDict) ret = NULL;
int64_t time;
ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-time'}");
@@ -651,8 +627,6 @@ static void test_qga_get_time(gconstpointer fix)
time = qdict_get_int(ret, "return");
g_assert_cmpint(time, >, 0);
-
- qobject_unref(ret);
}
static void test_qga_blacklist(gconstpointer data)
@@ -693,18 +667,22 @@ static void test_qga_blacklist(gconstpointer data)
static void test_qga_config(gconstpointer data)
{
GError *error = NULL;
- char *cwd, *cmd, *out, *err, *str, **strv, **argv = NULL;
+ g_autofree char *out = NULL;
+ g_autofree char *err = NULL;
+ g_autofree char *cwd = NULL;
+ g_autofree char *cmd = NULL;
+ g_auto(GStrv) argv = NULL;
+ g_auto(GStrv) strv = NULL;
+ g_autoptr(GKeyFile) kf = NULL;
+ char *str;
char *env[2];
int status;
gsize n;
- GKeyFile *kf;
cwd = g_get_current_dir();
cmd = g_strdup_printf("%s%cqga%cqemu-ga -D",
cwd, G_DIR_SEPARATOR, G_DIR_SEPARATOR);
- g_free(cwd);
g_shell_parse_argv(cmd, NULL, &argv, &error);
- g_free(cmd);
g_assert_no_error(error);
env[0] = g_strdup_printf("QGA_CONF=tests%cdata%ctest-qga-config",
@@ -712,7 +690,6 @@ static void test_qga_config(gconstpointer data)
env[1] = NULL;
g_spawn_sync(NULL, argv, env, 0,
NULL, NULL, &out, &err, &status, &error);
- g_strfreev(argv);
g_assert_no_error(error);
g_assert_cmpstr(err, ==, "");
@@ -759,18 +736,14 @@ static void test_qga_config(gconstpointer data)
g_assert_true(g_strv_contains((const char * const *)strv,
"guest-get-time"));
g_assert_no_error(error);
- g_strfreev(strv);
- g_free(out);
- g_free(err);
g_free(env[0]);
- g_key_file_free(kf);
}
static void test_qga_fsfreeze_status(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret;
+ g_autoptr(QDict) ret = NULL;
const gchar *status;
ret = qmp_fd(fixture->fd, "{'execute': 'guest-fsfreeze-status'}");
@@ -779,16 +752,15 @@ static void test_qga_fsfreeze_status(gconstpointer fix)
status = qdict_get_try_str(ret, "return");
g_assert_cmpstr(status, ==, "thawed");
-
- qobject_unref(ret);
}
static void test_qga_guest_exec(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret, *val;
+ g_autoptr(QDict) ret = NULL;
+ QDict *val;
const gchar *out;
- guchar *decoded;
+ g_autofree guchar *decoded = NULL;
int64_t pid, now, exitcode;
gsize len;
bool exited;
@@ -827,14 +799,13 @@ static void test_qga_guest_exec(gconstpointer fix)
decoded = g_base64_decode(out, &len);
g_assert_cmpint(len, ==, 12);
g_assert_cmpstr((char *)decoded, ==, "\" test_str \"");
- g_free(decoded);
- qobject_unref(ret);
}
static void test_qga_guest_exec_invalid(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret, *error;
+ g_autoptr(QDict) ret = NULL;
+ QDict *error;
const gchar *class, *desc;
/* invalid command */
@@ -859,13 +830,13 @@ static void test_qga_guest_exec_invalid(gconstpointer fix)
desc = qdict_get_str(error, "desc");
g_assert_cmpstr(class, ==, "GenericError");
g_assert_cmpint(strlen(desc), >, 0);
- qobject_unref(ret);
}
static void test_qga_guest_get_host_name(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret, *val;
+ g_autoptr(QDict) ret = NULL;
+ QDict *val;
ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-host-name'}");
g_assert_nonnull(ret);
@@ -873,14 +844,13 @@ static void test_qga_guest_get_host_name(gconstpointer fix)
val = qdict_get_qdict(ret, "return");
g_assert(qdict_haskey(val, "host-name"));
-
- qobject_unref(ret);
}
static void test_qga_guest_get_timezone(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret, *val;
+ g_autoptr(QDict) ret = NULL;
+ QDict *val;
ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-timezone'}");
g_assert_nonnull(ret);
@@ -889,14 +859,12 @@ static void test_qga_guest_get_timezone(gconstpointer fix)
/* Make sure there's at least offset */
val = qdict_get_qdict(ret, "return");
g_assert(qdict_haskey(val, "offset"));
-
- qobject_unref(ret);
}
static void test_qga_guest_get_users(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret;
+ g_autoptr(QDict) ret = NULL;
QList *val;
ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-users'}");
@@ -906,15 +874,13 @@ static void test_qga_guest_get_users(gconstpointer fix)
/* There is not much to test here */
val = qdict_get_qlist(ret, "return");
g_assert_nonnull(val);
-
- qobject_unref(ret);
}
static void test_qga_guest_get_osinfo(gconstpointer data)
{
TestFixture fixture;
const gchar *str;
- QDict *ret = NULL;
+ g_autoptr(QDict) ret = NULL;
char *env[2];
QDict *val;
@@ -958,7 +924,6 @@ static void test_qga_guest_get_osinfo(gconstpointer data)
g_assert_nonnull(str);
g_assert_cmpstr(str, ==, "unit-test");
- qobject_unref(ret);
g_free(env[0]);
fixture_tear_down(&fixture, NULL);
}
--
2.36.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2022-05-25 15:17 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-25 14:41 [PATCH v5 00/15] Misc cleanups marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 01/15] include: move qemu_*_exec_dir() to cutils marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 02/15] util/win32: simplify qemu_get_local_state_dir() marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 03/15] tests: make libqmp buildable for win32 marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 04/15] qga: flatten safe_open_or_create() marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 05/15] qga: add qga_open_cloexec() helper marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 06/15] qga: use qga_open_cloexec() for safe_open_or_create() marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 07/15] qga: throw an Error in ga_channel_open() marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 08/15] qga: replace qemu_open_old() with qga_open_cloexec() marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 09/15] qga: make build_fs_mount_list() return a bool marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 10/15] test/qga: use G_TEST_DIR to locate os-release test file marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 11/15] qga/wixl: prefer variables over environment marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 12/15] qga/wixl: require Mingw_bin marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 13/15] qga/wixl: simplify some pre-processing marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 14/15] qga/wixl: replace QEMU_GA_MSI_MINGW_BIN_PATH with glib bindir marcandre.lureau
2022-05-25 14:41 ` [PATCH v5 15/15] test/qga: use g_auto wherever sensible marcandre.lureau
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.