1:29 p.m.
qemu in Fedora 30 rejects migration if the VM has nested VMX
configured. This means managedsave and snapshots are rejected too.
Unfortunately the scraping we need to resort to with the text monitor
snapshot commands is not detecting that 'savevm' actually failed, so
the snapshot appears to succeed.
https://bugzilla.redhat.com/show_bug.cgi?id=1697997
qemu 4.0.0 will add an 'Error: ' prefix to most hmp command errors
(qemu commit 66363e9a43f), let's use that to detect error as well
Cole Robinson (3):
qemu: monitor: cleanup loadvm error handling
qemu: monitor cleanup delvm error handling
qemu: monitor: check for common 'Error: ' string
src/qemu/qemu_monitor_text.c | 39 ++++++++++++++++++------------------
1 file changed, 20 insertions(+), 19 deletions(-)
--
2.21.0
1:29 p.m.
New subject: [libvirt] [PATCH 1/3] qemu: monitor: cleanup loadvm error handling
Drop redundant NULL checks, add error string prefixes, consolidate
a few indentical reports.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu/qemu_monitor_text.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 0b2c1a6aed..414da0ca28 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -215,28 +215,25 @@ int qemuMonitorTextLoadSnapshot(qemuMonitorPtr mon, const char
*name)
if (qemuMonitorHMPCommand(mon, cmd, &reply))
goto cleanup;
- if (strstr(reply, "No block device supports snapshots") != NULL) {
+ if (strstr(reply, "No block device supports snapshots")) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("this domain does not have a device to load
snapshots"));
goto cleanup;
- } else if (strstr(reply, "Could not find snapshot") != NULL) {
+ } else if (strstr(reply, "Could not find snapshot")) {
virReportError(VIR_ERR_OPERATION_INVALID,
_("the snapshot '%s' does not exist, and was not
loaded"),
name);
goto cleanup;
- } else if (strstr(reply, "Snapshots not supported on device") != NULL) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s", reply);
- goto cleanup;
- } else if (strstr(reply, "Could not open VM state file") != NULL) {
- virReportError(VIR_ERR_OPERATION_FAILED, "%s", reply);
- goto cleanup;
- } else if (strstr(reply, "Error") != NULL
- && strstr(reply, "while loading VM state") != NULL) {
- virReportError(VIR_ERR_OPERATION_FAILED, "%s", reply);
+ } else if (strstr(reply, "Snapshots not supported on device")) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("Failed to load snapshot: %s"), reply);
goto cleanup;
- } else if (strstr(reply, "Error") != NULL
- && strstr(reply, "while activating snapshot on") != NULL)
{
- virReportError(VIR_ERR_OPERATION_FAILED, "%s", reply);
+ } else if (strstr(reply, "Could not open VM state file") ||
+ (strstr(reply, "Error") &&
+ (strstr(reply, "while loading VM state") ||
+ strstr(reply, "while activating snapshot on")))) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Failed to load snapshot: %s"), reply);
goto cleanup;
}
--
2.21.0
1:29 p.m.
New subject: [libvirt] [PATCH 2/3] qemu: monitor cleanup delvm error handling
Drop redundant NULL checks, and add an error string prefix
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu/qemu_monitor_text.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 414da0ca28..76095674ab 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -259,16 +259,17 @@ int qemuMonitorTextDeleteSnapshot(qemuMonitorPtr mon, const char
*name)
if (qemuMonitorHMPCommand(mon, cmd, &reply))
goto cleanup;
- if (strstr(reply, "No block device supports snapshots") != NULL) {
+ if (strstr(reply, "No block device supports snapshots")) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("this domain does not have a device to delete
snapshots"));
goto cleanup;
- } else if (strstr(reply, "Snapshots not supported on device") != NULL) {
+ } else if (strstr(reply, "Snapshots not supported on device")) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s", reply);
goto cleanup;
- } else if (strstr(reply, "Error") != NULL
- && strstr(reply, "while deleting snapshot") != NULL) {
- virReportError(VIR_ERR_OPERATION_FAILED, "%s", reply);
+ } else if (strstr(reply, "Error") &&
+ strstr(reply, "while deleting snapshot")) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Failed to delete snapshot: %s"), reply);
goto cleanup;
}
--
2.21.0
1:29 p.m.
New subject: [libvirt] [PATCH 3/3] qemu: monitor: check for common 'Error: ' string
qemu 4.0.0 will prefix most errors with 'Error: ', so consider any
string instance of that an error.
This fixes savevm failure detection when migration is blocked due to
usage of nested VMX
https://bugzilla.redhat.com/show_bug.cgi?id=1697997
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu/qemu_monitor_text.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 76095674ab..280cc58840 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -182,6 +182,7 @@ qemuMonitorTextCreateSnapshot(qemuMonitorPtr mon,
if (strstr(reply, "Error while creating snapshot") ||
strstr(reply, "Could not open VM state file") ||
strstr(reply, "State blocked by non-migratable device") ||
+ strstr(reply, "Error: ") ||
(strstr(reply, "Error") && strstr(reply, "while writing
VM"))) {
virReportError(VIR_ERR_OPERATION_FAILED,
_("Failed to take snapshot: %s"), reply);
@@ -229,6 +230,7 @@ int qemuMonitorTextLoadSnapshot(qemuMonitorPtr mon, const char *name)
_("Failed to load snapshot: %s"), reply);
goto cleanup;
} else if (strstr(reply, "Could not open VM state file") ||
+ strstr(reply, "Error: ") ||
(strstr(reply, "Error") &&
(strstr(reply, "while loading VM state") ||
strstr(reply, "while activating snapshot on")))) {
@@ -266,8 +268,9 @@ int qemuMonitorTextDeleteSnapshot(qemuMonitorPtr mon, const char
*name)
} else if (strstr(reply, "Snapshots not supported on device")) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s", reply);
goto cleanup;
- } else if (strstr(reply, "Error") &&
- strstr(reply, "while deleting snapshot")) {
+ } else if (strstr(reply, "Error: ") ||
+ (strstr(reply, "Error") &&
+ strstr(reply, "while deleting snapshot"))) {
virReportError(VIR_ERR_OPERATION_FAILED,
_("Failed to delete snapshot: %s"), reply);
goto cleanup;
--
2.21.0
4:04 a.m.
On 4/10/19 8:29 PM, Cole Robinson wrote:
qemu in Fedora 30 rejects migration if the VM has nested VMX
configured. This means managedsave and snapshots are rejected too.
Unfortunately the scraping we need to resort to with the text monitor
snapshot commands is not detecting that 'savevm' actually failed, so
the snapshot appears to succeed.
https://bugzilla.redhat.com/show_bug.cgi?id=1697997
qemu 4.0.0 will add an 'Error: ' prefix to most hmp command errors
(qemu commit 66363e9a43f), let's use that to detect error as well
Cole Robinson (3):
qemu: monitor: cleanup loadvm error handling
qemu: monitor cleanup delvm error handling
qemu: monitor: check for common 'Error: ' string
src/qemu/qemu_monitor_text.c | 39 ++++++++++++++++++------------------
1 file changed, 20 insertions(+), 19 deletions(-)
ACK
Michal
1993
days inactive
2006
days old
4 comments
2 participants
participants (2)
-
Cole Robinson -
Michal Privoznik