* [PULL 01/12] qga: Rename guest-get-devices return member 'address' to 'id'
2020-10-27 5:53 [PULL 00/12] qemu-ga patch queue for soft-freeze Michael Roth
@ 2020-10-27 5:53 ` Michael Roth
2020-10-27 5:53 ` [PULL 02/12] qga: Use common time encoding for guest-get-devices 'driver-date' Michael Roth
` (12 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Michael Roth @ 2020-10-27 5:53 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, Philippe Mathieu-Daudé,
Markus Armbruster, Marc-André Lureau
From: Markus Armbruster <armbru@redhat.com>
Member 'address' is union GuestDeviceAddress with a single branch
GuestDeviceAddressPCI, containing PCI vendor ID and device ID. This
is not a PCI address. Type GuestPCIAddress is. Messed up in recent
commit 2e4211cee4 "qga: add command guest-get-devices for reporting
VirtIO devices".
Rename type GuestDeviceAddressPCI to GuestDeviceIdPCI, type
GuestDeviceAddress to GuestDeviceId, and member 'address' to 'id'.
Document the member properly while there.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
qga/commands-win32.c | 16 ++++++++--------
qga/qapi-schema.json | 17 +++++++++--------
2 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 0c3c05484f..879b02b6c3 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2390,22 +2390,22 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
}
for (j = 0; hw_ids[j] != NULL; j++) {
GMatchInfo *match_info;
- GuestDeviceAddressPCI *address;
+ GuestDeviceIdPCI *id;
if (!g_regex_match(device_pci_re, hw_ids[j], 0, &match_info)) {
continue;
}
skip = false;
- address = g_new0(GuestDeviceAddressPCI, 1);
+ id = g_new0(GuestDeviceIdPCI, 1);
vendor_id = g_match_info_fetch(match_info, 1);
device_id = g_match_info_fetch(match_info, 2);
- address->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
- address->device_id = g_ascii_strtoull(device_id, NULL, 16);
+ id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
+ id->device_id = g_ascii_strtoull(device_id, NULL, 16);
- device->address = g_new0(GuestDeviceAddress, 1);
- device->has_address = true;
- device->address->type = GUEST_DEVICE_ADDRESS_KIND_PCI;
- device->address->u.pci.data = address;
+ device->id = g_new0(GuestDeviceId, 1);
+ device->has_id = true;
+ device->id->type = GUEST_DEVICE_ID_KIND_PCI;
+ device->id->u.pci.data = id;
g_match_info_free(match_info);
break;
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index cec98c7e06..f2c81cda2b 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1257,26 +1257,26 @@
'returns': 'GuestOSInfo' }
##
-# @GuestDeviceAddressPCI:
+# @GuestDeviceIdPCI:
#
# @vendor-id: vendor ID
# @device-id: device ID
#
# Since: 5.2
##
-{ 'struct': 'GuestDeviceAddressPCI',
+{ 'struct': 'GuestDeviceIdPCI',
'data': { 'vendor-id': 'uint16', 'device-id': 'uint16' } }
##
-# @GuestDeviceAddress:
+# @GuestDeviceId:
#
-# Address of the device
-# - @pci: address of PCI device, since: 5.2
+# Id of the device
+# - @pci: PCI ID, since: 5.2
#
# Since: 5.2
##
-{ 'union': 'GuestDeviceAddress',
- 'data': { 'pci': 'GuestDeviceAddressPCI' } }
+{ 'union': 'GuestDeviceId',
+ 'data': { 'pci': 'GuestDeviceIdPCI' } }
##
# @GuestDeviceInfo:
@@ -1284,6 +1284,7 @@
# @driver-name: name of the associated driver
# @driver-date: driver release date in format YYYY-MM-DD
# @driver-version: driver version
+# @id: device ID
#
# Since: 5.2
##
@@ -1292,7 +1293,7 @@
'driver-name': 'str',
'*driver-date': 'str',
'*driver-version': 'str',
- '*address': 'GuestDeviceAddress'
+ '*id': 'GuestDeviceId'
} }
##
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 02/12] qga: Use common time encoding for guest-get-devices 'driver-date'
2020-10-27 5:53 [PULL 00/12] qemu-ga patch queue for soft-freeze Michael Roth
2020-10-27 5:53 ` [PULL 01/12] qga: Rename guest-get-devices return member 'address' to 'id' Michael Roth
@ 2020-10-27 5:53 ` Michael Roth
2020-10-27 5:53 ` [PULL 03/12] qga-win: Fix guest-get-devices error API violations Michael Roth
` (11 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Michael Roth @ 2020-10-27 5:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Markus Armbruster, Marc-André Lureau
From: Markus Armbruster <armbru@redhat.com>
guest-get-devices returns 'driver-date' as string in the format
YYYY-MM-DD. Goes back to recent commit 2e4211cee4 "qga: add command
guest-get-devices for reporting VirtIO devices".
We should avoid use of multiple encodings for the same kind of data.
Especially string encodings. Change it to return nanoseconds since
the epoch, like guest-get-time does.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
qga/commands-win32.c | 19 +++++++++++--------
qga/qapi-schema.json | 4 ++--
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 879b02b6c3..b01616a992 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1641,6 +1641,12 @@ out:
return head;
}
+static int64_t filetime_to_ns(const FILETIME *tf)
+{
+ return ((((int64_t)tf->dwHighDateTime << 32) | tf->dwLowDateTime)
+ - W32_FT_OFFSET) * 100;
+}
+
int64_t qmp_guest_get_time(Error **errp)
{
SYSTEMTIME ts = {0};
@@ -1657,8 +1663,7 @@ int64_t qmp_guest_get_time(Error **errp)
return -1;
}
- return ((((int64_t)tf.dwHighDateTime << 32) | tf.dwLowDateTime)
- - W32_FT_OFFSET) * 100;
+ return filetime_to_ns(&tf);
}
void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp)
@@ -2363,7 +2368,6 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
slog("enumerating devices");
for (i = 0; SetupDiEnumDeviceInfo(dev_info, i, &dev_info_data); i++) {
bool skip = true;
- SYSTEMTIME utc_date;
g_autofree LPWSTR name = NULL;
g_autofree LPFILETIME date = NULL;
g_autofree LPWSTR version = NULL;
@@ -2434,13 +2438,12 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
slog("failed to get driver date");
continue;
}
- FileTimeToSystemTime(date, &utc_date);
- device->driver_date = g_strdup_printf("%04d-%02d-%02d",
- utc_date.wYear, utc_date.wMonth, utc_date.wDay);
+ device->driver_date = filetime_to_ns(date);
device->has_driver_date = true;
- slog("driver: %s\ndriver version: %s,%s\n", device->driver_name,
- device->driver_date, device->driver_version);
+ slog("driver: %s\ndriver version: %" PRId64 ",%s\n",
+ device->driver_name, device->driver_date,
+ device->driver_version);
item = g_new0(GuestDeviceInfoList, 1);
item->value = g_steal_pointer(&device);
if (!cur_item) {
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index f2c81cda2b..c7bfb8bf6a 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1282,7 +1282,7 @@
# @GuestDeviceInfo:
#
# @driver-name: name of the associated driver
-# @driver-date: driver release date in format YYYY-MM-DD
+# @driver-date: driver release date, in nanoseconds since the epoch
# @driver-version: driver version
# @id: device ID
#
@@ -1291,7 +1291,7 @@
{ 'struct': 'GuestDeviceInfo',
'data': {
'driver-name': 'str',
- '*driver-date': 'str',
+ '*driver-date': 'int',
'*driver-version': 'str',
'*id': 'GuestDeviceId'
} }
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 03/12] qga-win: Fix guest-get-devices error API violations
2020-10-27 5:53 [PULL 00/12] qemu-ga patch queue for soft-freeze Michael Roth
2020-10-27 5:53 ` [PULL 01/12] qga: Rename guest-get-devices return member 'address' to 'id' Michael Roth
2020-10-27 5:53 ` [PULL 02/12] qga: Use common time encoding for guest-get-devices 'driver-date' Michael Roth
@ 2020-10-27 5:53 ` Michael Roth
2020-10-27 5:53 ` [PULL 04/12] qga: Flatten simple union GuestDeviceId Michael Roth
` (10 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Michael Roth @ 2020-10-27 5:53 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, Philippe Mathieu-Daudé,
Markus Armbruster, Marc-André Lureau
From: Markus Armbruster <armbru@redhat.com>
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
qmp_guest_get_devices() is wrong that way: it calls error_setg() in a
loop.
If no iteration fails, the function returns a value and sets no error.
Okay.
If exactly one iteration fails, the function returns a value and sets
an error. Wrong.
If multiple iterations fail, the function trips error_setv()'s
assertion.
Fix it to return immediately on error.
Perhaps the failure to convert the driver version to UTF-8 should not
be an error. We could simply not report the botched version string
instead.
Drop a superfluous continue while there.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
qga/commands-win32.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index b01616a992..1efe3ba076 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2385,7 +2385,7 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
device->driver_name = g_utf16_to_utf8(name, -1, NULL, NULL, NULL);
if (device->driver_name == NULL) {
error_setg(errp, "conversion to utf8 failed (driver name)");
- continue;
+ return NULL;
}
slog("querying device: %s", device->driver_name);
hw_ids = ga_get_hardware_ids(dev_info_data.DevInst);
@@ -2428,7 +2428,7 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
NULL, NULL);
if (device->driver_version == NULL) {
error_setg(errp, "conversion to utf8 failed (driver version)");
- continue;
+ return NULL;
}
device->has_driver_version = true;
@@ -2452,7 +2452,6 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
cur_item->next = item;
cur_item = item;
}
- continue;
}
if (dev_info != INVALID_HANDLE_VALUE) {
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 04/12] qga: Flatten simple union GuestDeviceId
2020-10-27 5:53 [PULL 00/12] qemu-ga patch queue for soft-freeze Michael Roth
` (2 preceding siblings ...)
2020-10-27 5:53 ` [PULL 03/12] qga-win: Fix guest-get-devices error API violations Michael Roth
@ 2020-10-27 5:53 ` Michael Roth
2020-10-27 5:53 ` [PULL 05/12] qga: add command guest-get-disks Michael Roth
` (9 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Michael Roth @ 2020-10-27 5:53 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, Philippe Mathieu-Daudé,
Markus Armbruster, Marc-André Lureau
From: Markus Armbruster <armbru@redhat.com>
Simple unions are simpler than flat unions in the schema, but more
complicated in C and on the QMP wire: there's extra indirection in C
and extra nesting on the wire, both pointless. They should be avoided
in new code.
GuestDeviceId was recently added for guest-get-devices. Convert it to
a flat union.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
qga/commands-win32.c | 9 ++++-----
qga/qapi-schema.json | 8 ++++++++
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 1efe3ba076..0c33d48aaa 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2400,16 +2400,15 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
}
skip = false;
- id = g_new0(GuestDeviceIdPCI, 1);
vendor_id = g_match_info_fetch(match_info, 1);
device_id = g_match_info_fetch(match_info, 2);
- id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
- id->device_id = g_ascii_strtoull(device_id, NULL, 16);
device->id = g_new0(GuestDeviceId, 1);
device->has_id = true;
- device->id->type = GUEST_DEVICE_ID_KIND_PCI;
- device->id->u.pci.data = id;
+ device->id->type = GUEST_DEVICE_TYPE_PCI;
+ id = &device->id->u.pci;
+ id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
+ id->device_id = g_ascii_strtoull(device_id, NULL, 16);
g_match_info_free(match_info);
break;
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index c7bfb8bf6a..fe10631e4c 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1256,6 +1256,12 @@
{ 'command': 'guest-get-osinfo',
'returns': 'GuestOSInfo' }
+##
+# @GuestDeviceType:
+##
+{ 'enum': 'GuestDeviceType',
+ 'data': [ 'pci' ] }
+
##
# @GuestDeviceIdPCI:
#
@@ -1276,6 +1282,8 @@
# Since: 5.2
##
{ 'union': 'GuestDeviceId',
+ 'base': { 'type': 'GuestDeviceType' },
+ 'discriminator': 'type',
'data': { 'pci': 'GuestDeviceIdPCI' } }
##
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 05/12] qga: add command guest-get-disks
2020-10-27 5:53 [PULL 00/12] qemu-ga patch queue for soft-freeze Michael Roth
` (3 preceding siblings ...)
2020-10-27 5:53 ` [PULL 04/12] qga: Flatten simple union GuestDeviceId Michael Roth
@ 2020-10-27 5:53 ` Michael Roth
2020-10-27 5:53 ` [PULL 06/12] qga: add implementation of guest-get-disks for Linux Michael Roth
` (8 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Michael Roth @ 2020-10-27 5:53 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, Tomáš Golembiovský,
Philippe Mathieu-Daudé,
Marc-André Lureau
From: Tomáš Golembiovský <tgolembi@redhat.com>
Add API and stubs for new guest-get-disks command.
The command guest-get-fsinfo can be used to list information about disks
and partitions but it is limited only to mounted disks with filesystem.
This new command should allow listing information about disks of the VM
regardles whether they are mounted or not. This can be usefull for
management applications for mapping virtualized devices or pass-through
devices to device names in the guest OS.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
qga/commands-posix.c | 6 ++++++
qga/commands-win32.c | 6 ++++++
qga/qapi-schema.json | 31 +++++++++++++++++++++++++++++++
3 files changed, 43 insertions(+)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 3bffee99d4..422144bcff 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -3051,3 +3051,9 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
return NULL;
}
+
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+ error_setg(errp, QERR_UNSUPPORTED);
+ return NULL;
+}
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 0c33d48aaa..f7bdd5a8b5 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2458,3 +2458,9 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
}
return head;
}
+
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+ error_setg(errp, QERR_UNSUPPORTED);
+ return NULL;
+}
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index fe10631e4c..e123a000be 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -865,6 +865,37 @@
'bus': 'int', 'target': 'int', 'unit': 'int',
'*serial': 'str', '*dev': 'str'} }
+##
+# @GuestDiskInfo:
+#
+# @name: device node (Linux) or device UNC (Windows)
+# @partition: whether this is a partition or disk
+# @dependents: list of dependent devices; e.g. for LVs of the LVM this will
+# hold the list of PVs, for LUKS encrypted volume this will
+# contain the disk where the volume is placed. (Linux)
+# @address: disk address information (only for non-virtual devices)
+# @alias: optional alias assigned to the disk, on Linux this is a name assigned
+# by device mapper
+#
+# Since 5.2
+##
+{ 'struct': 'GuestDiskInfo',
+ 'data': {'name': 'str', 'partition': 'bool', 'dependents': ['str'],
+ '*address': 'GuestDiskAddress', '*alias': 'str'} }
+
+##
+# @guest-get-disks:
+#
+# Returns: The list of disks in the guest. For Windows these are only the
+# physical disks. On Linux these are all root block devices of
+# non-zero size including e.g. removable devices, loop devices,
+# NBD, etc.
+#
+# Since: 5.2
+##
+{ 'command': 'guest-get-disks',
+ 'returns': ['GuestDiskInfo'] }
+
##
# @GuestFilesystemInfo:
#
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 06/12] qga: add implementation of guest-get-disks for Linux
2020-10-27 5:53 [PULL 00/12] qemu-ga patch queue for soft-freeze Michael Roth
` (4 preceding siblings ...)
2020-10-27 5:53 ` [PULL 05/12] qga: add command guest-get-disks Michael Roth
@ 2020-10-27 5:53 ` Michael Roth
2020-10-27 5:53 ` [PULL 07/12] qga: add implementation of guest-get-disks for Windows Michael Roth
` (7 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Michael Roth @ 2020-10-27 5:53 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, Tomáš Golembiovský, Marc-André Lureau
From: Tomáš Golembiovský <tgolembi@redhat.com>
The command lists all disks (real and virtual) as well as disk
partitions. For each disk the list of dependent disks is also listed and
/dev path is used as a handle so it can be matched with "name" field of
other returned disk entries. For disk partitions the "dependents" list
is populated with the the parent device for easier tracking of
hierarchy.
Example output:
{
"return": [
...
{
"name": "/dev/dm-0",
"partition": false,
"dependents": [
"/dev/sda2"
],
"alias": "luks-7062202e-5b9b-433e-81e8-6628c40da9f7"
},
{
"name": "/dev/sda2",
"partition": true,
"dependents": [
"/dev/sda"
]
},
{
"name": "/dev/sda",
"partition": false,
"address": {
"serial": "SAMSUNG_MZ7LN512HCHP-000L1_S1ZKNXAG822493",
"bus-type": "sata",
...
"dev": "/dev/sda",
"target": 0
},
"dependents": []
},
...
]
}
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
qga/commands-posix.c | 296 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 285 insertions(+), 11 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 422144bcff..14683dfbd5 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1150,13 +1150,27 @@ static void build_guest_fsinfo_for_virtual_device(char const *syspath,
closedir(dir);
}
+static bool is_disk_virtual(const char *devpath, Error **errp)
+{
+ g_autofree char *syspath = realpath(devpath, NULL);
+
+ if (!syspath) {
+ error_setg_errno(errp, errno, "realpath(\"%s\")", devpath);
+ return false;
+ }
+ return strstr(syspath, "/devices/virtual/block/") != NULL;
+}
+
/* Dispatch to functions for virtual/real device */
static void build_guest_fsinfo_for_device(char const *devpath,
GuestFilesystemInfo *fs,
Error **errp)
{
- char *syspath = realpath(devpath, NULL);
+ ERRP_GUARD();
+ g_autofree char *syspath = NULL;
+ bool is_virtual = false;
+ syspath = realpath(devpath, NULL);
if (!syspath) {
error_setg_errno(errp, errno, "realpath(\"%s\")", devpath);
return;
@@ -1167,16 +1181,281 @@ static void build_guest_fsinfo_for_device(char const *devpath,
}
g_debug(" parse sysfs path '%s'", syspath);
-
- if (strstr(syspath, "/devices/virtual/block/")) {
+ is_virtual = is_disk_virtual(syspath, errp);
+ if (*errp != NULL) {
+ return;
+ }
+ if (is_virtual) {
build_guest_fsinfo_for_virtual_device(syspath, fs, errp);
} else {
build_guest_fsinfo_for_real_device(syspath, fs, errp);
}
+}
+
+#ifdef CONFIG_LIBUDEV
+
+/*
+ * Wrapper around build_guest_fsinfo_for_device() for getting just
+ * the disk address.
+ */
+static GuestDiskAddress *get_disk_address(const char *syspath, Error **errp)
+{
+ g_autoptr(GuestFilesystemInfo) fs = NULL;
+
+ fs = g_new0(GuestFilesystemInfo, 1);
+ build_guest_fsinfo_for_device(syspath, fs, errp);
+ if (fs->disk != NULL) {
+ return g_steal_pointer(&fs->disk->value);
+ }
+ return NULL;
+}
+
+static char *get_alias_for_syspath(const char *syspath)
+{
+ struct udev *udev = NULL;
+ struct udev_device *udevice = NULL;
+ char *ret = NULL;
+
+ udev = udev_new();
+ if (udev == NULL) {
+ g_debug("failed to query udev");
+ goto out;
+ }
+ udevice = udev_device_new_from_syspath(udev, syspath);
+ if (udevice == NULL) {
+ g_debug("failed to query udev for path: %s", syspath);
+ goto out;
+ } else {
+ const char *alias = udev_device_get_property_value(
+ udevice, "DM_NAME");
+ /*
+ * NULL means there was an error and empty string means there is no
+ * alias. In case of no alias we return NULL instead of empty string.
+ */
+ if (alias == NULL) {
+ g_debug("failed to query udev for device alias for: %s",
+ syspath);
+ } else if (*alias != 0) {
+ ret = g_strdup(alias);
+ }
+ }
+
+out:
+ udev_unref(udev);
+ udev_device_unref(udevice);
+ return ret;
+}
+
+static char *get_device_for_syspath(const char *syspath)
+{
+ struct udev *udev = NULL;
+ struct udev_device *udevice = NULL;
+ char *ret = NULL;
+
+ udev = udev_new();
+ if (udev == NULL) {
+ g_debug("failed to query udev");
+ goto out;
+ }
+ udevice = udev_device_new_from_syspath(udev, syspath);
+ if (udevice == NULL) {
+ g_debug("failed to query udev for path: %s", syspath);
+ goto out;
+ } else {
+ ret = g_strdup(udev_device_get_devnode(udevice));
+ }
+
+out:
+ udev_unref(udev);
+ udev_device_unref(udevice);
+ return ret;
+}
+
+static void get_disk_deps(const char *disk_dir, GuestDiskInfo *disk)
+{
+ g_autofree char *deps_dir = NULL;
+ const gchar *dep;
+ GDir *dp_deps = NULL;
+
+ /* List dependent disks */
+ deps_dir = g_strdup_printf("%s/slaves", disk_dir);
+ g_debug(" listing entries in: %s", deps_dir);
+ dp_deps = g_dir_open(deps_dir, 0, NULL);
+ if (dp_deps == NULL) {
+ g_debug("failed to list entries in %s", deps_dir);
+ return;
+ }
+ while ((dep = g_dir_read_name(dp_deps)) != NULL) {
+ g_autofree char *dep_dir = NULL;
+ strList *dep_item = NULL;
+ char *dev_name;
- free(syspath);
+ /* Add dependent disks */
+ dep_dir = g_strdup_printf("%s/%s", deps_dir, dep);
+ dev_name = get_device_for_syspath(dep_dir);
+ if (dev_name != NULL) {
+ g_debug(" adding dependent device: %s", dev_name);
+ dep_item = g_new0(strList, 1);
+ dep_item->value = dev_name;
+ dep_item->next = disk->dependents;
+ disk->dependents = dep_item;
+ }
+ }
+ g_dir_close(dp_deps);
}
+/*
+ * Detect partitions subdirectory, name is "<disk_name><number>" or
+ * "<disk_name>p<number>"
+ *
+ * @disk_name -- last component of /sys path (e.g. sda)
+ * @disk_dir -- sys path of the disk (e.g. /sys/block/sda)
+ * @disk_dev -- device node of the disk (e.g. /dev/sda)
+ */
+static GuestDiskInfoList *get_disk_partitions(
+ GuestDiskInfoList *list,
+ const char *disk_name, const char *disk_dir,
+ const char *disk_dev)
+{
+ GuestDiskInfoList *item, *ret = list;
+ struct dirent *de_disk;
+ DIR *dp_disk = NULL;
+ size_t len = strlen(disk_name);
+
+ dp_disk = opendir(disk_dir);
+ while ((de_disk = readdir(dp_disk)) != NULL) {
+ g_autofree char *partition_dir = NULL;
+ char *dev_name;
+ GuestDiskInfo *partition;
+
+ if (!(de_disk->d_type & DT_DIR)) {
+ continue;
+ }
+
+ if (!(strncmp(disk_name, de_disk->d_name, len) == 0 &&
+ ((*(de_disk->d_name + len) == 'p' &&
+ isdigit(*(de_disk->d_name + len + 1))) ||
+ isdigit(*(de_disk->d_name + len))))) {
+ continue;
+ }
+
+ partition_dir = g_strdup_printf("%s/%s",
+ disk_dir, de_disk->d_name);
+ dev_name = get_device_for_syspath(partition_dir);
+ if (dev_name == NULL) {
+ g_debug("Failed to get device name for syspath: %s",
+ disk_dir);
+ continue;
+ }
+ partition = g_new0(GuestDiskInfo, 1);
+ partition->name = dev_name;
+ partition->partition = true;
+ /* Add parent disk as dependent for easier tracking of hierarchy */
+ partition->dependents = g_new0(strList, 1);
+ partition->dependents->value = g_strdup(disk_dev);
+
+ item = g_new0(GuestDiskInfoList, 1);
+ item->value = partition;
+ item->next = ret;
+ ret = item;
+
+ }
+ closedir(dp_disk);
+
+ return ret;
+}
+
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+ GuestDiskInfoList *item, *ret = NULL;
+ GuestDiskInfo *disk;
+ DIR *dp = NULL;
+ struct dirent *de = NULL;
+
+ g_debug("listing /sys/block directory");
+ dp = opendir("/sys/block");
+ if (dp == NULL) {
+ error_setg_errno(errp, errno, "Can't open directory \"/sys/block\"");
+ return NULL;
+ }
+ while ((de = readdir(dp)) != NULL) {
+ g_autofree char *disk_dir = NULL, *line = NULL,
+ *size_path = NULL, *deps_dir = NULL;
+ char *dev_name;
+ Error *local_err = NULL;
+ if (de->d_type != DT_LNK) {
+ g_debug(" skipping entry: %s", de->d_name);
+ continue;
+ }
+
+ /* Check size and skip zero-sized disks */
+ g_debug(" checking disk size");
+ size_path = g_strdup_printf("/sys/block/%s/size", de->d_name);
+ if (!g_file_get_contents(size_path, &line, NULL, NULL)) {
+ g_debug(" failed to read disk size");
+ continue;
+ }
+ if (g_strcmp0(line, "0\n") == 0) {
+ g_debug(" skipping zero-sized disk");
+ continue;
+ }
+
+ g_debug(" adding %s", de->d_name);
+ disk_dir = g_strdup_printf("/sys/block/%s", de->d_name);
+ dev_name = get_device_for_syspath(disk_dir);
+ if (dev_name == NULL) {
+ g_debug("Failed to get device name for syspath: %s",
+ disk_dir);
+ continue;
+ }
+ disk = g_new0(GuestDiskInfo, 1);
+ disk->name = dev_name;
+ disk->partition = false;
+ disk->alias = get_alias_for_syspath(disk_dir);
+ disk->has_alias = (disk->alias != NULL);
+ item = g_new0(GuestDiskInfoList, 1);
+ item->value = disk;
+ item->next = ret;
+ ret = item;
+
+ /* Get address for non-virtual devices */
+ bool is_virtual = is_disk_virtual(disk_dir, &local_err);
+ if (local_err != NULL) {
+ g_debug(" failed to check disk path, ignoring error: %s",
+ error_get_pretty(local_err));
+ error_free(local_err);
+ local_err = NULL;
+ /* Don't try to get the address */
+ is_virtual = true;
+ }
+ if (!is_virtual) {
+ disk->address = get_disk_address(disk_dir, &local_err);
+ if (local_err != NULL) {
+ g_debug(" failed to get device info, ignoring error: %s",
+ error_get_pretty(local_err));
+ error_free(local_err);
+ local_err = NULL;
+ } else if (disk->address != NULL) {
+ disk->has_address = true;
+ }
+ }
+
+ get_disk_deps(disk_dir, disk);
+ ret = get_disk_partitions(ret, de->d_name, disk_dir, dev_name);
+ }
+ return ret;
+}
+
+#else
+
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+ error_setg(errp, QERR_UNSUPPORTED);
+ return NULL;
+}
+
+#endif
+
/* Return a list of the disk device(s)' info which @mount lies on */
static GuestFilesystemInfo *build_guest_fsinfo(struct FsMount *mount,
Error **errp)
@@ -2809,7 +3088,8 @@ GList *ga_command_blacklist_init(GList *blacklist)
const char *list[] = {
"guest-get-fsinfo", "guest-fsfreeze-status",
"guest-fsfreeze-freeze", "guest-fsfreeze-freeze-list",
- "guest-fsfreeze-thaw", "guest-get-fsinfo", NULL};
+ "guest-fsfreeze-thaw", "guest-get-fsinfo",
+ "guest-get-disks", NULL};
char **p = (char **)list;
while (*p) {
@@ -3051,9 +3331,3 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
return NULL;
}
-
-GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
-{
- error_setg(errp, QERR_UNSUPPORTED);
- return NULL;
-}
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 07/12] qga: add implementation of guest-get-disks for Windows
2020-10-27 5:53 [PULL 00/12] qemu-ga patch queue for soft-freeze Michael Roth
` (5 preceding siblings ...)
2020-10-27 5:53 ` [PULL 06/12] qga: add implementation of guest-get-disks for Linux Michael Roth
@ 2020-10-27 5:53 ` Michael Roth
2020-10-27 5:53 ` [PULL 08/12] glib-compat: add g_unix_get_passwd_entry_qemu() Michael Roth
` (6 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Michael Roth @ 2020-10-27 5:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Tomáš Golembiovský
From: Tomáš Golembiovský <tgolembi@redhat.com>
The command lists all the physical disk drives. Unlike for Linux
partitions and virtual volumes are not listed.
Example output:
{
"return": [
{
"name": "\\\\.\\PhysicalDrive0",
"partition": false,
"address": {
"serial": "QM00001",
"bus-type": "sata",
...
},
"dependents": []
}
]
}
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
qga/commands-win32.c | 107 ++++++++++++++++++++++++++++++++++++++++---
1 file changed, 101 insertions(+), 6 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index f7bdd5a8b5..300b87c859 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -979,6 +979,101 @@ out:
return list;
}
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+ ERRP_GUARD();
+ GuestDiskInfoList *new = NULL, *ret = NULL;
+ HDEVINFO dev_info;
+ SP_DEVICE_INTERFACE_DATA dev_iface_data;
+ int i;
+
+ 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 device tree");
+ return NULL;
+ }
+
+ g_debug("enumerating devices");
+ dev_iface_data.cbSize = sizeof(SP_DEVICE_INTERFACE_DATA);
+ for (i = 0;
+ SetupDiEnumDeviceInterfaces(dev_info, NULL, &GUID_DEVINTERFACE_DISK,
+ i, &dev_iface_data);
+ i++) {
+ GuestDiskAddress *address = NULL;
+ GuestDiskInfo *disk = NULL;
+ Error *local_err = NULL;
+ g_autofree PSP_DEVICE_INTERFACE_DETAIL_DATA
+ pdev_iface_detail_data = NULL;
+ STORAGE_DEVICE_NUMBER sdn;
+ HANDLE dev_file;
+ DWORD size = 0;
+ BOOL result;
+ int attempt;
+
+ g_debug(" getting device path");
+ for (attempt = 0, result = FALSE; attempt < 2 && !result; attempt++) {
+ result = SetupDiGetDeviceInterfaceDetail(dev_info,
+ &dev_iface_data, pdev_iface_detail_data, size, &size, NULL);
+ if (result) {
+ break;
+ }
+ if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) {
+ pdev_iface_detail_data = g_realloc(pdev_iface_detail_data,
+ size);
+ pdev_iface_detail_data->cbSize =
+ sizeof(*pdev_iface_detail_data);
+ } else {
+ g_debug("failed to get device interface details");
+ break;
+ }
+ }
+ if (!result) {
+ g_debug("skipping device");
+ continue;
+ }
+
+ g_debug(" device: %s", pdev_iface_detail_data->DevicePath);
+ dev_file = CreateFile(pdev_iface_detail_data->DevicePath, 0,
+ FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, NULL);
+ if (!DeviceIoControl(dev_file, IOCTL_STORAGE_GET_DEVICE_NUMBER,
+ NULL, 0, &sdn, sizeof(sdn), &size, NULL)) {
+ CloseHandle(dev_file);
+ debug_error("failed to get storage device number");
+ continue;
+ }
+ CloseHandle(dev_file);
+
+ disk = g_new0(GuestDiskInfo, 1);
+ disk->name = g_strdup_printf("\\\\.\\PhysicalDrive%lu",
+ sdn.DeviceNumber);
+
+ g_debug(" number: %lu", sdn.DeviceNumber);
+ address = g_malloc0(sizeof(GuestDiskAddress));
+ address->has_dev = true;
+ address->dev = g_strdup(disk->name);
+ get_single_disk_info(sdn.DeviceNumber, address, &local_err);
+ if (local_err) {
+ g_debug("failed to get disk info: %s",
+ error_get_pretty(local_err));
+ error_free(local_err);
+ qapi_free_GuestDiskAddress(address);
+ address = NULL;
+ } else {
+ disk->address = address;
+ disk->has_address = true;
+ }
+
+ new = g_malloc0(sizeof(GuestDiskInfoList));
+ new->value = disk;
+ new->next = ret;
+ ret = new;
+ }
+
+ SetupDiDestroyDeviceInfoList(dev_info);
+ return ret;
+}
+
#else
static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
@@ -986,6 +1081,12 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
return NULL;
}
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+ error_setg(errp, QERR_UNSUPPORTED);
+ return NULL;
+}
+
#endif /* CONFIG_QGA_NTDDSCSI */
static GuestFilesystemInfo *build_guest_fsinfo(char *guid, Error **errp)
@@ -2458,9 +2559,3 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
}
return head;
}
-
-GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
-{
- error_setg(errp, QERR_UNSUPPORTED);
- return NULL;
-}
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 08/12] glib-compat: add g_unix_get_passwd_entry_qemu()
2020-10-27 5:53 [PULL 00/12] qemu-ga patch queue for soft-freeze Michael Roth
` (6 preceding siblings ...)
2020-10-27 5:53 ` [PULL 07/12] qga: add implementation of guest-get-disks for Windows Michael Roth
@ 2020-10-27 5:53 ` Michael Roth
2020-10-27 5:53 ` [PULL 09/12] qga: add ssh-{add,remove}-authorized-keys Michael Roth
` (5 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Michael Roth @ 2020-10-27 5:53 UTC (permalink / raw)
To: qemu-devel; +Cc: Michal Privoznik, peter.maydell, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The glib function was introduced in 2.64. It's a safer version of
getpwnam, and also simpler to use than getpwnam_r.
Currently, it's only use by the next patch in qemu-ga, which doesn't
(well well...) need the thread safety guarantees. Since the fallback
version is still unsafe, I would rather keep the _qemu postfix, to make
sure it's not being misused by mistake. When/if necessary, we can
implement a safer fallback and drop the _qemu suffix.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
include/glib-compat.h | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/include/glib-compat.h b/include/glib-compat.h
index 0b0ec76299..64e68aa730 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -30,6 +30,11 @@
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
#include <glib.h>
+#if defined(G_OS_UNIX)
+#include <glib-unix.h>
+#include <sys/types.h>
+#include <pwd.h>
+#endif
/*
* Note that because of the GLIB_VERSION_MAX_ALLOWED constant above, allowing
@@ -72,6 +77,27 @@
gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout);
#endif
+#if defined(G_OS_UNIX)
+/* Note: The fallback implementation is not MT-safe, and it returns a copy of
+ * the libc passwd (must be g_free() after use) but not the content. Because of
+ * these important differences the caller must be aware of, it's not #define for
+ * GLib API substitution. */
+static inline struct passwd *
+g_unix_get_passwd_entry_qemu(const gchar *user_name, GError **error)
+{
+#if GLIB_CHECK_VERSION(2, 64, 0)
+ return g_unix_get_passwd_entry(user_name, error);
+#else
+ struct passwd *p = getpwnam(user_name);
+ if (!p) {
+ g_set_error_literal(error, G_UNIX_ERROR, 0, g_strerror(errno));
+ return NULL;
+ }
+ return (struct passwd *)g_memdup(p, sizeof(*p));
+#endif
+}
+#endif /* G_OS_UNIX */
+
#pragma GCC diagnostic pop
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 09/12] qga: add ssh-{add,remove}-authorized-keys
2020-10-27 5:53 [PULL 00/12] qemu-ga patch queue for soft-freeze Michael Roth
` (7 preceding siblings ...)
2020-10-27 5:53 ` [PULL 08/12] glib-compat: add g_unix_get_passwd_entry_qemu() Michael Roth
@ 2020-10-27 5:53 ` Michael Roth
2020-11-02 15:49 ` Markus Armbruster
2020-10-27 5:53 ` [PULL 10/12] qga: add *reset argument to ssh-add-authorized-keys Michael Roth
` (4 subsequent siblings)
13 siblings, 1 reply; 18+ messages in thread
From: Michael Roth @ 2020-10-27 5:53 UTC (permalink / raw)
To: qemu-devel
Cc: Michal Privoznik, peter.maydell, Daniel P . Berrangé,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Add new commands to add and remove SSH public keys from
~/.ssh/authorized_keys.
I took a different approach for testing, including the unit tests right
with the code. I wanted to overwrite the function to get the user
details, I couldn't easily do that over QMP. Furthermore, I prefer
having unit tests very close to the code, and unit files that are domain
specific (commands-posix is too crowded already). FWIW, that
coding/testing style is Rust-style (where tests can or should even be
part of the documentation!).
Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1885332
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
*squashed in fix-ups for setting file ownership and use of QAPI
conditionals for CONFIG_POSIX instead of stub definitions
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
qga/commands-posix-ssh.c | 407 +++++++++++++++++++++++++++++++++++++++
qga/meson.build | 20 +-
qga/qapi-schema.json | 35 ++++
3 files changed, 461 insertions(+), 1 deletion(-)
create mode 100644 qga/commands-posix-ssh.c
diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
new file mode 100644
index 0000000000..a7bc9a1c24
--- /dev/null
+++ b/qga/commands-posix-ssh.c
@@ -0,0 +1,407 @@
+ /*
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+
+#include <glib-unix.h>
+#include <glib/gstdio.h>
+#include <locale.h>
+#include <pwd.h>
+
+#include "qapi/error.h"
+#include "qga-qapi-commands.h"
+
+#ifdef QGA_BUILD_UNIT_TEST
+static struct passwd *
+test_get_passwd_entry(const gchar *user_name, GError **error)
+{
+ struct passwd *p;
+ int ret;
+
+ if (!user_name || g_strcmp0(user_name, g_get_user_name())) {
+ g_set_error(error, G_UNIX_ERROR, 0, "Invalid user name");
+ return NULL;
+ }
+
+ p = g_new0(struct passwd, 1);
+ p->pw_dir = (char *)g_get_home_dir();
+ p->pw_uid = geteuid();
+ p->pw_gid = getegid();
+
+ ret = g_mkdir_with_parents(p->pw_dir, 0700);
+ g_assert_cmpint(ret, ==, 0);
+
+ return p;
+}
+
+#define g_unix_get_passwd_entry_qemu(username, err) \
+ test_get_passwd_entry(username, err)
+#endif
+
+static struct passwd *
+get_passwd_entry(const char *username, Error **errp)
+{
+ g_autoptr(GError) err = NULL;
+ struct passwd *p;
+
+ ERRP_GUARD();
+
+ p = g_unix_get_passwd_entry_qemu(username, &err);
+ if (p == NULL) {
+ error_setg(errp, "failed to lookup user '%s': %s",
+ username, err->message);
+ return NULL;
+ }
+
+ return p;
+}
+
+static bool
+mkdir_for_user(const char *path, const struct passwd *p,
+ mode_t mode, Error **errp)
+{
+ ERRP_GUARD();
+
+ if (g_mkdir(path, mode) == -1) {
+ error_setg(errp, "failed to create directory '%s': %s",
+ path, g_strerror(errno));
+ return false;
+ }
+
+ if (chown(path, p->pw_uid, p->pw_gid) == -1) {
+ error_setg(errp, "failed to set ownership of directory '%s': %s",
+ path, g_strerror(errno));
+ return false;
+ }
+
+ if (chmod(path, mode) == -1) {
+ error_setg(errp, "failed to set permissions of directory '%s': %s",
+ path, g_strerror(errno));
+ return false;
+ }
+
+ return true;
+}
+
+static bool
+check_openssh_pub_key(const char *key, Error **errp)
+{
+ ERRP_GUARD();
+
+ /* simple sanity-check, we may want more? */
+ if (!key || key[0] == '#' || strchr(key, '\n')) {
+ error_setg(errp, "invalid OpenSSH public key: '%s'", key);
+ return false;
+ }
+
+ return true;
+}
+
+static bool
+check_openssh_pub_keys(strList *keys, size_t *nkeys, Error **errp)
+{
+ size_t n = 0;
+ strList *k;
+
+ ERRP_GUARD();
+
+ for (k = keys; k != NULL; k = k->next) {
+ if (!check_openssh_pub_key(k->value, errp)) {
+ return false;
+ }
+ n++;
+ }
+
+ if (nkeys) {
+ *nkeys = n;
+ }
+ return true;
+}
+
+static bool
+write_authkeys(const char *path, const GStrv keys,
+ const struct passwd *p, Error **errp)
+{
+ g_autofree char *contents = NULL;
+ g_autoptr(GError) err = NULL;
+
+ ERRP_GUARD();
+
+ contents = g_strjoinv("\n", keys);
+ if (!g_file_set_contents(path, contents, -1, &err)) {
+ error_setg(errp, "failed to write to '%s': %s", path, err->message);
+ return false;
+ }
+
+ if (chown(path, p->pw_uid, p->pw_gid) == -1) {
+ error_setg(errp, "failed to set ownership of directory '%s': %s",
+ path, g_strerror(errno));
+ return false;
+ }
+
+ if (chmod(path, 0600) == -1) {
+ error_setg(errp, "failed to set permissions of '%s': %s",
+ path, g_strerror(errno));
+ return false;
+ }
+
+ return true;
+}
+
+static GStrv
+read_authkeys(const char *path, Error **errp)
+{
+ g_autoptr(GError) err = NULL;
+ g_autofree char *contents = NULL;
+
+ ERRP_GUARD();
+
+ if (!g_file_get_contents(path, &contents, NULL, &err)) {
+ error_setg(errp, "failed to read '%s': %s", path, err->message);
+ return NULL;
+ }
+
+ return g_strsplit(contents, "\n", -1);
+
+}
+
+void
+qmp_guest_ssh_add_authorized_keys(const char *username, strList *keys,
+ Error **errp)
+{
+ g_autofree struct passwd *p = NULL;
+ g_autofree char *ssh_path = NULL;
+ g_autofree char *authkeys_path = NULL;
+ g_auto(GStrv) authkeys = NULL;
+ strList *k;
+ size_t nkeys, nauthkeys;
+
+ ERRP_GUARD();
+
+ if (!check_openssh_pub_keys(keys, &nkeys, errp)) {
+ return;
+ }
+
+ p = get_passwd_entry(username, errp);
+ if (p == NULL) {
+ return;
+ }
+
+ ssh_path = g_build_filename(p->pw_dir, ".ssh", NULL);
+ authkeys_path = g_build_filename(ssh_path, "authorized_keys", NULL);
+
+ authkeys = read_authkeys(authkeys_path, NULL);
+ if (authkeys == NULL) {
+ if (!g_file_test(ssh_path, G_FILE_TEST_IS_DIR) &&
+ !mkdir_for_user(ssh_path, p, 0700, errp)) {
+ return;
+ }
+ }
+
+ nauthkeys = authkeys ? g_strv_length(authkeys) : 0;
+ authkeys = g_realloc_n(authkeys, nauthkeys + nkeys + 1, sizeof(char *));
+ memset(authkeys + nauthkeys, 0, (nkeys + 1) * sizeof(char *));
+
+ for (k = keys; k != NULL; k = k->next) {
+ if (g_strv_contains((const gchar * const *)authkeys, k->value)) {
+ continue;
+ }
+ authkeys[nauthkeys++] = g_strdup(k->value);
+ }
+
+ write_authkeys(authkeys_path, authkeys, p, errp);
+}
+
+void
+qmp_guest_ssh_remove_authorized_keys(const char *username, strList *keys,
+ Error **errp)
+{
+ g_autofree struct passwd *p = NULL;
+ g_autofree char *authkeys_path = NULL;
+ g_autofree GStrv new_keys = NULL; /* do not own the strings */
+ g_auto(GStrv) authkeys = NULL;
+ GStrv a;
+ size_t nkeys = 0;
+
+ ERRP_GUARD();
+
+ if (!check_openssh_pub_keys(keys, NULL, errp)) {
+ return;
+ }
+
+ p = get_passwd_entry(username, errp);
+ if (p == NULL) {
+ return;
+ }
+
+ authkeys_path = g_build_filename(p->pw_dir, ".ssh",
+ "authorized_keys", NULL);
+ if (!g_file_test(authkeys_path, G_FILE_TEST_EXISTS)) {
+ return;
+ }
+ authkeys = read_authkeys(authkeys_path, errp);
+ if (authkeys == NULL) {
+ return;
+ }
+
+ new_keys = g_new0(char *, g_strv_length(authkeys) + 1);
+ for (a = authkeys; *a != NULL; a++) {
+ strList *k;
+
+ for (k = keys; k != NULL; k = k->next) {
+ if (g_str_equal(k->value, *a)) {
+ break;
+ }
+ }
+ if (k != NULL) {
+ continue;
+ }
+
+ new_keys[nkeys++] = *a;
+ }
+
+ write_authkeys(authkeys_path, new_keys, p, errp);
+}
+
+
+#ifdef QGA_BUILD_UNIT_TEST
+#if GLIB_CHECK_VERSION(2, 60, 0)
+static const strList test_key2 = {
+ .value = (char *)"algo key2 comments"
+};
+
+static const strList test_key1_2 = {
+ .value = (char *)"algo key1 comments",
+ .next = (strList *)&test_key2,
+};
+
+static char *
+test_get_authorized_keys_path(void)
+{
+ return g_build_filename(g_get_home_dir(), ".ssh", "authorized_keys", NULL);
+}
+
+static void
+test_authorized_keys_set(const char *contents)
+{
+ g_autoptr(GError) err = NULL;
+ g_autofree char *path = NULL;
+ int ret;
+
+ path = g_build_filename(g_get_home_dir(), ".ssh", NULL);
+ ret = g_mkdir_with_parents(path, 0700);
+ g_assert_cmpint(ret, ==, 0);
+ g_free(path);
+
+ path = test_get_authorized_keys_path();
+ g_file_set_contents(path, contents, -1, &err);
+ g_assert_no_error(err);
+}
+
+static void
+test_authorized_keys_equal(const char *expected)
+{
+ g_autoptr(GError) err = NULL;
+ g_autofree char *path = NULL;
+ g_autofree char *contents = NULL;
+
+ path = test_get_authorized_keys_path();
+ g_file_get_contents(path, &contents, NULL, &err);
+ g_assert_no_error(err);
+
+ g_assert_cmpstr(contents, ==, expected);
+}
+
+static void
+test_invalid_user(void)
+{
+ Error *err = NULL;
+
+ qmp_guest_ssh_add_authorized_keys("", NULL, &err);
+ error_free_or_abort(&err);
+
+ qmp_guest_ssh_remove_authorized_keys("", NULL, &err);
+ error_free_or_abort(&err);
+}
+
+static void
+test_invalid_key(void)
+{
+ strList key = {
+ .value = (char *)"not a valid\nkey"
+ };
+ Error *err = NULL;
+
+ qmp_guest_ssh_add_authorized_keys(g_get_user_name(), &key, &err);
+ error_free_or_abort(&err);
+
+ qmp_guest_ssh_remove_authorized_keys(g_get_user_name(), &key, &err);
+ error_free_or_abort(&err);
+}
+
+static void
+test_add_keys(void)
+{
+ Error *err = NULL;
+
+ qmp_guest_ssh_add_authorized_keys(g_get_user_name(),
+ (strList *)&test_key2, &err);
+ g_assert_null(err);
+
+ test_authorized_keys_equal("algo key2 comments");
+
+ qmp_guest_ssh_add_authorized_keys(g_get_user_name(),
+ (strList *)&test_key1_2, &err);
+ g_assert_null(err);
+
+ /* key2 came first, and should'nt be duplicated */
+ test_authorized_keys_equal("algo key2 comments\n"
+ "algo key1 comments");
+}
+
+static void
+test_remove_keys(void)
+{
+ Error *err = NULL;
+ static const char *authkeys =
+ "algo key1 comments\n"
+ /* originally duplicated */
+ "algo key1 comments\n"
+ "# a commented line\n"
+ "algo some-key another\n";
+
+ test_authorized_keys_set(authkeys);
+ qmp_guest_ssh_remove_authorized_keys(g_get_user_name(),
+ (strList *)&test_key2, &err);
+ g_assert_null(err);
+ test_authorized_keys_equal(authkeys);
+
+ qmp_guest_ssh_remove_authorized_keys(g_get_user_name(),
+ (strList *)&test_key1_2, &err);
+ g_assert_null(err);
+ test_authorized_keys_equal("# a commented line\n"
+ "algo some-key another\n");
+}
+
+int main(int argc, char *argv[])
+{
+ setlocale(LC_ALL, "");
+
+ g_test_init(&argc, &argv, G_TEST_OPTION_ISOLATE_DIRS, NULL);
+
+ g_test_add_func("/qga/ssh/invalid_user", test_invalid_user);
+ g_test_add_func("/qga/ssh/invalid_key", test_invalid_key);
+ g_test_add_func("/qga/ssh/add_keys", test_add_keys);
+ g_test_add_func("/qga/ssh/remove_keys", test_remove_keys);
+
+ return g_test_run();
+}
+#else
+int main(int argc, char *argv[])
+{
+ g_test_message("test skipped, needs glib >= 2.60");
+ return 0;
+}
+#endif /* GLIB_2_60 */
+#endif /* BUILD_UNIT_TEST */
diff --git a/qga/meson.build b/qga/meson.build
index cd08bd953a..6315bb357e 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -35,7 +35,9 @@ qga_ss.add(files(
))
qga_ss.add(when: 'CONFIG_POSIX', if_true: files(
'channel-posix.c',
- 'commands-posix.c'))
+ 'commands-posix.c',
+ 'commands-posix-ssh.c',
+))
qga_ss.add(when: 'CONFIG_WIN32', if_true: files(
'channel-win32.c',
'commands-win32.c',
@@ -87,3 +89,19 @@ else
endif
alias_target('qemu-ga', all_qga)
+
+test_env = environment()
+test_env.set('G_TEST_SRCDIR', meson.current_source_dir())
+test_env.set('G_TEST_BUILDDIR', meson.current_build_dir())
+
+if 'CONFIG_POSIX' in config_host
+ qga_ssh_test = executable('qga-ssh-test',
+ files('commands-posix-ssh.c'),
+ dependencies: [qemuutil],
+ c_args: ['-DQGA_BUILD_UNIT_TEST'])
+
+ test('qga-ssh-test',
+ qga_ssh_test,
+ env: test_env,
+ suite: ['unit', 'qga'])
+endif
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index e123a000be..a2727ed86b 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1346,3 +1346,38 @@
##
{ 'command': 'guest-get-devices',
'returns': ['GuestDeviceInfo'] }
+
+##
+# @guest-ssh-add-authorized-keys:
+#
+# @username: the user account to add the authorized keys
+# @keys: the public keys to add (in OpenSSH/sshd(8) authorized_keys format)
+#
+# Append public keys to user .ssh/authorized_keys on Unix systems (not
+# implemented for other systems).
+#
+# Returns: Nothing on success.
+#
+# Since: 5.2
+##
+{ 'command': 'guest-ssh-add-authorized-keys',
+ 'data': { 'username': 'str', 'keys': ['str'] },
+ 'if': 'defined(CONFIG_POSIX)' }
+
+##
+# @guest-ssh-remove-authorized-keys:
+#
+# @username: the user account to remove the authorized keys
+# @keys: the public keys to remove (in OpenSSH/sshd(8) authorized_keys format)
+#
+# Remove public keys from the user .ssh/authorized_keys on Unix systems (not
+# implemented for other systems). It's not an error if the key is already
+# missing.
+#
+# Returns: Nothing on success.
+#
+# Since: 5.2
+##
+{ 'command': 'guest-ssh-remove-authorized-keys',
+ 'data': { 'username': 'str', 'keys': ['str'] },
+ 'if': 'defined(CONFIG_POSIX)' }
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PULL 09/12] qga: add ssh-{add,remove}-authorized-keys
2020-10-27 5:53 ` [PULL 09/12] qga: add ssh-{add,remove}-authorized-keys Michael Roth
@ 2020-11-02 15:49 ` Markus Armbruster
2020-11-03 2:11 ` Michael Roth
0 siblings, 1 reply; 18+ messages in thread
From: Markus Armbruster @ 2020-11-02 15:49 UTC (permalink / raw)
To: Michael Roth
Cc: Michal Privoznik, Daniel P . Berrangé,
qemu-devel, Marc-André Lureau, peter.maydell
Michael Roth <michael.roth@amd.com> writes:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Add new commands to add and remove SSH public keys from
> ~/.ssh/authorized_keys.
>
> I took a different approach for testing, including the unit tests right
> with the code. I wanted to overwrite the function to get the user
> details, I couldn't easily do that over QMP. Furthermore, I prefer
> having unit tests very close to the code, and unit files that are domain
> specific (commands-posix is too crowded already). FWIW, that
> coding/testing style is Rust-style (where tests can or should even be
> part of the documentation!).
>
> Fixes:
> https://bugzilla.redhat.com/show_bug.cgi?id=1885332
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> *squashed in fix-ups for setting file ownership and use of QAPI
> conditionals for CONFIG_POSIX instead of stub definitions
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> ---
> qga/commands-posix-ssh.c | 407 +++++++++++++++++++++++++++++++++++++++
> qga/meson.build | 20 +-
> qga/qapi-schema.json | 35 ++++
> 3 files changed, 461 insertions(+), 1 deletion(-)
> create mode 100644 qga/commands-posix-ssh.c
>
> diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
> new file mode 100644
> index 0000000000..a7bc9a1c24
> --- /dev/null
> +++ b/qga/commands-posix-ssh.c
> @@ -0,0 +1,407 @@
> + /*
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +#include "qemu/osdep.h"
> +
> +#include <glib-unix.h>
> +#include <glib/gstdio.h>
> +#include <locale.h>
> +#include <pwd.h>
> +
> +#include "qapi/error.h"
> +#include "qga-qapi-commands.h"
> +
> +#ifdef QGA_BUILD_UNIT_TEST
> +static struct passwd *
> +test_get_passwd_entry(const gchar *user_name, GError **error)
> +{
> + struct passwd *p;
> + int ret;
> +
> + if (!user_name || g_strcmp0(user_name, g_get_user_name())) {
> + g_set_error(error, G_UNIX_ERROR, 0, "Invalid user name");
> + return NULL;
> + }
> +
> + p = g_new0(struct passwd, 1);
> + p->pw_dir = (char *)g_get_home_dir();
> + p->pw_uid = geteuid();
> + p->pw_gid = getegid();
> +
> + ret = g_mkdir_with_parents(p->pw_dir, 0700);
> + g_assert_cmpint(ret, ==, 0);
checkpatch ERROR: Use g_assert or g_assert_not_reached
See commit 6e9389563e "checkpatch: Disallow glib asserts in main code"
for rationale.
More below, and in PATCH 10+12.
[...]
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PULL 09/12] qga: add ssh-{add,remove}-authorized-keys
2020-11-02 15:49 ` Markus Armbruster
@ 2020-11-03 2:11 ` Michael Roth
2020-11-03 6:25 ` Markus Armbruster
0 siblings, 1 reply; 18+ messages in thread
From: Michael Roth @ 2020-11-03 2:11 UTC (permalink / raw)
To: Markus Armbruster
Cc: Michal Privoznik, Daniel P . Berrangé,
qemu-devel, Marc-André Lureau, peter.maydell
On Mon, Nov 02, 2020 at 04:49:27PM +0100, Markus Armbruster wrote:
> Michael Roth <michael.roth@amd.com> writes:
>
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Add new commands to add and remove SSH public keys from
> > ~/.ssh/authorized_keys.
> >
> > I took a different approach for testing, including the unit tests right
> > with the code. I wanted to overwrite the function to get the user
> > details, I couldn't easily do that over QMP. Furthermore, I prefer
> > having unit tests very close to the code, and unit files that are domain
> > specific (commands-posix is too crowded already). FWIW, that
> > coding/testing style is Rust-style (where tests can or should even be
> > part of the documentation!).
> >
> > Fixes:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.redhat.com%2Fshow_bug.cgi%3Fid%3D1885332&data=04%7C01%7Cmichael.roth%40amd.com%7C7302cfdd51b547a8c3de08d87f46e6cf%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637399289788005891%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=IcYz5b1yBg3Q%2BPg82Z5VMdpf60vYHsLL518ENd0T1A4%3D&reserved=0
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > *squashed in fix-ups for setting file ownership and use of QAPI
> > conditionals for CONFIG_POSIX instead of stub definitions
> > Signed-off-by: Michael Roth <michael.roth@amd.com>
> > ---
> > qga/commands-posix-ssh.c | 407 +++++++++++++++++++++++++++++++++++++++
> > qga/meson.build | 20 +-
> > qga/qapi-schema.json | 35 ++++
> > 3 files changed, 461 insertions(+), 1 deletion(-)
> > create mode 100644 qga/commands-posix-ssh.c
> >
> > diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
> > new file mode 100644
> > index 0000000000..a7bc9a1c24
> > --- /dev/null
> > +++ b/qga/commands-posix-ssh.c
> > @@ -0,0 +1,407 @@
> > + /*
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +#include "qemu/osdep.h"
> > +
> > +#include <glib-unix.h>
> > +#include <glib/gstdio.h>
> > +#include <locale.h>
> > +#include <pwd.h>
> > +
> > +#include "qapi/error.h"
> > +#include "qga-qapi-commands.h"
> > +
> > +#ifdef QGA_BUILD_UNIT_TEST
> > +static struct passwd *
> > +test_get_passwd_entry(const gchar *user_name, GError **error)
> > +{
> > + struct passwd *p;
> > + int ret;
> > +
> > + if (!user_name || g_strcmp0(user_name, g_get_user_name())) {
> > + g_set_error(error, G_UNIX_ERROR, 0, "Invalid user name");
> > + return NULL;
> > + }
> > +
> > + p = g_new0(struct passwd, 1);
> > + p->pw_dir = (char *)g_get_home_dir();
> > + p->pw_uid = geteuid();
> > + p->pw_gid = getegid();
> > +
> > + ret = g_mkdir_with_parents(p->pw_dir, 0700);
> > + g_assert_cmpint(ret, ==, 0);
>
> checkpatch ERROR: Use g_assert or g_assert_not_reached
>
> See commit 6e9389563e "checkpatch: Disallow glib asserts in main code"
> for rationale.
Thanks for the pointer, I couldn't figure out what the issue was and
assumed it was just noise. Wish I noticed this message before I sent out
v2... v3 incoming.
>
> More below, and in PATCH 10+12.
>
> [...]
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PULL 09/12] qga: add ssh-{add,remove}-authorized-keys
2020-11-03 2:11 ` Michael Roth
@ 2020-11-03 6:25 ` Markus Armbruster
0 siblings, 0 replies; 18+ messages in thread
From: Markus Armbruster @ 2020-11-03 6:25 UTC (permalink / raw)
To: Michael Roth
Cc: peter.maydell, Michal Privoznik, Daniel P . Berrangé,
qemu-devel, Marc-André Lureau
Michael Roth <michael.roth@amd.com> writes:
> On Mon, Nov 02, 2020 at 04:49:27PM +0100, Markus Armbruster wrote:
[...]
>> checkpatch ERROR: Use g_assert or g_assert_not_reached
>>
>> See commit 6e9389563e "checkpatch: Disallow glib asserts in main code"
>> for rationale.
>
> Thanks for the pointer, I couldn't figure out what the issue was and
You can always ask :)
> assumed it was just noise. Wish I noticed this message before I sent out
> v2... v3 incoming.
Thanks!
[...]
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PULL 10/12] qga: add *reset argument to ssh-add-authorized-keys
2020-10-27 5:53 [PULL 00/12] qemu-ga patch queue for soft-freeze Michael Roth
` (8 preceding siblings ...)
2020-10-27 5:53 ` [PULL 09/12] qga: add ssh-{add,remove}-authorized-keys Michael Roth
@ 2020-10-27 5:53 ` Michael Roth
2020-10-27 5:53 ` [PULL 11/12] meson: minor simplification Michael Roth
` (3 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Michael Roth @ 2020-10-27 5:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
I prefer 'reset' over 'clear', since 'clear' and keys may have some
other relations or meaning.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
qga/commands-posix-ssh.c | 53 ++++++++++++++++++++++++++++++++++++----
qga/qapi-schema.json | 3 ++-
2 files changed, 50 insertions(+), 6 deletions(-)
diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
index a7bc9a1c24..f974bc4b64 100644
--- a/qga/commands-posix-ssh.c
+++ b/qga/commands-posix-ssh.c
@@ -168,6 +168,7 @@ read_authkeys(const char *path, Error **errp)
void
qmp_guest_ssh_add_authorized_keys(const char *username, strList *keys,
+ bool has_reset, bool reset,
Error **errp)
{
g_autofree struct passwd *p = NULL;
@@ -178,6 +179,7 @@ qmp_guest_ssh_add_authorized_keys(const char *username, strList *keys,
size_t nkeys, nauthkeys;
ERRP_GUARD();
+ reset = has_reset && reset;
if (!check_openssh_pub_keys(keys, &nkeys, errp)) {
return;
@@ -191,7 +193,9 @@ qmp_guest_ssh_add_authorized_keys(const char *username, strList *keys,
ssh_path = g_build_filename(p->pw_dir, ".ssh", NULL);
authkeys_path = g_build_filename(ssh_path, "authorized_keys", NULL);
- authkeys = read_authkeys(authkeys_path, NULL);
+ if (!reset) {
+ authkeys = read_authkeys(authkeys_path, NULL);
+ }
if (authkeys == NULL) {
if (!g_file_test(ssh_path, G_FILE_TEST_IS_DIR) &&
!mkdir_for_user(ssh_path, p, 0700, errp)) {
@@ -318,7 +322,7 @@ test_invalid_user(void)
{
Error *err = NULL;
- qmp_guest_ssh_add_authorized_keys("", NULL, &err);
+ qmp_guest_ssh_add_authorized_keys("", NULL, FALSE, FALSE, &err);
error_free_or_abort(&err);
qmp_guest_ssh_remove_authorized_keys("", NULL, &err);
@@ -333,7 +337,8 @@ test_invalid_key(void)
};
Error *err = NULL;
- qmp_guest_ssh_add_authorized_keys(g_get_user_name(), &key, &err);
+ qmp_guest_ssh_add_authorized_keys(g_get_user_name(), &key,
+ FALSE, FALSE, &err);
error_free_or_abort(&err);
qmp_guest_ssh_remove_authorized_keys(g_get_user_name(), &key, &err);
@@ -346,13 +351,17 @@ test_add_keys(void)
Error *err = NULL;
qmp_guest_ssh_add_authorized_keys(g_get_user_name(),
- (strList *)&test_key2, &err);
+ (strList *)&test_key2,
+ FALSE, FALSE,
+ &err);
g_assert_null(err);
test_authorized_keys_equal("algo key2 comments");
qmp_guest_ssh_add_authorized_keys(g_get_user_name(),
- (strList *)&test_key1_2, &err);
+ (strList *)&test_key1_2,
+ FALSE, FALSE,
+ &err);
g_assert_null(err);
/* key2 came first, and should'nt be duplicated */
@@ -360,6 +369,39 @@ test_add_keys(void)
"algo key1 comments");
}
+static void
+test_add_reset_keys(void)
+{
+ Error *err = NULL;
+
+ qmp_guest_ssh_add_authorized_keys(g_get_user_name(),
+ (strList *)&test_key1_2,
+ FALSE, FALSE,
+ &err);
+ g_assert_null(err);
+
+ /* reset with key2 only */
+ test_authorized_keys_equal("algo key1 comments\n"
+ "algo key2 comments");
+
+ qmp_guest_ssh_add_authorized_keys(g_get_user_name(),
+ (strList *)&test_key2,
+ TRUE, TRUE,
+ &err);
+ g_assert_null(err);
+
+ test_authorized_keys_equal("algo key2 comments");
+
+ /* empty should clear file */
+ qmp_guest_ssh_add_authorized_keys(g_get_user_name(),
+ (strList *)NULL,
+ TRUE, TRUE,
+ &err);
+ g_assert_null(err);
+
+ test_authorized_keys_equal("");
+}
+
static void
test_remove_keys(void)
{
@@ -393,6 +435,7 @@ int main(int argc, char *argv[])
g_test_add_func("/qga/ssh/invalid_user", test_invalid_user);
g_test_add_func("/qga/ssh/invalid_key", test_invalid_key);
g_test_add_func("/qga/ssh/add_keys", test_add_keys);
+ g_test_add_func("/qga/ssh/add_reset_keys", test_add_reset_keys);
g_test_add_func("/qga/ssh/remove_keys", test_remove_keys);
return g_test_run();
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index a2727ed86b..4ddea898fa 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1352,6 +1352,7 @@
#
# @username: the user account to add the authorized keys
# @keys: the public keys to add (in OpenSSH/sshd(8) authorized_keys format)
+# @reset: ignore the existing content, set it with the given keys only
#
# Append public keys to user .ssh/authorized_keys on Unix systems (not
# implemented for other systems).
@@ -1361,7 +1362,7 @@
# Since: 5.2
##
{ 'command': 'guest-ssh-add-authorized-keys',
- 'data': { 'username': 'str', 'keys': ['str'] },
+ 'data': { 'username': 'str', 'keys': ['str'], '*reset': 'bool' },
'if': 'defined(CONFIG_POSIX)' }
##
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 11/12] meson: minor simplification
2020-10-27 5:53 [PULL 00/12] qemu-ga patch queue for soft-freeze Michael Roth
` (9 preceding siblings ...)
2020-10-27 5:53 ` [PULL 10/12] qga: add *reset argument to ssh-add-authorized-keys Michael Roth
@ 2020-10-27 5:53 ` Michael Roth
2020-10-27 5:53 ` [PULL 12/12] qga: add ssh-get-authorized-keys Michael Roth
` (2 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Michael Roth @ 2020-10-27 5:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
qga/meson.build | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/qga/meson.build b/qga/meson.build
index 6315bb357e..8340892139 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -22,12 +22,7 @@ qga_qapi_files = custom_target('QGA QAPI files',
depend_files: qapi_gen_depends)
qga_ss = ss.source_set()
-i = 0
-foreach output: qga_qapi_outputs
- qga_ss.add(qga_qapi_files[i])
- i = i + 1
-endforeach
-
+qga_ss.add(qga_qapi_files.to_list())
qga_ss.add(files(
'commands.c',
'guest-agent-command-state.c',
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PULL 12/12] qga: add ssh-get-authorized-keys
2020-10-27 5:53 [PULL 00/12] qemu-ga patch queue for soft-freeze Michael Roth
` (10 preceding siblings ...)
2020-10-27 5:53 ` [PULL 11/12] meson: minor simplification Michael Roth
@ 2020-10-27 5:53 ` Michael Roth
2020-10-27 8:03 ` [PULL 00/12] qemu-ga patch queue for soft-freeze no-reply
2020-10-28 20:39 ` Peter Maydell
13 siblings, 0 replies; 18+ messages in thread
From: Michael Roth @ 2020-10-27 5:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
qga/commands-posix-ssh.c | 66 ++++++++++++++++++++++++++++++++++++++++
qga/meson.build | 11 +++++--
qga/qapi-schema.json | 31 +++++++++++++++++++
3 files changed, 106 insertions(+), 2 deletions(-)
diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
index f974bc4b64..4d75cb0113 100644
--- a/qga/commands-posix-ssh.c
+++ b/qga/commands-posix-ssh.c
@@ -268,6 +268,46 @@ qmp_guest_ssh_remove_authorized_keys(const char *username, strList *keys,
write_authkeys(authkeys_path, new_keys, p, errp);
}
+GuestAuthorizedKeys *
+qmp_guest_ssh_get_authorized_keys(const char *username, Error **errp)
+{
+ g_autofree struct passwd *p = NULL;
+ g_autofree char *authkeys_path = NULL;
+ g_auto(GStrv) authkeys = NULL;
+ g_autoptr(GuestAuthorizedKeys) ret = NULL;
+ int i;
+
+ ERRP_GUARD();
+
+ p = get_passwd_entry(username, errp);
+ if (p == NULL) {
+ return NULL;
+ }
+
+ authkeys_path = g_build_filename(p->pw_dir, ".ssh",
+ "authorized_keys", NULL);
+ authkeys = read_authkeys(authkeys_path, errp);
+ if (authkeys == NULL) {
+ return NULL;
+ }
+
+ ret = g_new0(GuestAuthorizedKeys, 1);
+ for (i = 0; authkeys[i] != NULL; i++) {
+ strList *new;
+
+ g_strstrip(authkeys[i]);
+ if (!authkeys[i][0] || authkeys[i][0] == '#') {
+ continue;
+ }
+
+ new = g_new0(strList, 1);
+ new->value = g_strdup(authkeys[i]);
+ new->next = ret->keys;
+ ret->keys = new;
+ }
+
+ return g_steal_pointer (&ret);
+}
#ifdef QGA_BUILD_UNIT_TEST
#if GLIB_CHECK_VERSION(2, 60, 0)
@@ -426,6 +466,31 @@ test_remove_keys(void)
"algo some-key another\n");
}
+static void
+test_get_keys(void)
+{
+ Error *err = NULL;
+ static const char *authkeys =
+ "algo key1 comments\n"
+ "# a commented line\n"
+ "algo some-key another\n";
+ g_autoptr(GuestAuthorizedKeys) ret = NULL;
+ strList *k;
+ size_t len = 0;
+
+ test_authorized_keys_set(authkeys);
+
+ ret = qmp_guest_ssh_get_authorized_keys(g_get_user_name(), &err);
+ g_assert_null(err);
+
+ for (len = 0, k = ret->keys; k != NULL; k = k->next) {
+ g_assert(g_str_has_prefix(k->value, "algo "));
+ len++;
+ }
+
+ g_assert_cmpint(len, ==, 2);
+}
+
int main(int argc, char *argv[])
{
setlocale(LC_ALL, "");
@@ -437,6 +502,7 @@ int main(int argc, char *argv[])
g_test_add_func("/qga/ssh/add_keys", test_add_keys);
g_test_add_func("/qga/ssh/add_reset_keys", test_add_reset_keys);
g_test_add_func("/qga/ssh/remove_keys", test_remove_keys);
+ g_test_add_func("/qga/ssh/get_keys", test_get_keys);
return g_test_run();
}
diff --git a/qga/meson.build b/qga/meson.build
index 8340892139..80e7487f32 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -90,8 +90,15 @@ test_env.set('G_TEST_SRCDIR', meson.current_source_dir())
test_env.set('G_TEST_BUILDDIR', meson.current_build_dir())
if 'CONFIG_POSIX' in config_host
- qga_ssh_test = executable('qga-ssh-test',
- files('commands-posix-ssh.c'),
+ srcs = [files('commands-posix-ssh.c')]
+ i = 0
+ foreach output: qga_qapi_outputs
+ if output.startswith('qga-qapi-types') or output.startswith('qga-qapi-visit')
+ srcs += qga_qapi_files[i]
+ endif
+ i = i + 1
+ endforeach
+ qga_ssh_test = executable('qga-ssh-test', srcs,
dependencies: [qemuutil],
c_args: ['-DQGA_BUILD_UNIT_TEST'])
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 4ddea898fa..6ca85f995f 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1347,6 +1347,37 @@
{ 'command': 'guest-get-devices',
'returns': ['GuestDeviceInfo'] }
+##
+# @GuestAuthorizedKeys:
+#
+# @keys: public keys (in OpenSSH/sshd(8) authorized_keys format)
+#
+# Since: 5.2
+##
+{ 'struct': 'GuestAuthorizedKeys',
+ 'data': {
+ 'keys': ['str']
+ },
+ 'if': 'defined(CONFIG_POSIX)' }
+
+
+##
+# @guest-ssh-get-authorized-keys:
+#
+# @username: the user account to add the authorized keys
+#
+# Return the public keys from user .ssh/authorized_keys on Unix systems (not
+# implemented for other systems).
+#
+# Returns: @GuestAuthorizedKeys
+#
+# Since: 5.2
+##
+{ 'command': 'guest-ssh-get-authorized-keys',
+ 'data': { 'username': 'str' },
+ 'returns': 'GuestAuthorizedKeys',
+ 'if': 'defined(CONFIG_POSIX)' }
+
##
# @guest-ssh-add-authorized-keys:
#
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PULL 00/12] qemu-ga patch queue for soft-freeze
2020-10-27 5:53 [PULL 00/12] qemu-ga patch queue for soft-freeze Michael Roth
` (11 preceding siblings ...)
2020-10-27 5:53 ` [PULL 12/12] qga: add ssh-get-authorized-keys Michael Roth
@ 2020-10-27 8:03 ` no-reply
2020-10-28 20:39 ` Peter Maydell
13 siblings, 0 replies; 18+ messages in thread
From: no-reply @ 2020-10-27 8:03 UTC (permalink / raw)
To: michael.roth; +Cc: peter.maydell, qemu-devel
Patchew URL: https://patchew.org/QEMU/20201027055317.351868-1-michael.roth@amd.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20201027055317.351868-1-michael.roth@amd.com
Subject: [PULL 00/12] qemu-ga patch queue for soft-freeze
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
- [tag update] patchew/20201010204106.1368710-1-marcandre.lureau@redhat.com -> patchew/20201010204106.1368710-1-marcandre.lureau@redhat.com
- [tag update] patchew/20201026195131.13848-1-jsnow@redhat.com -> patchew/20201026195131.13848-1-jsnow@redhat.com
- [tag update] patchew/20201027050556.269064-1-eblake@redhat.com -> patchew/20201027050556.269064-1-eblake@redhat.com
* [new tag] patchew/20201027055317.351868-1-michael.roth@amd.com -> patchew/20201027055317.351868-1-michael.roth@amd.com
Switched to a new branch 'test'
d9477b5 qga: add ssh-get-authorized-keys
244442a meson: minor simplification
4c591be qga: add *reset argument to ssh-add-authorized-keys
4857cb4 qga: add ssh-{add,remove}-authorized-keys
8ec1e2c glib-compat: add g_unix_get_passwd_entry_qemu()
20a139e qga: add implementation of guest-get-disks for Windows
4b631f0 qga: add implementation of guest-get-disks for Linux
7ae8fe2 qga: add command guest-get-disks
0b92ab2 qga: Flatten simple union GuestDeviceId
46c759d qga-win: Fix guest-get-devices error API violations
be683aa qga: Use common time encoding for guest-get-devices 'driver-date'
c9ae53d qga: Rename guest-get-devices return member 'address' to 'id'
=== OUTPUT BEGIN ===
1/12 Checking commit c9ae53d3e18d (qga: Rename guest-get-devices return member 'address' to 'id')
2/12 Checking commit be683aab37bc (qga: Use common time encoding for guest-get-devices 'driver-date')
3/12 Checking commit 46c759dcdc69 (qga-win: Fix guest-get-devices error API violations)
4/12 Checking commit 0b92ab2b2592 (qga: Flatten simple union GuestDeviceId)
5/12 Checking commit 7ae8fe2e031c (qga: add command guest-get-disks)
6/12 Checking commit 4b631f069f98 (qga: add implementation of guest-get-disks for Linux)
7/12 Checking commit 20a139e50765 (qga: add implementation of guest-get-disks for Windows)
8/12 Checking commit 8ec1e2cbda3f (glib-compat: add g_unix_get_passwd_entry_qemu())
WARNING: Block comments use a leading /* on a separate line
#42: FILE: include/glib-compat.h:81:
+/* Note: The fallback implementation is not MT-safe, and it returns a copy of
WARNING: Block comments use a trailing */ on a separate line
#45: FILE: include/glib-compat.h:84:
+ * GLib API substitution. */
total: 0 errors, 2 warnings, 38 lines checked
Patch 8/12 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/12 Checking commit 4857cb4903be (qga: add ssh-{add,remove}-authorized-keys)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#30:
new file mode 100644
ERROR: Use g_assert or g_assert_not_reached
#67: FILE: qga/commands-posix-ssh.c:33:
+ g_assert_cmpint(ret, ==, 0);
ERROR: Use g_assert or g_assert_not_reached
#328: FILE: qga/commands-posix-ssh.c:294:
+ g_assert_cmpint(ret, ==, 0);
ERROR: Use g_assert or g_assert_not_reached
#333: FILE: qga/commands-posix-ssh.c:299:
+ g_assert_no_error(err);
ERROR: Use g_assert or g_assert_not_reached
#345: FILE: qga/commands-posix-ssh.c:311:
+ g_assert_no_error(err);
ERROR: Use g_assert or g_assert_not_reached
#347: FILE: qga/commands-posix-ssh.c:313:
+ g_assert_cmpstr(contents, ==, expected);
ERROR: Use g_assert or g_assert_not_reached
#384: FILE: qga/commands-posix-ssh.c:350:
+ g_assert_null(err);
ERROR: Use g_assert or g_assert_not_reached
#390: FILE: qga/commands-posix-ssh.c:356:
+ g_assert_null(err);
ERROR: Use g_assert or g_assert_not_reached
#411: FILE: qga/commands-posix-ssh.c:377:
+ g_assert_null(err);
ERROR: Use g_assert or g_assert_not_reached
#416: FILE: qga/commands-posix-ssh.c:382:
+ g_assert_null(err);
total: 9 errors, 1 warnings, 474 lines checked
Patch 9/12 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/12 Checking commit 4c591be51737 (qga: add *reset argument to ssh-add-authorized-keys)
ERROR: Use g_assert or g_assert_not_reached
#96: FILE: qga/commands-posix-ssh.c:381:
+ g_assert_null(err);
ERROR: Use g_assert or g_assert_not_reached
#106: FILE: qga/commands-posix-ssh.c:391:
+ g_assert_null(err);
ERROR: Use g_assert or g_assert_not_reached
#115: FILE: qga/commands-posix-ssh.c:400:
+ g_assert_null(err);
total: 3 errors, 0 warnings, 121 lines checked
Patch 10/12 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/12 Checking commit 244442a80936 (meson: minor simplification)
12/12 Checking commit d9477b5e77e2 (qga: add ssh-get-authorized-keys)
ERROR: space prohibited between function name and open parenthesis '('
#57: FILE: qga/commands-posix-ssh.c:309:
+ return g_steal_pointer (&ret);
ERROR: Use g_assert or g_assert_not_reached
#81: FILE: qga/commands-posix-ssh.c:484:
+ g_assert_null(err);
ERROR: Use g_assert or g_assert_not_reached
#88: FILE: qga/commands-posix-ssh.c:491:
+ g_assert_cmpint(len, ==, 2);
total: 3 errors, 0 warnings, 138 lines checked
Patch 12/12 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20201027055317.351868-1-michael.roth@amd.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PULL 00/12] qemu-ga patch queue for soft-freeze
2020-10-27 5:53 [PULL 00/12] qemu-ga patch queue for soft-freeze Michael Roth
` (12 preceding siblings ...)
2020-10-27 8:03 ` [PULL 00/12] qemu-ga patch queue for soft-freeze no-reply
@ 2020-10-28 20:39 ` Peter Maydell
13 siblings, 0 replies; 18+ messages in thread
From: Peter Maydell @ 2020-10-28 20:39 UTC (permalink / raw)
To: Michael Roth; +Cc: QEMU Developers
On Tue, 27 Oct 2020 at 05:53, Michael Roth <michael.roth@amd.com> wrote:
>
> The following changes since commit a46e72710566eea0f90f9c673a0f02da0064acce:
>
> Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20201026' into staging (2020-10-26 14:50:03 +0000)
>
> are available in the Git repository at:
>
> git://github.com/mdroth/qemu.git tags/qga-pull-2020-10-27-tag
>
> for you to fetch changes up to 568979ea819d945e8af6c14658793b58bcd4485c:
>
> qga: add ssh-get-authorized-keys (2020-10-27 00:22:30 -0500)
>
> ----------------------------------------------------------------
> qemu-ga patch queue for soft-freeze
>
> * add guest-get-disks for w32/linux
> * add guest-{add,remove,get}-authorized-keys
> * fix API violations and schema documentation inconsistencies with
> recently-added guest-get-devices
>
> ----------------------------------------------------------------
Link failure trying to link qemu-ga on OSX and the BSDs:
ld: error: undefined symbol: qmp_guest_get_disks
>>> referenced by qga-qapi-commands.c:987 (qga/qga-qapi-commands.c:987)
>>> qga/qemu-ga.p/meson-generated_.._qga-qapi-commands.c.o:(qmp_marshal_guest_get_disks)
and a compile failure with Clang on x86-64 Linux:
../../qga/commands-posix.c:1383:33: error: unused variable 'deps_dir'
[-Werror,-Wunused-variable]
*size_path = NULL, *deps_dir = NULL;
^
1 error generated.
thanks
-- PMM
^ permalink raw reply [flat|nested] 18+ messages in thread