On 25.09.2014 17:00, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1141621
The crash in this case was because the qemu-attach code did not create
aliases for qemu cli args. When the detach-interface code was called
it assumed aliases were set resulting in a core when dereferencing the
still NULL alias.
Adding a call to qemuAssignDeviceAliases() resolves the path for
qemu-attach; however, to prevent future issues an additional check
for a NULL value is made and an error provided in the deatch path.
Add some more verbiage to the virsh man page.
Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
v1 is here:
http://www.redhat.com/archives/libvir-list/2014-September/msg01331.html
Changes since v1:
- Add the call to qemuAssignDeviceAliases() in qemuDomainQemuAttach().
- Move the check for NULL alias inside the CAPS_DEVICE check and emit
an error rather than trying to remove as an "else" condition.
src/qemu/qemu_driver.c | 3 +++
src/qemu/qemu_hotplug.c | 7 +++++++
tools/virsh.pod | 5 +++--
3 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 117138a..ef4ecd2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14746,6 +14746,9 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn,
if (qemuCanonicalizeMachine(def, qemuCaps) < 0)
goto cleanup;
+ if (qemuAssignDeviceAliases(def, qemuCaps) < 0)
+ goto cleanup;
+
if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0)
goto cleanup;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index d631887..daebe82 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3521,6 +3521,13 @@ qemuDomainDetachNetDevice(virConnectPtr conn,
qemuDomainObjEnterMonitor(driver, vm);
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
+ if (!detach->info.alias) {
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("device alias not found: cannot delete the "
+ "net device"));
+ qemuDomainObjExitMonitor(driver, vm);
+ goto cleanup;
+ }
Is there a reason to not check this upfront rather than this late in the
process? And I don't think that net devices are the only thing affected
here. For instance a virtio disks seems vulnerable too (well, at first
glance on the code).
if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) {
The other possibility would be to make qemuMonitorDelDevice fail on
@devalias == NULL.
qemuDomainObjExitMonitor(driver, vm);
virDomainAuditNet(vm, detach, NULL, "detach", false);
diff --git a/tools/virsh.pod b/tools/virsh.pod
index eae9195..bd17f68 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -3698,8 +3698,9 @@ using the UNIX driver. Ideally the process will also have had the
Not all functions of libvirt are expected to work reliably after
attaching to an externally launched QEMU process. There may be
-issues with the guest ABI changing upon migration, and hotunplug
-may not work.
+issues with the guest ABI changing upon migration and device hotplug
+or hotunplug may not work. The attached environment should be considered
+primarily read-only.
=item B<qemu-monitor-command> I<domain> { [I<--hmp>] | [I<--pretty>] }
I<command>...
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list