9:56 a.m.
Ján Tomko (2):
Fix crash after attempting to hotplug a spicevmc channel
Display nicer error message for unsupported chardev hotplug
src/conf/domain_conf.c | 3 +--
src/qemu/qemu_monitor_json.c | 12 +++++++++---
2 files changed, 10 insertions(+), 5 deletions(-)
--
2.0.4
9:56 a.m.
New subject: [libvirt] [PATCH 1/2] Fix crash after attempting to hotplug a spicevmc channel
In qemuDomainAttachChrDevice, we add the device to the domain XML,
then we fail to construct the JSON command, saying we don't support
it.
But we don't clean up the device from the domain XML, because
virDomainChrEquals returns false when comparing the device against
itself.
https://bugzilla.redhat.com/show_bug.cgi?id=1162097
---
src/conf/domain_conf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e8d8f7d..31378cd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1596,8 +1596,7 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
case VIR_DOMAIN_CHR_TYPE_STDIO:
case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
case VIR_DOMAIN_CHR_TYPE_LAST:
- /* nada */
- break;
+ return true;
}
return false;
--
2.0.4
11:03 a.m.
New subject: [libvirt] [PATCH 1/2] Fix crash after attempting to hotplug a spicevmc channel
On 11/10/2014 04:56 PM, Ján Tomko wrote:
In qemuDomainAttachChrDevice, we add the device to the domain XML,
then we fail to construct the JSON command, saying we don't support
it.
But we don't clean up the device from the domain XML, because
virDomainChrEquals returns false when comparing the device against
itself.
https://bugzilla.redhat.com/show_bug.cgi?id=1162097
---
src/conf/domain_conf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e8d8f7d..31378cd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1596,8 +1596,7 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
case VIR_DOMAIN_CHR_TYPE_STDIO:
case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
case VIR_DOMAIN_CHR_TYPE_LAST:
- /* nada */
- break;
+ return true;
}
return false;
This isn't true for VIR_DOMAIN_CHR_TYPE_SPICEVMC as we are setting
data.spicevmc to some value and it could have different value.
See enum virDomainChrSpicevmcName.
Pavel
5:23 a.m.
New subject: [libvirt] [PATCHv2] Fix virDomainChrEquals for spicevmc
virDomainChrSourceDefIsEqual should return 'true' for
identical SPICEVMC chardevs, and those that have no source
specification.
After this change, a failed hotplug no longer leaves a stale
pointer in the domain definition.
https://bugzilla.redhat.com/show_bug.cgi?id=1162097
---
src/conf/domain_conf.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 54b2bfe..73b2393 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1591,13 +1591,14 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
tgt->data.spiceport.channel);
break;
+ case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
+ return src->data.spicevmc == tgt->data.spicevmc;
+
case VIR_DOMAIN_CHR_TYPE_NULL:
case VIR_DOMAIN_CHR_TYPE_VC:
case VIR_DOMAIN_CHR_TYPE_STDIO:
- case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
case VIR_DOMAIN_CHR_TYPE_LAST:
- /* nada */
- break;
+ return true;
}
return false;
--
2.0.4
6:23 a.m.
New subject: [libvirt] [PATCHv2] Fix virDomainChrEquals for spicevmc
On 11/11/2014 12:23 PM, Ján Tomko wrote:
virDomainChrSourceDefIsEqual should return 'true' for
identical SPICEVMC chardevs, and those that have no source
specification.
After this change, a failed hotplug no longer leaves a stale
pointer in the domain definition.
https://bugzilla.redhat.com/show_bug.cgi?id=1162097
---
src/conf/domain_conf.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 54b2bfe..73b2393 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1591,13 +1591,14 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
tgt->data.spiceport.channel);
break;
+ case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
+ return src->data.spicevmc == tgt->data.spicevmc;
+
case VIR_DOMAIN_CHR_TYPE_NULL:
case VIR_DOMAIN_CHR_TYPE_VC:
case VIR_DOMAIN_CHR_TYPE_STDIO:
- case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
case VIR_DOMAIN_CHR_TYPE_LAST:
- /* nada */
- break;
+ return true;
}
return false;
ACK,
Pavel
6:34 a.m.
New subject: [libvirt] [PATCHv2] Fix virDomainChrEquals for spicevmc
On Tue, Nov 11, 2014 at 12:23:44PM +0100, Ján Tomko wrote:
virDomainChrSourceDefIsEqual should return 'true' for
identical SPICEVMC chardevs, and those that have no source
specification.
After this change, a failed hotplug no longer leaves a stale
pointer in the domain definition.
https://bugzilla.redhat.com/show_bug.cgi?id=1162097
---
src/conf/domain_conf.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 54b2bfe..73b2393 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1591,13 +1591,14 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
tgt->data.spiceport.channel);
break;
+ case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
+ return src->data.spicevmc == tgt->data.spicevmc;
+
case VIR_DOMAIN_CHR_TYPE_NULL:
case VIR_DOMAIN_CHR_TYPE_VC:
case VIR_DOMAIN_CHR_TYPE_STDIO:
- case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
case VIR_DOMAIN_CHR_TYPE_LAST:
- /* nada */
- break;
+ return true;
}
return false;
Probably a dead code and very possibly an ewww statement. Either
remove it or change it to true and remove the one you added few lines
up, please.
Martin
9:57 a.m.
New subject: [libvirt] [PATCH 2/2] Display nicer error message for unsupported chardev hotplug
Use the device type name if we know it instead of its number,
even if we can't hotplug it:
qemuMonitorJSONAttachCharDevCommand:6094 : operation failed: Unsupported
char device type '10'
---
src/qemu/qemu_monitor_json.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 7870664..2d082bc 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5915,9 +5915,15 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
case VIR_DOMAIN_CHR_TYPE_STDIO:
case VIR_DOMAIN_CHR_TYPE_NMDM:
case VIR_DOMAIN_CHR_TYPE_LAST:
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("Unsupported char device type '%d'"),
- chr->type);
+ if (virDomainChrTypeToString(chr->type)) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Hotplug unsupported char device type
'%s'"),
+ virDomainChrTypeToString(chr->type));
+ } else {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Unsupported char device type '%d'"),
+ chr->type);
+ }
goto error;
}
--
2.0.4
11:05 a.m.
New subject: [libvirt] [PATCH 2/2] Display nicer error message for unsupported chardev hotplug
On 11/10/2014 04:57 PM, Ján Tomko wrote:
Use the device type name if we know it instead of its number,
even if we can't hotplug it:
qemuMonitorJSONAttachCharDevCommand:6094 : operation failed: Unsupported
char device type '10'
---
src/qemu/qemu_monitor_json.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 7870664..2d082bc 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5915,9 +5915,15 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
case VIR_DOMAIN_CHR_TYPE_STDIO:
case VIR_DOMAIN_CHR_TYPE_NMDM:
case VIR_DOMAIN_CHR_TYPE_LAST:
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("Unsupported char device type '%d'"),
- chr->type);
+ if (virDomainChrTypeToString(chr->type)) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Hotplug unsupported char device type
'%s'"),
+ virDomainChrTypeToString(chr->type));
+ } else {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Unsupported char device type '%d'"),
+ chr->type);
+ }
goto error;
}
Both error messages could be the same as they are reporting the same
issue. ACK with that change.
Pavel
3664
days inactive
3665
days old
7 comments
3 participants
participants (3)
-
Ján Tomko -
Martin Kletzander -
Pavel Hrdina