4:33 p.m.
This is v2 of:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/PN...
Changes since v1:
- Report more accurate errors instead of suppressing the error message
when the drive is empty.
- Add check for this condition earlier in the API code.
Fabian Leditzky (2):
qemu: Check empty drives in block I/O throttle API
qemu: Clarify block I/O throttle JSON parsing errors
src/qemu/qemu_driver.c | 8 ++++++++
src/qemu/qemu_monitor_json.c | 11 +++++++++--
2 files changed, 17 insertions(+), 2 deletions(-)
--
2.48.1
4:33 p.m.
New subject: [PATCH v2 1/2] qemu: Check empty drives in block I/O throttle API
Provide a proper user facing error when attempting to query block
I/O throttling settings for an empty drive. Without this patch, a less
meaningful internal error produced by qemuMonitorJSONBlockIoThrottleInfo
would be propagated to the user.
Signed-off-by: Fabian Leditzky <fabian(a)ldsoft.dev>
---
src/qemu/qemu_driver.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d2eddbd9ae..1dd30ed444 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15368,6 +15368,14 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
if (!qemuDomainDiskBlockIoTuneIsSupported(disk))
goto endjob;
+ /* qemu won't return block IO throttle settings for an empty cd-rom drive */
+ if (virStorageSourceIsEmpty(disk->src)) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("disk '%1$s' does not currently have a source
assigned"),
+ path);
+ goto endjob;
+ }
+
qemuDomainObjEnterMonitor(vm);
rc = qemuMonitorGetBlockIoThrottle(priv->mon,
QEMU_DOMAIN_DISK_PRIVATE(disk)->qomName, &reply);
qemuDomainObjExitMonitor(vm);
--
2.48.1
4:33 p.m.
New subject: [PATCH v2 2/2] qemu: Clarify block I/O throttle JSON parsing errors
The original code incorrectly assumes that the 'inserted' entry is not
in the right format (wrong JSON value type, not an 'Object') when
virJSONValueObjectGetObject(temp_dev, "inserted") returns NULL. However,
NULL is also returned when the 'inserted' entry is simply not present.
This is the case when querying an empty CD-ROM device.
With this patch, both conditions are reported accurately.
Signed-off-by: Fabian Leditzky <fabian(a)ldsoft.dev>
---
src/qemu/qemu_monitor_json.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 9f417d27c6..06e7bb07a3 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4581,8 +4581,15 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValue *io_throttle,
found = true;
if (!(inserted = virJSONValueObjectGetObject(temp_dev, "inserted"))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("block_io_throttle inserted entry was not in expected
format"));
+ if (virJSONValueObjectGet(temp_dev, "inserted")) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("block_io_throttle inserted entry was not in
expected format"));
+ } else {
+ /* caller should validate source presence prior, hence this is an
internal error */
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("block_io_throttle inserted entry is not
present"));
+ }
+
return -1;
}
GET_THROTTLE_STATS("bps", total_bytes_sec);
--
2.48.1
8
days inactive
8
days old
2 comments
1 participants
participants (1)
-
Fabian Leditzky