* [Qemu-devel] [PULL v2 01/24] qga: Support Unicode paths in guest-file-open on win32
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
@ 2018-10-31 1:37 ` Michael Roth
2018-10-31 1:37 ` [Qemu-devel] [PULL v2 02/24] qga-win: add support for qmp_guest_fsfreeze_freeze_list Michael Roth
` (22 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:37 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Jonathon Reinhart
From: Jonathon Reinhart <jreinhart@cc-sw.com>
Currently, the win32 port of QEMU Guest Agent does not properly handle Unicode
paths. The JSON decoder produces a valid UTF-8 path string, but this is passed
directly to CreateFileA, which is expecting an ANSI string and not UTF-8. This
leads to mangled filenames.
This patch follows the example of qmp_guest_set_user_password() and uses
g_utf8_to_utf16() to convert the string to UTF-16 and calls CreateFileW()
explicitly.
Signed-off-by: Jonathon Reinhart <jreinhart@cc-sw.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/commands-win32.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 98d9735389..416343b97b 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -160,13 +160,15 @@ static void handle_set_nonblocking(HANDLE fh)
int64_t qmp_guest_file_open(const char *path, bool has_mode,
const char *mode, Error **errp)
{
- int64_t fd;
+ int64_t fd = -1;
HANDLE fh;
HANDLE templ_file = NULL;
DWORD share_mode = FILE_SHARE_READ;
DWORD flags_and_attr = FILE_ATTRIBUTE_NORMAL;
LPSECURITY_ATTRIBUTES sa_attr = NULL;
OpenFlags *guest_flags;
+ GError *gerr = NULL;
+ wchar_t *w_path = NULL;
if (!has_mode) {
mode = "r";
@@ -175,16 +177,21 @@ int64_t qmp_guest_file_open(const char *path, bool has_mode,
guest_flags = find_open_flag(mode);
if (guest_flags == NULL) {
error_setg(errp, "invalid file open mode");
- return -1;
+ goto done;
+ }
+
+ w_path = g_utf8_to_utf16(path, -1, NULL, NULL, &gerr);
+ if (!w_path) {
+ goto done;
}
- fh = CreateFile(path, guest_flags->desired_access, share_mode, sa_attr,
+ fh = CreateFileW(w_path, guest_flags->desired_access, share_mode, sa_attr,
guest_flags->creation_disposition, flags_and_attr,
templ_file);
if (fh == INVALID_HANDLE_VALUE) {
error_setg_win32(errp, GetLastError(), "failed to open file '%s'",
path);
- return -1;
+ goto done;
}
/* set fd non-blocking to avoid common use cases (like reading from a
@@ -196,10 +203,17 @@ int64_t qmp_guest_file_open(const char *path, bool has_mode,
if (fd < 0) {
CloseHandle(fh);
error_setg(errp, "failed to add handle to qmp handle table");
- return -1;
+ goto done;
}
slog("guest-file-open, handle: % " PRId64, fd);
+
+done:
+ if (gerr) {
+ error_setg(errp, QERR_QGA_COMMAND_FAILED, gerr->message);
+ g_error_free(gerr);
+ }
+ g_free(w_path);
return fd;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 02/24] qga-win: add support for qmp_guest_fsfreeze_freeze_list
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
2018-10-31 1:37 ` [Qemu-devel] [PULL v2 01/24] qga: Support Unicode paths in guest-file-open on win32 Michael Roth
@ 2018-10-31 1:37 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 03/24] qga: ignore non present cpus when handling qmp_guest_get_vcpus() Michael Roth
` (21 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:37 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Chen Hanxiao
From: Chen Hanxiao <chenhanxiao@gmail.com>
This patch add support for freeze specified fs.
The valid mountpoints list member are [1]:
The path of a mounted folder, for example, Y:\MountX\
A drive letter, for example, D:\
A volume GUID path of the form \\?\Volume{GUID}\,
where GUID identifies the volume
A UNC path that specifies a remote file share,
for example, \\Clusterx\Share1\
[1] https://docs.microsoft.com/en-us/windows/desktop/api/vsbackup/nf-vsbackup-ivssbackupcomponents-addtosnapshotset
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/commands-win32.c | 21 ++++-----
qga/main.c | 2 +-
qga/vss-win32.c | 5 +-
qga/vss-win32.h | 3 +-
qga/vss-win32/requester.cpp | 92 ++++++++++++++++++++++++++-----------
qga/vss-win32/requester.h | 13 ++++--
6 files changed, 91 insertions(+), 45 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 416343b97b..347577f2a4 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -790,6 +790,13 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp)
* The frozen state is limited for up to 10 seconds by VSS.
*/
int64_t qmp_guest_fsfreeze_freeze(Error **errp)
+{
+ return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
+}
+
+int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
+ strList *mountpoints,
+ Error **errp)
{
int i;
Error *local_err = NULL;
@@ -804,7 +811,7 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
/* cannot risk guest agent blocking itself on a write in this state */
ga_set_frozen(ga_state);
- qga_vss_fsfreeze(&i, true, &local_err);
+ qga_vss_fsfreeze(&i, true, mountpoints, &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto error;
@@ -822,15 +829,6 @@ error:
return 0;
}
-int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
- strList *mountpoints,
- Error **errp)
-{
- error_setg(errp, QERR_UNSUPPORTED);
-
- return 0;
-}
-
/*
* Thaw local file systems using Volume Shadow-copy Service.
*/
@@ -843,7 +841,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **errp)
return 0;
}
- qga_vss_fsfreeze(&i, false, errp);
+ qga_vss_fsfreeze(&i, false, NULL, errp);
ga_unset_frozen(ga_state);
return i;
@@ -1660,7 +1658,6 @@ GList *ga_command_blacklist_init(GList *blacklist)
"guest-set-vcpus",
"guest-get-memory-blocks", "guest-set-memory-blocks",
"guest-get-memory-block-size",
- "guest-fsfreeze-freeze-list",
NULL};
char **p = (char **)list_unsupported;
diff --git a/qga/main.c b/qga/main.c
index c399320d3c..afcd268ee3 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -151,7 +151,7 @@ static void quit_handler(int sig)
WaitForSingleObject(hEventTimeout, 0);
CloseHandle(hEventTimeout);
}
- qga_vss_fsfreeze(&i, false, &err);
+ qga_vss_fsfreeze(&i, false, NULL, &err);
if (err) {
g_debug("Error unfreezing filesystems prior to exiting: %s",
error_get_pretty(err));
diff --git a/qga/vss-win32.c b/qga/vss-win32.c
index a541f3ae01..f444a25a70 100644
--- a/qga/vss-win32.c
+++ b/qga/vss-win32.c
@@ -147,7 +147,8 @@ void ga_uninstall_vss_provider(void)
}
/* Call VSS requester and freeze/thaw filesystems and applications */
-void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error **errp)
+void qga_vss_fsfreeze(int *nr_volume, bool freeze,
+ strList *mountpoints, Error **errp)
{
const char *func_name = freeze ? "requester_freeze" : "requester_thaw";
QGAVSSRequesterFunc func;
@@ -164,5 +165,5 @@ void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error **errp)
return;
}
- func(nr_volume, &errset);
+ func(nr_volume, mountpoints, &errset);
}
diff --git a/qga/vss-win32.h b/qga/vss-win32.h
index 4f8e39aa5c..ce2abe5a72 100644
--- a/qga/vss-win32.h
+++ b/qga/vss-win32.h
@@ -22,6 +22,7 @@ bool vss_initialized(void);
int ga_install_vss_provider(void);
void ga_uninstall_vss_provider(void);
-void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error **errp);
+void qga_vss_fsfreeze(int *nr_volume, bool freeze,
+ strList *mountpints, Error **errp);
#endif
diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index 3d9c9716c0..5378c55d23 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -234,7 +234,7 @@ out:
}
}
-void requester_freeze(int *num_vols, ErrorSet *errset)
+void requester_freeze(int *num_vols, void *mountpoints, ErrorSet *errset)
{
COMPointer<IVssAsync> pAsync;
HANDLE volume;
@@ -246,6 +246,7 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
WCHAR short_volume_name[64], *display_name = short_volume_name;
DWORD wait_status;
int num_fixed_drives = 0, i;
+ int num_mount_points = 0;
if (vss_ctx.pVssbc) { /* already frozen */
*num_vols = 0;
@@ -337,39 +338,73 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
goto out;
}
- volume = FindFirstVolumeW(short_volume_name, sizeof(short_volume_name));
- if (volume == INVALID_HANDLE_VALUE) {
- err_set(errset, hr, "failed to find first volume");
- goto out;
- }
- for (;;) {
- if (GetDriveTypeW(short_volume_name) == DRIVE_FIXED) {
+ if (mountpoints) {
+ PWCHAR volume_name_wchar;
+ for (volList *list = (volList *)mountpoints; list; list = list->next) {
+ size_t len = strlen(list->value) + 1;
+ size_t converted = 0;
VSS_ID pid;
- hr = vss_ctx.pVssbc->AddToSnapshotSet(short_volume_name,
+
+ volume_name_wchar = new wchar_t[len];
+ mbstowcs_s(&converted, volume_name_wchar, len,
+ list->value, _TRUNCATE);
+
+ hr = vss_ctx.pVssbc->AddToSnapshotSet(volume_name_wchar,
g_gProviderId, &pid);
if (FAILED(hr)) {
- WCHAR volume_path_name[PATH_MAX];
- if (GetVolumePathNamesForVolumeNameW(
- short_volume_name, volume_path_name,
- sizeof(volume_path_name), NULL) && *volume_path_name) {
- display_name = volume_path_name;
- }
err_set(errset, hr, "failed to add %S to snapshot set",
- display_name);
- FindVolumeClose(volume);
+ volume_name_wchar);
+ delete volume_name_wchar;
goto out;
}
- num_fixed_drives++;
+ num_mount_points++;
+
+ delete volume_name_wchar;
}
- if (!FindNextVolumeW(volume, short_volume_name,
- sizeof(short_volume_name))) {
- FindVolumeClose(volume);
- break;
+
+ if (num_mount_points == 0) {
+ /* If there is no valid mount points, just exit. */
+ goto out;
}
}
- if (num_fixed_drives == 0) {
- goto out; /* If there is no fixed drive, just exit. */
+ if (!mountpoints) {
+ volume = FindFirstVolumeW(short_volume_name, sizeof(short_volume_name));
+ if (volume == INVALID_HANDLE_VALUE) {
+ err_set(errset, hr, "failed to find first volume");
+ goto out;
+ }
+
+ for (;;) {
+ if (GetDriveTypeW(short_volume_name) == DRIVE_FIXED) {
+ VSS_ID pid;
+ hr = vss_ctx.pVssbc->AddToSnapshotSet(short_volume_name,
+ g_gProviderId, &pid);
+ if (FAILED(hr)) {
+ WCHAR volume_path_name[PATH_MAX];
+ if (GetVolumePathNamesForVolumeNameW(
+ short_volume_name, volume_path_name,
+ sizeof(volume_path_name), NULL) &&
+ *volume_path_name) {
+ display_name = volume_path_name;
+ }
+ err_set(errset, hr, "failed to add %S to snapshot set",
+ display_name);
+ FindVolumeClose(volume);
+ goto out;
+ }
+ num_fixed_drives++;
+ }
+ if (!FindNextVolumeW(volume, short_volume_name,
+ sizeof(short_volume_name))) {
+ FindVolumeClose(volume);
+ break;
+ }
+ }
+
+ if (num_fixed_drives == 0) {
+ goto out; /* If there is no fixed drive, just exit. */
+ }
}
hr = vss_ctx.pVssbc->PrepareForBackup(pAsync.replace());
@@ -435,7 +470,12 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
goto out;
}
- *num_vols = vss_ctx.cFrozenVols = num_fixed_drives;
+ if (mountpoints) {
+ *num_vols = vss_ctx.cFrozenVols = num_mount_points;
+ } else {
+ *num_vols = vss_ctx.cFrozenVols = num_fixed_drives;
+ }
+
return;
out:
@@ -449,7 +489,7 @@ out1:
}
-void requester_thaw(int *num_vols, ErrorSet *errset)
+void requester_thaw(int *num_vols, void *mountpints, ErrorSet *errset)
{
COMPointer<IVssAsync> pAsync;
diff --git a/qga/vss-win32/requester.h b/qga/vss-win32/requester.h
index 2a39d734a2..5a8e8faf0c 100644
--- a/qga/vss-win32/requester.h
+++ b/qga/vss-win32/requester.h
@@ -34,9 +34,16 @@ typedef struct ErrorSet {
STDAPI requester_init(void);
STDAPI requester_deinit(void);
-typedef void (*QGAVSSRequesterFunc)(int *, ErrorSet *);
-void requester_freeze(int *num_vols, ErrorSet *errset);
-void requester_thaw(int *num_vols, ErrorSet *errset);
+typedef struct volList volList;
+
+struct volList {
+ volList *next;
+ char *value;
+};
+
+typedef void (*QGAVSSRequesterFunc)(int *, void *, ErrorSet *);
+void requester_freeze(int *num_vols, void *volList, ErrorSet *errset);
+void requester_thaw(int *num_vols, void *volList, ErrorSet *errset);
#ifdef __cplusplus
}
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 03/24] qga: ignore non present cpus when handling qmp_guest_get_vcpus()
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
2018-10-31 1:37 ` [Qemu-devel] [PULL v2 01/24] qga: Support Unicode paths in guest-file-open on win32 Michael Roth
2018-10-31 1:37 ` [Qemu-devel] [PULL v2 02/24] qga-win: add support for qmp_guest_fsfreeze_freeze_list Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 04/24] configure: add test for libudev Michael Roth
` (20 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Igor Mammedov
From: Igor Mammedov <imammedo@redhat.com>
If VM has VCPUs plugged sparselly (for example a VM started with
3 VCPUs (cpu0, cpu1 and cpu2) and then cpu1 was hotunplugged so
only cpu0 and cpu2 are present), QGA will rise a error
error: internal error: unable to execute QEMU agent command 'guest-get-vcpus':
open("/sys/devices/system/cpu/cpu1/"): No such file or directory
when
virsh vcpucount FOO --guest
is executed.
Fix it by ignoring non present CPUs when fetching CPUs status from sysfs.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/commands-posix.c | 115 ++++++++++++++++++++++---------------------
1 file changed, 59 insertions(+), 56 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 37e8a2d791..42d30f078e 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2035,61 +2035,56 @@ static long sysconf_exact(int name, const char *name_str, Error **errp)
* Written members remain unmodified on error.
*/
static void transfer_vcpu(GuestLogicalProcessor *vcpu, bool sys2vcpu,
- Error **errp)
+ char *dirpath, Error **errp)
{
- char *dirpath;
+ int fd;
+ int res;
int dirfd;
+ static const char fn[] = "online";
- dirpath = g_strdup_printf("/sys/devices/system/cpu/cpu%" PRId64 "/",
- vcpu->logical_id);
dirfd = open(dirpath, O_RDONLY | O_DIRECTORY);
if (dirfd == -1) {
error_setg_errno(errp, errno, "open(\"%s\")", dirpath);
- } else {
- static const char fn[] = "online";
- int fd;
- int res;
-
- fd = openat(dirfd, fn, sys2vcpu ? O_RDONLY : O_RDWR);
- if (fd == -1) {
- if (errno != ENOENT) {
- error_setg_errno(errp, errno, "open(\"%s/%s\")", dirpath, fn);
- } else if (sys2vcpu) {
- vcpu->online = true;
- vcpu->can_offline = false;
- } else if (!vcpu->online) {
- error_setg(errp, "logical processor #%" PRId64 " can't be "
- "offlined", vcpu->logical_id);
- } /* otherwise pretend successful re-onlining */
- } else {
- unsigned char status;
-
- res = pread(fd, &status, 1, 0);
- if (res == -1) {
- error_setg_errno(errp, errno, "pread(\"%s/%s\")", dirpath, fn);
- } else if (res == 0) {
- error_setg(errp, "pread(\"%s/%s\"): unexpected EOF", dirpath,
- fn);
- } else if (sys2vcpu) {
- vcpu->online = (status != '0');
- vcpu->can_offline = true;
- } else if (vcpu->online != (status != '0')) {
- status = '0' + vcpu->online;
- if (pwrite(fd, &status, 1, 0) == -1) {
- error_setg_errno(errp, errno, "pwrite(\"%s/%s\")", dirpath,
- fn);
- }
- } /* otherwise pretend successful re-(on|off)-lining */
+ return;
+ }
- res = close(fd);
- g_assert(res == 0);
- }
+ fd = openat(dirfd, fn, sys2vcpu ? O_RDONLY : O_RDWR);
+ if (fd == -1) {
+ if (errno != ENOENT) {
+ error_setg_errno(errp, errno, "open(\"%s/%s\")", dirpath, fn);
+ } else if (sys2vcpu) {
+ vcpu->online = true;
+ vcpu->can_offline = false;
+ } else if (!vcpu->online) {
+ error_setg(errp, "logical processor #%" PRId64 " can't be "
+ "offlined", vcpu->logical_id);
+ } /* otherwise pretend successful re-onlining */
+ } else {
+ unsigned char status;
+
+ res = pread(fd, &status, 1, 0);
+ if (res == -1) {
+ error_setg_errno(errp, errno, "pread(\"%s/%s\")", dirpath, fn);
+ } else if (res == 0) {
+ error_setg(errp, "pread(\"%s/%s\"): unexpected EOF", dirpath,
+ fn);
+ } else if (sys2vcpu) {
+ vcpu->online = (status != '0');
+ vcpu->can_offline = true;
+ } else if (vcpu->online != (status != '0')) {
+ status = '0' + vcpu->online;
+ if (pwrite(fd, &status, 1, 0) == -1) {
+ error_setg_errno(errp, errno, "pwrite(\"%s/%s\")", dirpath,
+ fn);
+ }
+ } /* otherwise pretend successful re-(on|off)-lining */
- res = close(dirfd);
+ res = close(fd);
g_assert(res == 0);
}
- g_free(dirpath);
+ res = close(dirfd);
+ g_assert(res == 0);
}
GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
@@ -2107,17 +2102,21 @@ GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
while (local_err == NULL && current < sc_max) {
GuestLogicalProcessor *vcpu;
GuestLogicalProcessorList *entry;
-
- vcpu = g_malloc0(sizeof *vcpu);
- vcpu->logical_id = current++;
- vcpu->has_can_offline = true; /* lolspeak ftw */
- transfer_vcpu(vcpu, true, &local_err);
-
- entry = g_malloc0(sizeof *entry);
- entry->value = vcpu;
-
- *link = entry;
- link = &entry->next;
+ int64_t id = current++;
+ char *path = g_strdup_printf("/sys/devices/system/cpu/cpu%" PRId64 "/",
+ id);
+
+ if (g_file_test(path, G_FILE_TEST_EXISTS)) {
+ vcpu = g_malloc0(sizeof *vcpu);
+ vcpu->logical_id = id;
+ vcpu->has_can_offline = true; /* lolspeak ftw */
+ transfer_vcpu(vcpu, true, path, &local_err);
+ entry = g_malloc0(sizeof *entry);
+ entry->value = vcpu;
+ *link = entry;
+ link = &entry->next;
+ }
+ g_free(path);
}
if (local_err == NULL) {
@@ -2138,7 +2137,11 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
processed = 0;
while (vcpus != NULL) {
- transfer_vcpu(vcpus->value, false, &local_err);
+ char *path = g_strdup_printf("/sys/devices/system/cpu/cpu%" PRId64 "/",
+ vcpus->value->logical_id);
+
+ transfer_vcpu(vcpus->value, false, path, &local_err);
+ g_free(path);
if (local_err != NULL) {
break;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 04/24] configure: add test for libudev
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (2 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 03/24] qga: ignore non present cpus when handling qmp_guest_get_vcpus() Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 05/24] qga: linux: report disk serial number Michael Roth
` (19 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Tomáš Golembiovský
From: Tomáš Golembiovský <tgolembi@redhat.com>
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
*make libudev optional to avoid breaking existing build/test environments
*disable libudev for --static builds
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
configure | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/configure b/configure
index a898e21c68..406f48f46e 100755
--- a/configure
+++ b/configure
@@ -474,6 +474,7 @@ libxml2=""
docker="no"
debug_mutex="no"
libpmem=""
+libudev="no"
# cross compilers defaults, can be overridden with --cross-cc-ARCH
cross_cc_aarch64="aarch64-linux-gnu-gcc"
@@ -870,6 +871,7 @@ Linux)
vhost_vsock="yes"
QEMU_INCLUDES="-I\$(SRC_PATH)/linux-headers -I$(pwd)/linux-headers $QEMU_INCLUDES"
supported_os="yes"
+ libudev="yes"
;;
esac
@@ -5609,6 +5611,19 @@ if test "$libnfs" != "no" ; then
fi
fi
+##########################################
+# Do we have libudev
+if test "$libudev" != "no" ; then
+ if $pkg_config libudev && test "$static" != "yes"; then
+ if test "$static" != "yes"; then
+ libudev="yes"
+ libudev_libs=$($pkg_config --libs libudev)
+ fi
+ else
+ libudev="no"
+ fi
+fi
+
# Now we've finished running tests it's OK to add -Werror to the compiler flags
if test "$werror" = "yes"; then
QEMU_CFLAGS="-Werror $QEMU_CFLAGS"
@@ -6033,6 +6048,7 @@ echo "VxHS block device $vxhs"
echo "capstone $capstone"
echo "docker $docker"
echo "libpmem support $libpmem"
+echo "libudev $libudev"
if test "$sdl_too_old" = "yes"; then
echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -6868,6 +6884,11 @@ if test "$docker" != "no"; then
echo "HAVE_USER_DOCKER=y" >> $config_host_mak
fi
+if test "$libudev" != "no"; then
+ echo "CONFIG_LIBUDEV=y" >> $config_host_mak
+ echo "LIBUDEV_LIBS=$libudev_libs" >> $config_host_mak
+fi
+
# use included Linux headers
if test "$linux" = "yes" ; then
mkdir -p linux-headers
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 05/24] qga: linux: report disk serial number
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (3 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 04/24] configure: add test for libudev Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 06/24] qga: linux: return disk device in guest-get-fsinfo Michael Roth
` (18 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Tomáš Golembiovský
From: Tomáš Golembiovský <tgolembi@redhat.com>
Add reporting of disk serial number on Linux guests. The feature depends
on libudev.
Example:
{
"name": "dm-2",
"mountpoint": "/",
...
"disk": [
{
"serial": "SAMSUNG_MZ7LN512HCHP-000L1_S1ZKNXAG822493",
...
}
],
}
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/Makefile.objs | 1 +
qga/commands-posix.c | 32 ++++++++++++++++++++++++++++++--
qga/qapi-schema.json | 4 +++-
3 files changed, 34 insertions(+), 3 deletions(-)
diff --git a/qga/Makefile.objs b/qga/Makefile.objs
index ed08c5917c..80e6bb3c2e 100644
--- a/qga/Makefile.objs
+++ b/qga/Makefile.objs
@@ -1,3 +1,4 @@
+commands-posix.o-libs := $(LIBUDEV_LIBS)
qga-obj-y = commands.o guest-agent-command-state.o main.o
qga-obj-$(CONFIG_POSIX) += commands-posix.o channel-posix.o
qga-obj-$(CONFIG_WIN32) += commands-win32.o channel-win32.o service-win32.o
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 42d30f078e..41bd11b2b5 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -48,6 +48,10 @@ extern char **environ;
#include <net/if.h>
#include <sys/statvfs.h>
+#ifdef CONFIG_LIBUDEV
+#include <libudev.h>
+#endif
+
#ifdef FIFREEZE
#define CONFIG_FSFREEZE
#endif
@@ -872,6 +876,10 @@ static void build_guest_fsinfo_for_real_device(char const *syspath,
GuestDiskAddressList *list = NULL;
bool has_ata = false, has_host = false, has_tgt = false;
char *p, *q, *driver = NULL;
+#ifdef CONFIG_LIBUDEV
+ struct udev *udev = NULL;
+ struct udev_device *udevice = NULL;
+#endif
p = strstr(syspath, "/devices/pci");
if (!p || sscanf(p + 12, "%*x:%*x/%x:%x:%x.%x%n",
@@ -936,6 +944,21 @@ static void build_guest_fsinfo_for_real_device(char const *syspath,
list = g_malloc0(sizeof(*list));
list->value = disk;
+#ifdef CONFIG_LIBUDEV
+ udev = udev_new();
+ udevice = udev_device_new_from_syspath(udev, syspath);
+ if (udev == NULL || udevice == NULL) {
+ g_debug("failed to query udev");
+ } else {
+ const char *serial;
+ serial = udev_device_get_property_value(udevice, "ID_SERIAL");
+ if (serial != NULL && *serial != 0) {
+ disk->serial = g_strdup(serial);
+ disk->has_serial = true;
+ }
+ }
+#endif
+
if (strcmp(driver, "ata_piix") == 0) {
/* a host per ide bus, target*:0:<unit>:0 */
if (!has_host || !has_tgt) {
@@ -995,14 +1018,19 @@ static void build_guest_fsinfo_for_real_device(char const *syspath,
list->next = fs->disk;
fs->disk = list;
- g_free(driver);
- return;
+ goto out;
cleanup:
if (list) {
qapi_free_GuestDiskAddressList(list);
}
+out:
g_free(driver);
+#ifdef CONFIG_LIBUDEV
+ udev_unref(udev);
+ udev_device_unref(udevice);
+#endif
+ return;
}
static void build_guest_fsinfo_for_device(char const *devpath,
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index dfbc4a5e32..3bcda6257e 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -834,13 +834,15 @@
# @bus: bus id
# @target: target id
# @unit: unit id
+# @serial: serial number (since: 3.1)
#
# Since: 2.2
##
{ 'struct': 'GuestDiskAddress',
'data': {'pci-controller': 'GuestPCIAddress',
'bus-type': 'GuestDiskBusType',
- 'bus': 'int', 'target': 'int', 'unit': 'int'} }
+ 'bus': 'int', 'target': 'int', 'unit': 'int',
+ '*serial': 'str'} }
##
# @GuestFilesystemInfo:
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 06/24] qga: linux: return disk device in guest-get-fsinfo
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (4 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 05/24] qga: linux: report disk serial number Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 07/24] qga-win: prevent crash when executing fsinfo command Michael Roth
` (17 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Tomáš Golembiovský
From: Tomáš Golembiovský <tgolembi@redhat.com>
Report device node of the disk on Linux (e.g. "/dev/sda2").
Requirs libudev.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/commands-posix.c | 7 ++++++-
qga/qapi-schema.json | 3 ++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 41bd11b2b5..1877976522 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -950,7 +950,12 @@ static void build_guest_fsinfo_for_real_device(char const *syspath,
if (udev == NULL || udevice == NULL) {
g_debug("failed to query udev");
} else {
- const char *serial;
+ const char *devnode, *serial;
+ devnode = udev_device_get_devnode(udevice);
+ if (devnode != NULL) {
+ disk->dev = g_strdup(devnode);
+ disk->has_dev = true;
+ }
serial = udev_device_get_property_value(udevice, "ID_SERIAL");
if (serial != NULL && *serial != 0) {
disk->serial = g_strdup(serial);
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 3bcda6257e..c6725b3ec8 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -835,6 +835,7 @@
# @target: target id
# @unit: unit id
# @serial: serial number (since: 3.1)
+# @dev: device node (POSIX) or device UNC (Windows) (since: 3.1)
#
# Since: 2.2
##
@@ -842,7 +843,7 @@
'data': {'pci-controller': 'GuestPCIAddress',
'bus-type': 'GuestDiskBusType',
'bus': 'int', 'target': 'int', 'unit': 'int',
- '*serial': 'str'} }
+ '*serial': 'str', '*dev': 'str'} }
##
# @GuestFilesystemInfo:
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 07/24] qga-win: prevent crash when executing fsinfo command
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (5 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 06/24] qga: linux: return disk device in guest-get-fsinfo Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 08/24] qga-win: fsinfo: pci-info: allow partial info Michael Roth
` (16 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, Sameeh Jubran, Tomáš Golembiovský
From: Sameeh Jubran <sjubran@redhat.com>
The fsinfo command is currently implemented for Windows only and it's disk
parameter can be enabled by adding the define "CONFIG_QGA_NTDDSCSI" to the qga
code. When enabled and executed the qemu-ga crashed with the following message:
------------------------------------------------
File qapi/qapi-visit-core.c, Line 49
Expression: !(v->type & VISITOR_OUTPUT) || *obj)
------------------------------------------------
After some digging, turns out that the GuestPCIAddress is null and the
qapi visitor doesn't like that, so we can always allocate it instead and
initiate all it's members to -1.
Signed-off-by: Sameeh Jubran <sjubran@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/commands-win32.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 347577f2a4..f0e6f6128b 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -499,6 +499,11 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
char *buffer = NULL;
GuestPCIAddress *pci = NULL;
char *name = g_strdup(&guid[4]);
+ pci = g_malloc0(sizeof(*pci));
+ pci->domain = -1;
+ pci->slot = -1;
+ pci->function = -1;
+ pci->bus = -1;
if (!QueryDosDevice(name, dev_name, ARRAY_SIZE(dev_name))) {
error_setg_win32(errp, GetLastError(), "failed to get dos device name");
@@ -570,7 +575,6 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
func = addr & 0x0000FFFF;
dev = (addr >> 16) & 0x0000FFFF;
- pci = g_malloc0(sizeof(*pci));
pci->domain = dev;
pci->slot = slot;
pci->function = func;
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 08/24] qga-win: fsinfo: pci-info: allow partial info
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (6 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 07/24] qga-win: prevent crash when executing fsinfo command Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 09/24] build: rename CONFIG_QGA_NTDDDISK to CONFIG_QGA_NTDDSCSI Michael Roth
` (15 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, Sameeh Jubran, Tomáš Golembiovský
From: Sameeh Jubran <sjubran@redhat.com>
The call to SetupDiGetDeviceRegistryProperty might fail because the
value doesn't exist in the registry, in this case we shouldn't exit from
the loop but instead continue to look for other available values in the
registry and set this value as unavailable (-1).
Signed-off-by: Sameeh Jubran <sjubran@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
*squash in fix for when get_pci_info() returns NULL pci_controller field
*fix handling for error_set() cases in get_pci_info(), not just NULL return
*force all -1 PCI addr fields if any single one of them isn't found
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/commands-win32.c | 46 +++++++++++++++++++++++++++++++-------------
1 file changed, 33 insertions(+), 13 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index f0e6f6128b..4fe1517778 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -499,6 +499,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
char *buffer = NULL;
GuestPCIAddress *pci = NULL;
char *name = g_strdup(&guid[4]);
+ bool partial_pci = false;
pci = g_malloc0(sizeof(*pci));
pci->domain = -1;
pci->slot = -1;
@@ -519,7 +520,8 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
dev_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
for (i = 0; SetupDiEnumDeviceInfo(dev_info, i, &dev_info_data); i++) {
- DWORD addr, bus, slot, func, dev, data, size2;
+ DWORD addr, bus, slot, data, size2;
+ int func, dev;
while (!SetupDiGetDeviceRegistryProperty(dev_info, &dev_info_data,
SPDRP_PHYSICAL_DEVICE_OBJECT_NAME,
&data, (PBYTE)buffer, size,
@@ -549,21 +551,24 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
*/
if (!SetupDiGetDeviceRegistryProperty(dev_info, &dev_info_data,
SPDRP_BUSNUMBER, &data, (PBYTE)&bus, size, NULL)) {
- break;
+ bus = -1;
+ partial_pci = true;
}
/* The function retrieves the device's address. This value will be
* transformed into device function and number */
if (!SetupDiGetDeviceRegistryProperty(dev_info, &dev_info_data,
SPDRP_ADDRESS, &data, (PBYTE)&addr, size, NULL)) {
- break;
+ addr = -1;
+ partial_pci = true;
}
/* This call returns UINumber of DEVICE_CAPABILITIES structure.
* This number is typically a user-perceived slot number. */
if (!SetupDiGetDeviceRegistryProperty(dev_info, &dev_info_data,
SPDRP_UI_NUMBER, &data, (PBYTE)&slot, size, NULL)) {
- break;
+ slot = -1;
+ partial_pci = true;
}
/* SetupApi gives us the same information as driver with
@@ -573,12 +578,19 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
* DeviceNumber = (USHORT)(((propertyAddress) >> 16) & 0x0000FFFF);
* SPDRP_ADDRESS is propertyAddress, so we do the same.*/
- func = addr & 0x0000FFFF;
- dev = (addr >> 16) & 0x0000FFFF;
- pci->domain = dev;
- pci->slot = slot;
- pci->function = func;
- pci->bus = bus;
+ if (partial_pci) {
+ pci->domain = -1;
+ pci->slot = -1;
+ pci->function = -1;
+ pci->bus = -1;
+ } else {
+ func = ((int) addr == -1) ? -1 : addr & 0x0000FFFF;
+ dev = ((int) addr == -1) ? -1 : (addr >> 16) & 0x0000FFFF;
+ pci->domain = dev;
+ pci->slot = (int) slot;
+ pci->function = func;
+ pci->bus = (int) bus;
+ }
break;
}
@@ -622,6 +634,7 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
DWORD len;
int bus;
HANDLE vol_h;
+ Error *local_err = NULL;
scsi_ad = &addr;
char *name = g_strndup(guid, strlen(guid)-1);
@@ -640,6 +653,16 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
disk = g_malloc0(sizeof(*disk));
disk->bus_type = find_bus_type(bus);
+ /* always set pci_controller as required by schema. get_pci_info() should
+ * report -1 values for non-PCI buses rather than fail. fail the command
+ * if that doesn't hold since that suggests some other unexpected
+ * breakage
+ */
+ disk->pci_controller = get_pci_info(name, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ goto out_close;
+ }
if (bus == BusTypeScsi || bus == BusTypeAta || bus == BusTypeRAID
#if (_WIN32_WINNT >= 0x0600)
/* This bus type is not supported before Windows Server 2003 SP1 */
@@ -654,12 +677,9 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
disk->unit = addr.Lun;
disk->target = addr.TargetId;
disk->bus = addr.PathId;
- disk->pci_controller = get_pci_info(name, errp);
}
/* We do not set error in this case, because we still have enough
* information about volume. */
- } else {
- disk->pci_controller = NULL;
}
list = g_malloc0(sizeof(*list));
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 09/24] build: rename CONFIG_QGA_NTDDDISK to CONFIG_QGA_NTDDSCSI
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (7 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 08/24] qga-win: fsinfo: pci-info: allow partial info Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 10/24] qga-win: add debugging information Michael Roth
` (14 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Tomáš Golembiovský
From: Tomáš Golembiovský <tgolembi@redhat.com>
There was inconsistency between commits:
50cbebb9a3 configure: add configure check for ntdddisk.h
a3ef3b2272 qga: added bus type and disk location path
The first commit added #define CONFIG_QGA_NTDDDISK but the second commit
expected the name to be CONFIG_QGA_NTDDSCSI. As a result the code in
second patch was never used.
Renaming the option to CONFIG_QGA_NTDDSCSI to match the name of header
file that is being checked for.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Sameeh Jubran <sjubran@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
configure | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure b/configure
index 406f48f46e..081a3069e3 100755
--- a/configure
+++ b/configure
@@ -6144,7 +6144,7 @@ if test "$mingw32" = "yes" ; then
echo "WIN_SDK=\"$win_sdk\"" >> $config_host_mak
fi
if test "$guest_agent_ntddscsi" = "yes" ; then
- echo "CONFIG_QGA_NTDDDISK=y" >> $config_host_mak
+ echo "CONFIG_QGA_NTDDSCSI=y" >> $config_host_mak
fi
if test "$guest_agent_msi" = "yes"; then
echo "QEMU_GA_MSI_ENABLED=yes" >> $config_host_mak
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 10/24] qga-win: add debugging information
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (8 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 09/24] build: rename CONFIG_QGA_NTDDDISK to CONFIG_QGA_NTDDSCSI Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 11/24] qga-win: refactor disk properties (bus) Michael Roth
` (13 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Tomáš Golembiovský
The windows code generaly lacks debug information (compared to posix
code). This patch adds some related to HW info in guest-get-fsinfo
command.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/commands-win32.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 4fe1517778..1a21aac5ad 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -89,6 +89,12 @@ static OpenFlags guest_file_open_modes[] = {
{"a+b", FILE_GENERIC_APPEND|GENERIC_READ, OPEN_ALWAYS }
};
+#define debug_error(msg) do { \
+ char *suffix = g_win32_error_message(GetLastError()); \
+ g_debug("%s: %s", (msg), suffix); \
+ g_free(suffix); \
+} while (0)
+
static OpenFlags *find_open_flag(const char *mode_str)
{
int mode;
@@ -518,6 +524,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
goto out;
}
+ g_debug("enumerating devices");
dev_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
for (i = 0; SetupDiEnumDeviceInfo(dev_info, i, &dev_info_data); i++) {
DWORD addr, bus, slot, data, size2;
@@ -543,6 +550,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
if (g_strcmp0(buffer, dev_name)) {
continue;
}
+ g_debug("found device %s", dev_name);
/* There is no need to allocate buffer in the next functions. The size
* is known and ULONG according to
@@ -551,6 +559,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
*/
if (!SetupDiGetDeviceRegistryProperty(dev_info, &dev_info_data,
SPDRP_BUSNUMBER, &data, (PBYTE)&bus, size, NULL)) {
+ debug_error("failed to get bus");
bus = -1;
partial_pci = true;
}
@@ -559,6 +568,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
* transformed into device function and number */
if (!SetupDiGetDeviceRegistryProperty(dev_info, &dev_info_data,
SPDRP_ADDRESS, &data, (PBYTE)&addr, size, NULL)) {
+ debug_error("failed to get address");
addr = -1;
partial_pci = true;
}
@@ -567,6 +577,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
* This number is typically a user-perceived slot number. */
if (!SetupDiGetDeviceRegistryProperty(dev_info, &dev_info_data,
SPDRP_UI_NUMBER, &data, (PBYTE)&slot, size, NULL)) {
+ debug_error("failed to get slot");
slot = -1;
partial_pci = true;
}
@@ -639,6 +650,7 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
scsi_ad = &addr;
char *name = g_strndup(guid, strlen(guid)-1);
+ g_debug("getting disk info for: %s", name);
vol_h = CreateFile(name, 0, FILE_SHARE_READ, NULL, OPEN_EXISTING,
0, NULL);
if (vol_h == INVALID_HANDLE_VALUE) {
@@ -646,6 +658,7 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
goto out_free;
}
+ g_debug("getting bus type");
bus = get_disk_bus_type(vol_h, errp);
if (bus < 0) {
goto out_close;
@@ -653,6 +666,7 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
disk = g_malloc0(sizeof(*disk));
disk->bus_type = find_bus_type(bus);
+ g_debug("bus type %d", disk->bus_type);
/* always set pci_controller as required by schema. get_pci_info() should
* report -1 values for non-PCI buses rather than fail. fail the command
* if that doesn't hold since that suggests some other unexpected
@@ -672,6 +686,7 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
/* We are able to use the same ioctls for different bus types
* according to Microsoft docs
* https://technet.microsoft.com/en-us/library/ee851589(v=ws.10).aspx */
+ g_debug("getting pci-controller info");
if (DeviceIoControl(vol_h, IOCTL_SCSI_GET_ADDRESS, NULL, 0, scsi_ad,
sizeof(SCSI_ADDRESS), &len, NULL)) {
disk->unit = addr.Lun;
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 11/24] qga-win: refactor disk properties (bus)
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (9 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 10/24] qga-win: add debugging information Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 12/24] qga-win: report disk serial number Michael Roth
` (12 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Tomáš Golembiovský
From: Tomáš Golembiovský <tgolembi@redhat.com>
Refactor code that queries bus type to be more generic. The function
get_disk_bus_type() has been renamed to build_guest_disk_info().
Following commit(s) will extend this function.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/commands-win32.c | 45 ++++++++++++++++++++++++++------------------
1 file changed, 27 insertions(+), 18 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 1a21aac5ad..1e91aa2343 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -613,25 +613,28 @@ out:
return pci;
}
-static int get_disk_bus_type(HANDLE vol_h, Error **errp)
+static void get_disk_properties(HANDLE vol_h, GuestDiskAddress *disk,
+ Error **errp)
{
STORAGE_PROPERTY_QUERY query;
STORAGE_DEVICE_DESCRIPTOR *dev_desc, buf;
DWORD received;
+ ULONG size = sizeof(buf);
dev_desc = &buf;
- dev_desc->Size = sizeof(buf);
query.PropertyId = StorageDeviceProperty;
query.QueryType = PropertyStandardQuery;
if (!DeviceIoControl(vol_h, IOCTL_STORAGE_QUERY_PROPERTY, &query,
sizeof(STORAGE_PROPERTY_QUERY), dev_desc,
- dev_desc->Size, &received, NULL)) {
+ size, &received, NULL)) {
error_setg_win32(errp, GetLastError(), "failed to get bus type");
- return -1;
+ return;
}
+ disk->bus_type = find_bus_type(dev_desc->BusType);
+ g_debug("bus type %d", disk->bus_type);
- return dev_desc->BusType;
+ return;
}
/* VSS provider works with volumes, thus there is no difference if
@@ -643,7 +646,6 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
GuestDiskAddress *disk;
SCSI_ADDRESS addr, *scsi_ad;
DWORD len;
- int bus;
HANDLE vol_h;
Error *local_err = NULL;
@@ -655,17 +657,16 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
0, NULL);
if (vol_h == INVALID_HANDLE_VALUE) {
error_setg_win32(errp, GetLastError(), "failed to open volume");
- goto out_free;
+ goto err;
}
- g_debug("getting bus type");
- bus = get_disk_bus_type(vol_h, errp);
- if (bus < 0) {
- goto out_close;
+ disk = g_malloc0(sizeof(*disk));
+ get_disk_properties(vol_h, disk, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ goto err_close;
}
- disk = g_malloc0(sizeof(*disk));
- disk->bus_type = find_bus_type(bus);
g_debug("bus type %d", disk->bus_type);
/* always set pci_controller as required by schema. get_pci_info() should
* report -1 values for non-PCI buses rather than fail. fail the command
@@ -675,12 +676,14 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
disk->pci_controller = get_pci_info(name, &local_err);
if (local_err) {
error_propagate(errp, local_err);
- goto out_close;
+ goto err_close;
}
- if (bus == BusTypeScsi || bus == BusTypeAta || bus == BusTypeRAID
+ if (disk->bus_type == GUEST_DISK_BUS_TYPE_SCSI
+ || disk->bus_type == GUEST_DISK_BUS_TYPE_IDE
+ || disk->bus_type == GUEST_DISK_BUS_TYPE_RAID
#if (_WIN32_WINNT >= 0x0600)
/* This bus type is not supported before Windows Server 2003 SP1 */
- || bus == BusTypeSas
+ || disk->bus_type == GUEST_DISK_BUS_TYPE_SAS
#endif
) {
/* We are able to use the same ioctls for different bus types
@@ -700,11 +703,17 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
list = g_malloc0(sizeof(*list));
list->value = disk;
list->next = NULL;
-out_close:
CloseHandle(vol_h);
-out_free:
g_free(name);
return list;
+
+err_close:
+ g_free(disk);
+ CloseHandle(vol_h);
+err:
+ g_free(name);
+
+ return NULL;
}
#else
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 12/24] qga-win: report disk serial number
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (10 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 11/24] qga-win: refactor disk properties (bus) Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 13/24] qga-win: refactor disk info Michael Roth
` (11 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Tomáš Golembiovský
From: Tomáš Golembiovský <tgolembi@redhat.com>
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
*coding style fix-ups (declarations at beginning of block)
*improve readability for user-visible errors
*cover additional edge-cases with debug statements
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/commands-win32.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 1e91aa2343..2d7b56d538 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -634,6 +634,36 @@ static void get_disk_properties(HANDLE vol_h, GuestDiskAddress *disk,
disk->bus_type = find_bus_type(dev_desc->BusType);
g_debug("bus type %d", disk->bus_type);
+ /* Query once more. Now with long enough buffer. */
+ size = dev_desc->Size;
+ dev_desc = g_malloc0(size);
+ if (!DeviceIoControl(vol_h, IOCTL_STORAGE_QUERY_PROPERTY, &query,
+ sizeof(STORAGE_PROPERTY_QUERY), dev_desc,
+ size, &received, NULL)) {
+ error_setg_win32(errp, GetLastError(), "failed to get serial number");
+ g_debug("failed to get serial number");
+ goto out_free;
+ }
+ if (dev_desc->SerialNumberOffset > 0) {
+ const char *serial;
+ size_t len;
+
+ if (dev_desc->SerialNumberOffset >= received) {
+ error_setg(errp, "failed to get serial number: offset outside the buffer");
+ g_debug("serial number offset outside the buffer");
+ goto out_free;
+ }
+ serial = (char *)dev_desc + dev_desc->SerialNumberOffset;
+ len = received - dev_desc->SerialNumberOffset;
+ g_debug("serial number \"%s\"", serial);
+ if (*serial != 0) {
+ disk->serial = g_strndup(serial, len);
+ disk->has_serial = true;
+ }
+ }
+out_free:
+ g_free(dev_desc);
+
return;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 13/24] qga-win: refactor disk info
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (11 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 12/24] qga-win: report disk serial number Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 14/24] qga-win: handle multi-disk volumes Michael Roth
` (10 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Tomáš Golembiovský
From: Tomáš Golembiovský <tgolembi@redhat.com>
Refactor building of disk info into a function that builds the list and
a function that returns infor for single disk. This will be used in
future commit that will handle multi-disk volumes.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/commands-win32.c | 52 +++++++++++++++++++++++++++++---------------
1 file changed, 35 insertions(+), 17 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 2d7b56d538..21a88d31e1 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -667,20 +667,15 @@ out_free:
return;
}
-/* VSS provider works with volumes, thus there is no difference if
- * the volume consist of spanned disks. Info about the first disk in the
- * volume is returned for the spanned disk group (LVM) */
-static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
+static void get_single_disk_info(char *name, GuestDiskAddress *disk,
+ Error **errp)
{
- GuestDiskAddressList *list = NULL;
- GuestDiskAddress *disk;
SCSI_ADDRESS addr, *scsi_ad;
DWORD len;
HANDLE vol_h;
Error *local_err = NULL;
scsi_ad = &addr;
- char *name = g_strndup(guid, strlen(guid)-1);
g_debug("getting disk info for: %s", name);
vol_h = CreateFile(name, 0, FILE_SHARE_READ, NULL, OPEN_EXISTING,
@@ -690,7 +685,6 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
goto err;
}
- disk = g_malloc0(sizeof(*disk));
get_disk_properties(vol_h, disk, &local_err);
if (local_err) {
error_propagate(errp, local_err);
@@ -730,20 +724,44 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
* information about volume. */
}
- list = g_malloc0(sizeof(*list));
- list->value = disk;
- list->next = NULL;
- CloseHandle(vol_h);
- g_free(name);
- return list;
-
err_close:
- g_free(disk);
CloseHandle(vol_h);
err:
+ return;
+}
+
+/* VSS provider works with volumes, thus there is no difference if
+ * the volume consist of spanned disks. Info about the first disk in the
+ * volume is returned for the spanned disk group (LVM) */
+static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
+{
+ Error *local_err = NULL;
+ GuestDiskAddressList *list = NULL, *cur_item = NULL;
+ GuestDiskAddress *disk = NULL;
+
+ /* strip final backslash */
+ char *name = g_strdup(guid);
+ if (g_str_has_suffix(name, "\\")) {
+ name[strlen(name) - 1] = 0;
+ }
+
+ disk = g_malloc0(sizeof(GuestDiskAddress));
+ get_single_disk_info(name, disk, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ goto out;
+ }
+
+ cur_item = g_malloc0(sizeof(*list));
+ cur_item->value = disk;
+ disk = NULL;
+ list = cur_item;
+
+out:
+ qapi_free_GuestDiskAddress(disk);
g_free(name);
- return NULL;
+ return list;
}
#else
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 14/24] qga-win: handle multi-disk volumes
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (12 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 13/24] qga-win: refactor disk info Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 15/24] qga-win: return disk device in guest-get-fsinfo Michael Roth
` (9 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Tomáš Golembiovský
From: Tomáš Golembiovský <tgolembi@redhat.com>
Probe the volume for disk extents and return list of all disks.
Originally only first disk of composite volume was returned.
Note that the patch changes get_pci_info() from one state of brokenness
into a different state of brokenness. In other words it still does not do
what it's supposed to do (see comment in code). If anyone knows how to
fix it, please step in.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/commands-win32.c | 126 ++++++++++++++++++++++++++++++++++++-------
1 file changed, 108 insertions(+), 18 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 21a88d31e1..90432bbfce 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -491,9 +491,26 @@ static GuestDiskBusType find_bus_type(STORAGE_BUS_TYPE bus)
return win2qemu[(int)bus];
}
+/* XXX: The following function is BROKEN!
+ *
+ * It does not work and probably has never worked. When we query for list of
+ * disks we get cryptic names like "\Device\0000001d" instead of
+ * "\PhysicalDriveX" or "\HarddiskX". Whether the names can be translated one
+ * way or the other for comparison is an open question.
+ *
+ * When we query volume names (the original version) we are able to match those
+ * but then the property queries report error "Invalid function". (duh!)
+ */
+
+/*
DEFINE_GUID(GUID_DEVINTERFACE_VOLUME,
0x53f5630dL, 0xb6bf, 0x11d0, 0x94, 0xf2,
0x00, 0xa0, 0xc9, 0x1e, 0xfb, 0x8b);
+*/
+DEFINE_GUID(GUID_DEVINTERFACE_DISK,
+ 0x53f56307L, 0xb6bf, 0x11d0, 0x94, 0xf2,
+ 0x00, 0xa0, 0xc9, 0x1e, 0xfb, 0x8b);
+
static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
{
@@ -517,7 +534,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
goto out;
}
- dev_info = SetupDiGetClassDevs(&GUID_DEVINTERFACE_VOLUME, 0, 0,
+ dev_info = SetupDiGetClassDevs(&GUID_DEVINTERFACE_DISK, 0, 0,
DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
if (dev_info == INVALID_HANDLE_VALUE) {
error_setg_win32(errp, GetLastError(), "failed to get devices tree");
@@ -672,20 +689,20 @@ static void get_single_disk_info(char *name, GuestDiskAddress *disk,
{
SCSI_ADDRESS addr, *scsi_ad;
DWORD len;
- HANDLE vol_h;
+ HANDLE disk_h;
Error *local_err = NULL;
scsi_ad = &addr;
g_debug("getting disk info for: %s", name);
- vol_h = CreateFile(name, 0, FILE_SHARE_READ, NULL, OPEN_EXISTING,
+ disk_h = CreateFile(name, 0, FILE_SHARE_READ, NULL, OPEN_EXISTING,
0, NULL);
- if (vol_h == INVALID_HANDLE_VALUE) {
- error_setg_win32(errp, GetLastError(), "failed to open volume");
- goto err;
+ if (disk_h == INVALID_HANDLE_VALUE) {
+ error_setg_win32(errp, GetLastError(), "failed to open disk");
+ return;
}
- get_disk_properties(vol_h, disk, &local_err);
+ get_disk_properties(disk_h, disk, &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto err_close;
@@ -714,7 +731,7 @@ static void get_single_disk_info(char *name, GuestDiskAddress *disk,
* according to Microsoft docs
* https://technet.microsoft.com/en-us/library/ee851589(v=ws.10).aspx */
g_debug("getting pci-controller info");
- if (DeviceIoControl(vol_h, IOCTL_SCSI_GET_ADDRESS, NULL, 0, scsi_ad,
+ if (DeviceIoControl(disk_h, IOCTL_SCSI_GET_ADDRESS, NULL, 0, scsi_ad,
sizeof(SCSI_ADDRESS), &len, NULL)) {
disk->unit = addr.Lun;
disk->target = addr.TargetId;
@@ -725,8 +742,7 @@ static void get_single_disk_info(char *name, GuestDiskAddress *disk,
}
err_close:
- CloseHandle(vol_h);
-err:
+ CloseHandle(disk_h);
return;
}
@@ -738,6 +754,10 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
Error *local_err = NULL;
GuestDiskAddressList *list = NULL, *cur_item = NULL;
GuestDiskAddress *disk = NULL;
+ int i;
+ HANDLE vol_h;
+ DWORD size;
+ PVOLUME_DISK_EXTENTS extents = NULL;
/* strip final backslash */
char *name = g_strdup(guid);
@@ -745,20 +765,90 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
name[strlen(name) - 1] = 0;
}
- disk = g_malloc0(sizeof(GuestDiskAddress));
- get_single_disk_info(name, disk, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
+ g_debug("opening %s", name);
+ vol_h = CreateFile(name, 0, FILE_SHARE_READ, NULL, OPEN_EXISTING,
+ 0, NULL);
+ if (vol_h == INVALID_HANDLE_VALUE) {
+ error_setg_win32(errp, GetLastError(), "failed to open volume");
goto out;
}
- cur_item = g_malloc0(sizeof(*list));
- cur_item->value = disk;
- disk = NULL;
- list = cur_item;
+ /* Get list of extents */
+ g_debug("getting disk extents");
+ size = sizeof(VOLUME_DISK_EXTENTS);
+ extents = g_malloc0(size);
+ if (!DeviceIoControl(vol_h, IOCTL_VOLUME_GET_VOLUME_DISK_EXTENTS, NULL,
+ 0, extents, size, NULL, NULL)) {
+ DWORD last_err = GetLastError();
+ if (last_err == ERROR_MORE_DATA) {
+ /* Try once more with big enough buffer */
+ size = sizeof(VOLUME_DISK_EXTENTS)
+ + extents->NumberOfDiskExtents*sizeof(DISK_EXTENT);
+ g_free(extents);
+ extents = g_malloc0(size);
+ if (!DeviceIoControl(
+ vol_h, IOCTL_VOLUME_GET_VOLUME_DISK_EXTENTS, NULL,
+ 0, extents, size, NULL, NULL)) {
+ error_setg_win32(errp, GetLastError(),
+ "failed to get disk extents");
+ return NULL;
+ }
+ } else if (last_err == ERROR_INVALID_FUNCTION) {
+ /* Possibly CD-ROM or a shared drive. Try to pass the volume */
+ g_debug("volume not on disk");
+ disk = g_malloc0(sizeof(GuestDiskAddress));
+ get_single_disk_info(name, disk, &local_err);
+ if (local_err) {
+ g_debug("failed to get disk info, ignoring error: %s",
+ error_get_pretty(local_err));
+ error_free(local_err);
+ goto out;
+ }
+ list = g_malloc0(sizeof(*list));
+ list->value = disk;
+ disk = NULL;
+ list->next = NULL;
+ goto out;
+ } else {
+ error_setg_win32(errp, GetLastError(),
+ "failed to get disk extents");
+ goto out;
+ }
+ }
+ g_debug("Number of extents: %lu", extents->NumberOfDiskExtents);
+
+ /* Go through each extent */
+ for (i = 0; i < extents->NumberOfDiskExtents; i++) {
+ char *disk_name = NULL;
+ disk = g_malloc0(sizeof(GuestDiskAddress));
+
+ /* Disk numbers directly correspond to numbers used in UNCs
+ *
+ * See documentation for DISK_EXTENT:
+ * https://docs.microsoft.com/en-us/windows/desktop/api/winioctl/ns-winioctl-_disk_extent
+ *
+ * See also Naming Files, Paths and Namespaces:
+ * https://docs.microsoft.com/en-us/windows/desktop/FileIO/naming-a-file#win32-device-namespaces
+ */
+ disk_name = g_strdup_printf("\\\\.\\PhysicalDrive%lu",
+ extents->Extents[i].DiskNumber);
+ get_single_disk_info(disk_name, disk, &local_err);
+ g_free(disk_name);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ goto out;
+ }
+ cur_item = g_malloc0(sizeof(*list));
+ cur_item->value = disk;
+ disk = NULL;
+ cur_item->next = list;
+ list = cur_item;
+ }
+
out:
qapi_free_GuestDiskAddress(disk);
+ g_free(extents);
g_free(name);
return list;
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 15/24] qga-win: return disk device in guest-get-fsinfo
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (13 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 14/24] qga-win: handle multi-disk volumes Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 16/24] qga-win: demystify namespace stripping Michael Roth
` (8 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Tomáš Golembiovský
From: Tomáš Golembiovský <tgolembi@redhat.com>
Report device UNC of the disk. It is reported as "\\.\PhysicalDriveX".
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/commands-win32.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 90432bbfce..30d6c639c3 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -684,8 +684,7 @@ out_free:
return;
}
-static void get_single_disk_info(char *name, GuestDiskAddress *disk,
- Error **errp)
+static void get_single_disk_info(GuestDiskAddress *disk, Error **errp)
{
SCSI_ADDRESS addr, *scsi_ad;
DWORD len;
@@ -694,8 +693,8 @@ static void get_single_disk_info(char *name, GuestDiskAddress *disk,
scsi_ad = &addr;
- g_debug("getting disk info for: %s", name);
- disk_h = CreateFile(name, 0, FILE_SHARE_READ, NULL, OPEN_EXISTING,
+ g_debug("getting disk info for: %s", disk->dev);
+ disk_h = CreateFile(disk->dev, 0, FILE_SHARE_READ, NULL, OPEN_EXISTING,
0, NULL);
if (disk_h == INVALID_HANDLE_VALUE) {
error_setg_win32(errp, GetLastError(), "failed to open disk");
@@ -714,7 +713,7 @@ static void get_single_disk_info(char *name, GuestDiskAddress *disk,
* if that doesn't hold since that suggests some other unexpected
* breakage
*/
- disk->pci_controller = get_pci_info(name, &local_err);
+ disk->pci_controller = get_pci_info(disk->dev, &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto err_close;
@@ -797,7 +796,9 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
/* Possibly CD-ROM or a shared drive. Try to pass the volume */
g_debug("volume not on disk");
disk = g_malloc0(sizeof(GuestDiskAddress));
- get_single_disk_info(name, disk, &local_err);
+ disk->has_dev = true;
+ disk->dev = g_strdup(name);
+ get_single_disk_info(disk, &local_err);
if (local_err) {
g_debug("failed to get disk info, ignoring error: %s",
error_get_pretty(local_err));
@@ -819,7 +820,6 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
/* Go through each extent */
for (i = 0; i < extents->NumberOfDiskExtents; i++) {
- char *disk_name = NULL;
disk = g_malloc0(sizeof(GuestDiskAddress));
/* Disk numbers directly correspond to numbers used in UNCs
@@ -830,10 +830,11 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
* See also Naming Files, Paths and Namespaces:
* https://docs.microsoft.com/en-us/windows/desktop/FileIO/naming-a-file#win32-device-namespaces
*/
- disk_name = g_strdup_printf("\\\\.\\PhysicalDrive%lu",
+ disk->has_dev = true;
+ disk->dev = g_strdup_printf("\\\\.\\PhysicalDrive%lu",
extents->Extents[i].DiskNumber);
- get_single_disk_info(disk_name, disk, &local_err);
- g_free(disk_name);
+
+ get_single_disk_info(disk, &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto out;
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 16/24] qga-win: demystify namespace stripping
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (14 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 15/24] qga-win: return disk device in guest-get-fsinfo Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 17/24] qga: fix an off-by-one issue Michael Roth
` (7 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Tomáš Golembiovský
From: Tomáš Golembiovský <tgolembi@redhat.com>
It was not obvious what exactly the cryptic string copying does to the
GUID. This change makes the intent clearer.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/commands-win32.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 30d6c639c3..a1b7512d46 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -521,7 +521,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
char dev_name[MAX_PATH];
char *buffer = NULL;
GuestPCIAddress *pci = NULL;
- char *name = g_strdup(&guid[4]);
+ char *name = NULL;
bool partial_pci = false;
pci = g_malloc0(sizeof(*pci));
pci->domain = -1;
@@ -529,6 +529,13 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
pci->function = -1;
pci->bus = -1;
+ if (g_str_has_prefix(guid, "\\\\.\\") ||
+ g_str_has_prefix(guid, "\\\\?\\")) {
+ name = g_strdup(guid + 4);
+ } else {
+ name = g_strdup(guid);
+ }
+
if (!QueryDosDevice(name, dev_name, ARRAY_SIZE(dev_name))) {
error_setg_win32(errp, GetLastError(), "failed to get dos device name");
goto out;
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 17/24] qga: fix an off-by-one issue
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (15 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 16/24] qga-win: demystify namespace stripping Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 18/24] qga: group agent init/cleanup init separate routines Michael Roth
` (6 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Li Qiang
From: Li Qiang <liq3ea@gmail.com>
Signed-off-by: Li Qiang <liq3ea@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/commands-win32.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index a1b7512d46..ef1d7d48d2 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -485,7 +485,7 @@ static STORAGE_BUS_TYPE win2qemu[] = {
static GuestDiskBusType find_bus_type(STORAGE_BUS_TYPE bus)
{
- if (bus > ARRAY_SIZE(win2qemu) || (int)bus < 0) {
+ if (bus >= ARRAY_SIZE(win2qemu) || (int)bus < 0) {
return GUEST_DISK_BUS_TYPE_UNKNOWN;
}
return win2qemu[(int)bus];
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 18/24] qga: group agent init/cleanup init separate routines
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (16 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 17/24] qga: fix an off-by-one issue Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 19/24] qga: hang GAConfig/socket_activation off of GAState global Michael Roth
` (5 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
This patch better separates the init/cleanup routines out into
separate functions to make the start-up procedure a bit easier to
follow. This will be useful when we eventually break out the actual
start/stop of the agent's main loop into separates routines that
can be called multiple times after the init phase.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/main.c | 82 +++++++++++++++++++++++++++++++++---------------------
1 file changed, 50 insertions(+), 32 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index afcd268ee3..ab3eaa5163 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -1211,9 +1211,21 @@ static bool check_is_frozen(GAState *s)
return false;
}
-static int run_agent(GAState *s, GAConfig *config, int socket_activation)
+static GAState *initialize_agent(GAConfig *config)
{
- ga_state = s;
+ GAState *s = g_new0(GAState, 1);
+
+ g_assert(ga_state == NULL);
+
+ s->log_level = config->log_level;
+ s->log_file = stderr;
+#ifdef CONFIG_FSFREEZE
+ s->fsfreeze_hook = config->fsfreeze_hook;
+#endif
+ s->pstate_filepath = g_strdup_printf("%s/qga.state", config->state_dir);
+ s->state_filepath_isfrozen = g_strdup_printf("%s/qga.state.isfrozen",
+ config->state_dir);
+ s->frozen = check_is_frozen(s);
g_log_set_default_handler(ga_log, s);
g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR);
@@ -1229,7 +1241,7 @@ static int run_agent(GAState *s, GAConfig *config, int socket_activation)
if (g_mkdir_with_parents(config->state_dir, S_IRWXU) == -1) {
g_critical("unable to create (an ancestor of) the state directory"
" '%s': %s", config->state_dir, strerror(errno));
- return EXIT_FAILURE;
+ return NULL;
}
#endif
@@ -1254,7 +1266,7 @@ static int run_agent(GAState *s, GAConfig *config, int socket_activation)
if (!log_file) {
g_critical("unable to open specified log file: %s",
strerror(errno));
- return EXIT_FAILURE;
+ return NULL;
}
s->log_file = log_file;
}
@@ -1265,7 +1277,7 @@ static int run_agent(GAState *s, GAConfig *config, int socket_activation)
s->pstate_filepath,
ga_is_frozen(s))) {
g_critical("failed to load persistent state");
- return EXIT_FAILURE;
+ return NULL;
}
config->blacklist = ga_command_blacklist_init(config->blacklist);
@@ -1286,12 +1298,37 @@ static int run_agent(GAState *s, GAConfig *config, int socket_activation)
#ifndef _WIN32
if (!register_signal_handlers()) {
g_critical("failed to register signal handlers");
- return EXIT_FAILURE;
+ return NULL;
}
#endif
s->main_loop = g_main_loop_new(NULL, false);
+ ga_state = s;
+ return s;
+}
+
+static void cleanup_agent(GAState *s)
+{
+ if (s->command_state) {
+ ga_command_state_cleanup_all(s->command_state);
+ ga_command_state_free(s->command_state);
+ json_message_parser_destroy(&s->parser);
+ }
+ if (s->channel) {
+ ga_channel_free(s->channel);
+ }
+ g_free(s->pstate_filepath);
+ g_free(s->state_filepath_isfrozen);
+ if (s->main_loop) {
+ g_main_loop_unref(s->main_loop);
+ }
+ g_free(s);
+ ga_state = NULL;
+}
+
+static int run_agent(GAState *s, GAConfig *config, int socket_activation)
+{
if (!channel_init(ga_state, config->method, config->channel_path,
socket_activation ? FIRST_SOCKET_ACTIVATION_FD : -1)) {
g_critical("failed to initialize guest agent channel");
@@ -1315,7 +1352,7 @@ static int run_agent(GAState *s, GAConfig *config, int socket_activation)
int main(int argc, char **argv)
{
int ret = EXIT_SUCCESS;
- GAState *s = g_new0(GAState, 1);
+ GAState *s;
GAConfig *config = g_new0(GAConfig, 1);
int socket_activation;
@@ -1383,44 +1420,25 @@ int main(int argc, char **argv)
}
}
- s->log_level = config->log_level;
- s->log_file = stderr;
-#ifdef CONFIG_FSFREEZE
- s->fsfreeze_hook = config->fsfreeze_hook;
-#endif
- s->pstate_filepath = g_strdup_printf("%s/qga.state", config->state_dir);
- s->state_filepath_isfrozen = g_strdup_printf("%s/qga.state.isfrozen",
- config->state_dir);
- s->frozen = check_is_frozen(s);
-
if (config->dumpconf) {
config_dump(config);
goto end;
}
+ s = initialize_agent(config);
+ if (!s) {
+ g_critical("error initializing guest agent");
+ goto end;
+ }
ret = run_agent(s, config, socket_activation);
+ cleanup_agent(s);
end:
- if (s->command_state) {
- ga_command_state_cleanup_all(s->command_state);
- ga_command_state_free(s->command_state);
- json_message_parser_destroy(&s->parser);
- }
- if (s->channel) {
- ga_channel_free(s->channel);
- }
- g_free(s->pstate_filepath);
- g_free(s->state_filepath_isfrozen);
-
if (config->daemonize) {
unlink(config->pid_filepath);
}
config_free(config);
- if (s->main_loop) {
- g_main_loop_unref(s->main_loop);
- }
- g_free(s);
return ret;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 19/24] qga: hang GAConfig/socket_activation off of GAState global
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (17 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 18/24] qga: group agent init/cleanup init separate routines Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 20/24] qga: move w32 service handling out of run_agent() Michael Roth
` (4 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Sameeh Jubran
For w32 services we rely on the global GAState to access resources
associated with the agent within service_main(). Currently this is
sufficient for starting the agent since we open the channel once prior
to calling service_main(), and simply start the GMainLoop to start the
agent from within service_main().
Eventually we want to be able to also [re-]open the communication
channel from within service_main(), which requires access to
config/socket_activation variables, so we hang them off GAState in
preparation for that.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Sameeh Jubran <sameeh@daynix.com>
*dont move GAConfig struct, just the typedef
*fix build bisect for w32
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/main.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index ab3eaa5163..eb31f99b38 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -69,6 +69,8 @@ typedef struct GAPersistentState {
int64_t fd_counter;
} GAPersistentState;
+typedef struct GAConfig GAConfig;
+
struct GAState {
JSONMessageParser parser;
GMainLoop *main_loop;
@@ -94,6 +96,8 @@ struct GAState {
#endif
gchar *pstate_filepath;
GAPersistentState pstate;
+ GAConfig *config;
+ int socket_activation;
};
struct GAState *ga_state;
@@ -905,7 +909,7 @@ static GList *split_list(const gchar *str, const gchar *delim)
return list;
}
-typedef struct GAConfig {
+struct GAConfig {
char *channel_path;
char *method;
char *log_filepath;
@@ -922,7 +926,7 @@ typedef struct GAConfig {
int daemonize;
GLogLevelFlags log_level;
int dumpconf;
-} GAConfig;
+};
static void config_load(GAConfig *config)
{
@@ -1211,7 +1215,7 @@ static bool check_is_frozen(GAState *s)
return false;
}
-static GAState *initialize_agent(GAConfig *config)
+static GAState *initialize_agent(GAConfig *config, int socket_activation)
{
GAState *s = g_new0(GAState, 1);
@@ -1304,6 +1308,8 @@ static GAState *initialize_agent(GAConfig *config)
s->main_loop = g_main_loop_new(NULL, false);
+ s->config = config;
+ s->socket_activation = socket_activation;
ga_state = s;
return s;
}
@@ -1327,17 +1333,17 @@ static void cleanup_agent(GAState *s)
ga_state = NULL;
}
-static int run_agent(GAState *s, GAConfig *config, int socket_activation)
+static int run_agent(GAState *s)
{
- if (!channel_init(ga_state, config->method, config->channel_path,
- socket_activation ? FIRST_SOCKET_ACTIVATION_FD : -1)) {
+ if (!channel_init(s, s->config->method, s->config->channel_path,
+ s->socket_activation ? FIRST_SOCKET_ACTIVATION_FD : -1)) {
g_critical("failed to initialize guest agent channel");
return EXIT_FAILURE;
}
#ifndef _WIN32
g_main_loop_run(ga_state->main_loop);
#else
- if (config->daemonize) {
+ if (s->config->daemonize) {
SERVICE_TABLE_ENTRY service_table[] = {
{ (char *)QGA_SERVICE_NAME, service_main }, { NULL, NULL } };
StartServiceCtrlDispatcher(service_table);
@@ -1425,12 +1431,12 @@ int main(int argc, char **argv)
goto end;
}
- s = initialize_agent(config);
+ s = initialize_agent(config, socket_activation);
if (!s) {
g_critical("error initializing guest agent");
goto end;
}
- ret = run_agent(s, config, socket_activation);
+ ret = run_agent(s);
cleanup_agent(s);
end:
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 20/24] qga: move w32 service handling out of run_agent()
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (18 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 19/24] qga: hang GAConfig/socket_activation off of GAState global Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 21/24] qga: add --retry-path option for re-initializing channel on failure Michael Roth
` (3 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Bishara AbuHattoum
Eventually we want a w32 service to be able to restart the qga main
loop from within service_main(). To allow for this we move service
handling out of run_agent() such that service_main() calls
run_agent() instead of the reverse.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Bishara AbuHattoum <bishara@daynix.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/main.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index eb31f99b38..761007deb4 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -119,6 +119,7 @@ DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type, LPVOID data,
LPVOID ctx);
VOID WINAPI service_main(DWORD argc, TCHAR *argv[]);
#endif
+static int run_agent(GAState *s);
static void
init_dfl_pathnames(void)
@@ -712,7 +713,7 @@ VOID WINAPI service_main(DWORD argc, TCHAR *argv[])
service->status.dwWaitHint = 0;
SetServiceStatus(service->status_handle, &service->status);
- g_main_loop_run(ga_state->main_loop);
+ run_agent(ga_state);
service->status.dwCurrentState = SERVICE_STOPPED;
SetServiceStatus(service->status_handle, &service->status);
@@ -1340,17 +1341,8 @@ static int run_agent(GAState *s)
g_critical("failed to initialize guest agent channel");
return EXIT_FAILURE;
}
-#ifndef _WIN32
+
g_main_loop_run(ga_state->main_loop);
-#else
- if (s->config->daemonize) {
- SERVICE_TABLE_ENTRY service_table[] = {
- { (char *)QGA_SERVICE_NAME, service_main }, { NULL, NULL } };
- StartServiceCtrlDispatcher(service_table);
- } else {
- g_main_loop_run(ga_state->main_loop);
- }
-#endif
return EXIT_SUCCESS;
}
@@ -1436,7 +1428,19 @@ int main(int argc, char **argv)
g_critical("error initializing guest agent");
goto end;
}
+
+#ifdef _WIN32
+ if (config->daemonize) {
+ SERVICE_TABLE_ENTRY service_table[] = {
+ { (char *)QGA_SERVICE_NAME, service_main }, { NULL, NULL } };
+ StartServiceCtrlDispatcher(service_table);
+ } else {
+ ret = run_agent(s);
+ }
+#else
ret = run_agent(s);
+#endif
+
cleanup_agent(s);
end:
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 21/24] qga: add --retry-path option for re-initializing channel on failure
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (19 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 20/24] qga: move w32 service handling out of run_agent() Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 22/24] qga-win: install service with --retry-path set by default Michael Roth
` (2 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
This adds an option to instruct the agent to periodically attempt
re-opening the communication channel after a channel error has
occurred. The main use-case for this is providing an OS-independent
way of allowing the agent to survive situations like hotplug/unplug of
the communication channel, or initial guest set up where the agent may
be installed/started prior to the installation of the channel device's
driver.
There are nicer ways of implementing this functionality via things
like systemd services, but this option is useful for platforms like
*BSD/w32.
Currently a channel error will result in the GSource for that channel
being removed from the GMainLoop, but the main loop continuing to run.
That behavior results in a dead loop when --retry-path isn't set, and
prevents us from knowing when to attempt re-opening the channel when
it is set, so we also force the loop to exit as part of this patch.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/main.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 54 insertions(+), 8 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index 761007deb4..506a314140 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -58,6 +58,7 @@
#endif
#define QGA_SENTINEL_BYTE 0xFF
#define QGA_CONF_DEFAULT CONFIG_QEMU_CONFDIR G_DIR_SEPARATOR_S "qemu-ga.conf"
+#define QGA_RETRY_INTERVAL 5
static struct {
const char *state_dir;
@@ -98,6 +99,7 @@ struct GAState {
GAPersistentState pstate;
GAConfig *config;
int socket_activation;
+ bool force_exit;
};
struct GAState *ga_state;
@@ -120,6 +122,7 @@ DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type, LPVOID data,
VOID WINAPI service_main(DWORD argc, TCHAR *argv[]);
#endif
static int run_agent(GAState *s);
+static void stop_agent(GAState *s, bool requested);
static void
init_dfl_pathnames(void)
@@ -168,9 +171,7 @@ static void quit_handler(int sig)
}
g_debug("received signal num %d, quitting", sig);
- if (g_main_loop_is_running(ga_state->main_loop)) {
- g_main_loop_quit(ga_state->main_loop);
- }
+ stop_agent(ga_state, true);
}
#ifndef _WIN32
@@ -255,6 +256,10 @@ QEMU_COPYRIGHT "\n"
" to list available RPCs)\n"
" -D, --dump-conf dump a qemu-ga config file based on current config\n"
" options / command-line parameters to stdout\n"
+" -r, --retry-path attempt re-opening path if it's unavailable or closed\n"
+" due to an error which may be recoverable in the future\n"
+" (virtio-serial driver re-install, serial device hot\n"
+" plug/unplug, etc.)\n"
" -h, --help display this help and exit\n"
"\n"
QEMU_HELP_BOTTOM "\n"
@@ -614,6 +619,7 @@ static gboolean channel_event_cb(GIOCondition condition, gpointer data)
switch (status) {
case G_IO_STATUS_ERROR:
g_warning("error reading channel");
+ stop_agent(s, false);
return false;
case G_IO_STATUS_NORMAL:
buf[count] = 0;
@@ -927,6 +933,7 @@ struct GAConfig {
int daemonize;
GLogLevelFlags log_level;
int dumpconf;
+ bool retry_path;
};
static void config_load(GAConfig *config)
@@ -976,6 +983,10 @@ static void config_load(GAConfig *config)
/* enable all log levels */
config->log_level = G_LOG_LEVEL_MASK;
}
+ if (g_key_file_has_key(keyfile, "general", "retry-path", NULL)) {
+ config->retry_path =
+ g_key_file_get_boolean(keyfile, "general", "retry-path", &gerr);
+ }
if (g_key_file_has_key(keyfile, "general", "blacklist", NULL)) {
config->bliststr =
g_key_file_get_string(keyfile, "general", "blacklist", &gerr);
@@ -1037,6 +1048,8 @@ static void config_dump(GAConfig *config)
g_key_file_set_string(keyfile, "general", "statedir", config->state_dir);
g_key_file_set_boolean(keyfile, "general", "verbose",
config->log_level == G_LOG_LEVEL_MASK);
+ g_key_file_set_boolean(keyfile, "general", "retry-path",
+ config->retry_path);
tmp = list_join(config->blacklist, ',');
g_key_file_set_string(keyfile, "general", "blacklist", tmp);
g_free(tmp);
@@ -1055,7 +1068,7 @@ static void config_dump(GAConfig *config)
static void config_parse(GAConfig *config, int argc, char **argv)
{
- const char *sopt = "hVvdm:p:l:f:F::b:s:t:D";
+ const char *sopt = "hVvdm:p:l:f:F::b:s:t:Dr";
int opt_ind = 0, ch;
const struct option lopt[] = {
{ "help", 0, NULL, 'h' },
@@ -1075,6 +1088,7 @@ static void config_parse(GAConfig *config, int argc, char **argv)
{ "service", 1, NULL, 's' },
#endif
{ "statedir", 1, NULL, 't' },
+ { "retry-path", 0, NULL, 'r' },
{ NULL, 0, NULL, 0 }
};
@@ -1119,6 +1133,9 @@ static void config_parse(GAConfig *config, int argc, char **argv)
case 'D':
config->dumpconf = 1;
break;
+ case 'r':
+ config->retry_path = true;
+ break;
case 'b': {
if (is_help_option(optarg)) {
qmp_for_each_command(&ga_commands, ga_print_cmd, NULL);
@@ -1322,9 +1339,6 @@ static void cleanup_agent(GAState *s)
ga_command_state_free(s->command_state);
json_message_parser_destroy(&s->parser);
}
- if (s->channel) {
- ga_channel_free(s->channel);
- }
g_free(s->pstate_filepath);
g_free(s->state_filepath_isfrozen);
if (s->main_loop) {
@@ -1334,7 +1348,7 @@ static void cleanup_agent(GAState *s)
ga_state = NULL;
}
-static int run_agent(GAState *s)
+static int run_agent_once(GAState *s)
{
if (!channel_init(s, s->config->method, s->config->channel_path,
s->socket_activation ? FIRST_SOCKET_ACTIVATION_FD : -1)) {
@@ -1344,9 +1358,41 @@ static int run_agent(GAState *s)
g_main_loop_run(ga_state->main_loop);
+ if (s->channel) {
+ ga_channel_free(s->channel);
+ }
+
return EXIT_SUCCESS;
}
+static int run_agent(GAState *s)
+{
+ int ret = EXIT_SUCCESS;
+
+ s->force_exit = false;
+
+ do {
+ ret = run_agent_once(s);
+ if (s->config->retry_path && !s->force_exit) {
+ g_warning("agent stopped unexpectedly, restarting...");
+ sleep(QGA_RETRY_INTERVAL);
+ }
+ } while (s->config->retry_path && !s->force_exit);
+
+ return ret;
+}
+
+static void stop_agent(GAState *s, bool requested)
+{
+ if (!s->force_exit) {
+ s->force_exit = requested;
+ }
+
+ if (g_main_loop_is_running(s->main_loop)) {
+ g_main_loop_quit(s->main_loop);
+ }
+}
+
int main(int argc, char **argv)
{
int ret = EXIT_SUCCESS;
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 22/24] qga-win: install service with --retry-path set by default
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (20 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 21/24] qga: add --retry-path option for re-initializing channel on failure Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 23/24] qga-win: report specific error when failing to open channel Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 24/24] qga-win: changing --retry-path option behavior Michael Roth
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
It's nicer from a management perspective that the agent can survive
hotplug/unplug of the channel device, or be started prior to the
installation of the channel device's driver without and still be able
to resume normal function afterward. On linux there are alternatives
like systemd to support this, but on w32 --retry-path is the only
option so it makes sense to set it by default when installed as a
w32 service.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/installer/qemu-ga.wxs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index f751a7e9f7..64bf90bd85 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -78,7 +78,7 @@
Account="LocalSystem"
ErrorControl="ignore"
Interactive="no"
- Arguments="-d"
+ Arguments="-d --retry-path"
>
</ServiceInstall>
<ServiceControl Id="StartService" Start="install" Stop="both" Remove="uninstall" Name="QEMU-GA" Wait="no" />
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 23/24] qga-win: report specific error when failing to open channel
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (21 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 22/24] qga-win: install service with --retry-path set by default Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 24/24] qga-win: changing --retry-path option behavior Michael Roth
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Useful in general, but especially now that errors might occur more
frequently with --retry-path set.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/channel-win32.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/qga/channel-win32.c b/qga/channel-win32.c
index b3597a8a0f..c86f4388db 100644
--- a/qga/channel-win32.c
+++ b/qga/channel-win32.c
@@ -302,7 +302,8 @@ static gboolean ga_channel_open(GAChannel *c, GAChannelMethod method,
OPEN_EXISTING,
FILE_FLAG_NO_BUFFERING | FILE_FLAG_OVERLAPPED, NULL);
if (c->handle == INVALID_HANDLE_VALUE) {
- g_critical("error opening path %s", newpath);
+ g_critical("error opening path %s: %s", newpath,
+ g_win32_error_message(GetLastError()));
return false;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL v2 24/24] qga-win: changing --retry-path option behavior
2018-10-31 1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
` (22 preceding siblings ...)
2018-10-31 1:38 ` [Qemu-devel] [PULL v2 23/24] qga-win: report specific error when failing to open channel Michael Roth
@ 2018-10-31 1:38 ` Michael Roth
23 siblings, 0 replies; 25+ messages in thread
From: Michael Roth @ 2018-10-31 1:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Bishara AbuHattoum, Sameeh Jubran
From: Bishara AbuHattoum <bishara@daynix.com>
Currently whenever the qemu-ga's service doesn't find the virtio-serial
the run_agent() loops in a QGA_RETRY_INTERVAL (default 5 seconds)
intervals and try to restart the qemu-ga which causes a synchronous loop.
Changed to wait and listen for the serial events by registering for
notifications a proper serial event handler that deals with events:
DBT_DEVICEARRIVAL indicates that the device has been inserted and
is available
DBT_DEVICEREMOVECOMPLETE indicates that the devive has been removed
Which allow us to determine when the channel path is available for the
qemu-ga to restart.
Signed-off-by: Bishara AbuHattoum <bishara@daynix.com>
Signed-off-by: Sameeh Jubran <sameeh@daynix.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/main.c | 86 ++++++++++++++++++++++++++++++++++++++++++++-
qga/service-win32.h | 4 +++
2 files changed, 89 insertions(+), 1 deletion(-)
diff --git a/qga/main.c b/qga/main.c
index 506a314140..87a0711c14 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -34,6 +34,7 @@
#include "qemu/systemd.h"
#include "qemu-version.h"
#ifdef _WIN32
+#include <dbt.h>
#include "qga/service-win32.h"
#include "qga/vss-win32.h"
#endif
@@ -83,6 +84,7 @@ struct GAState {
bool logging_enabled;
#ifdef _WIN32
GAService service;
+ HANDLE wakeup_event;
#endif
bool delimit_response;
bool frozen;
@@ -119,6 +121,7 @@ static const char *ga_freeze_whitelist[] = {
#ifdef _WIN32
DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type, LPVOID data,
LPVOID ctx);
+DWORD WINAPI handle_serial_device_events(DWORD type, LPVOID data);
VOID WINAPI service_main(DWORD argc, TCHAR *argv[]);
#endif
static int run_agent(GAState *s);
@@ -677,6 +680,36 @@ static gboolean channel_init(GAState *s, const gchar *method, const gchar *path,
}
#ifdef _WIN32
+DWORD WINAPI handle_serial_device_events(DWORD type, LPVOID data)
+{
+ DWORD ret = NO_ERROR;
+ PDEV_BROADCAST_HDR broadcast_header = (PDEV_BROADCAST_HDR)data;
+
+ if (broadcast_header->dbch_devicetype == DBT_DEVTYP_DEVICEINTERFACE) {
+ switch (type) {
+ /* Device inserted */
+ case DBT_DEVICEARRIVAL:
+ /* Start QEMU-ga's service */
+ if (!SetEvent(ga_state->wakeup_event)) {
+ ret = GetLastError();
+ }
+ break;
+ /* Device removed */
+ case DBT_DEVICEQUERYREMOVE:
+ case DBT_DEVICEREMOVEPENDING:
+ case DBT_DEVICEREMOVECOMPLETE:
+ /* Stop QEMU-ga's service */
+ if (!ResetEvent(ga_state->wakeup_event)) {
+ ret = GetLastError();
+ }
+ break;
+ default:
+ ret = ERROR_CALL_NOT_IMPLEMENTED;
+ }
+ }
+ return ret;
+}
+
DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type, LPVOID data,
LPVOID ctx)
{
@@ -688,9 +721,13 @@ DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type, LPVOID data,
case SERVICE_CONTROL_STOP:
case SERVICE_CONTROL_SHUTDOWN:
quit_handler(SIGTERM);
+ SetEvent(ga_state->wakeup_event);
service->status.dwCurrentState = SERVICE_STOP_PENDING;
SetServiceStatus(service->status_handle, &service->status);
break;
+ case SERVICE_CONTROL_DEVICEEVENT:
+ handle_serial_device_events(type, data);
+ break;
default:
ret = ERROR_CALL_NOT_IMPLEMENTED;
@@ -717,10 +754,24 @@ VOID WINAPI service_main(DWORD argc, TCHAR *argv[])
service->status.dwServiceSpecificExitCode = NO_ERROR;
service->status.dwCheckPoint = 0;
service->status.dwWaitHint = 0;
+ DEV_BROADCAST_DEVICEINTERFACE notification_filter;
+ ZeroMemory(¬ification_filter, sizeof(notification_filter));
+ notification_filter.dbcc_devicetype = DBT_DEVTYP_DEVICEINTERFACE;
+ notification_filter.dbcc_size = sizeof(DEV_BROADCAST_DEVICEINTERFACE);
+ notification_filter.dbcc_classguid = GUID_VIOSERIAL_PORT;
+
+ service->device_notification_handle =
+ RegisterDeviceNotification(service->status_handle,
+ ¬ification_filter, DEVICE_NOTIFY_SERVICE_HANDLE);
+ if (!service->device_notification_handle) {
+ g_critical("Failed to register device notification handle!\n");
+ return;
+ }
SetServiceStatus(service->status_handle, &service->status);
run_agent(ga_state);
+ UnregisterDeviceNotification(service->device_notification_handle);
service->status.dwCurrentState = SERVICE_STOPPED;
SetServiceStatus(service->status_handle, &service->status);
}
@@ -1328,12 +1379,24 @@ static GAState *initialize_agent(GAConfig *config, int socket_activation)
s->config = config;
s->socket_activation = socket_activation;
+
+#ifdef _WIN32
+ s->wakeup_event = CreateEvent(NULL, TRUE, FALSE, TEXT("WakeUp"));
+ if (s->wakeup_event == NULL) {
+ g_critical("CreateEvent failed");
+ return NULL;
+ }
+#endif
+
ga_state = s;
return s;
}
static void cleanup_agent(GAState *s)
{
+#ifdef _WIN32
+ CloseHandle(s->wakeup_event);
+#endif
if (s->command_state) {
ga_command_state_cleanup_all(s->command_state);
ga_command_state_free(s->command_state);
@@ -1365,6 +1428,27 @@ static int run_agent_once(GAState *s)
return EXIT_SUCCESS;
}
+static void wait_for_channel_availability(GAState *s)
+{
+ g_warning("waiting for channel path...");
+#ifndef _WIN32
+ sleep(QGA_RETRY_INTERVAL);
+#else
+ DWORD dwWaitResult;
+
+ dwWaitResult = WaitForSingleObject(s->wakeup_event, INFINITE);
+
+ switch (dwWaitResult) {
+ case WAIT_OBJECT_0:
+ break;
+ case WAIT_TIMEOUT:
+ break;
+ default:
+ g_critical("WaitForSingleObject failed");
+ }
+#endif
+}
+
static int run_agent(GAState *s)
{
int ret = EXIT_SUCCESS;
@@ -1375,7 +1459,7 @@ static int run_agent(GAState *s)
ret = run_agent_once(s);
if (s->config->retry_path && !s->force_exit) {
g_warning("agent stopped unexpectedly, restarting...");
- sleep(QGA_RETRY_INTERVAL);
+ wait_for_channel_availability(s);
}
} while (s->config->retry_path && !s->force_exit);
diff --git a/qga/service-win32.h b/qga/service-win32.h
index 89e99dfede..7b16d69b57 100644
--- a/qga/service-win32.h
+++ b/qga/service-win32.h
@@ -20,9 +20,13 @@
#define QGA_SERVICE_NAME "qemu-ga"
#define QGA_SERVICE_DESCRIPTION "Enables integration with QEMU machine emulator and virtualizer."
+static const GUID GUID_VIOSERIAL_PORT = { 0x6fde7521, 0x1b65, 0x48ae,
+{ 0xb6, 0x28, 0x80, 0xbe, 0x62, 0x1, 0x60, 0x26 } };
+
typedef struct GAService {
SERVICE_STATUS status;
SERVICE_STATUS_HANDLE status_handle;
+ HDEVNOTIFY device_notification_handle;
} GAService;
int ga_install_service(const char *path, const char *logfile,
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread