7:08 p.m.
v2 is here:
http://www.redhat.com/archives/libvir-list/2014-September/msg01575.html
As Michal pointed out in his review - other devices could have the
same issue - so take care of each of them separately (I already knew
the answer to my last question...)
Totally different approach this time - rather than error out, follow
the example I from the Controller Detach code which will generate an
alias for the device (although it did miss one minor check to see if
it was already set leading to a potential memory leak since the alias
code would overwrite whatever was there).
Note: In a way 1/7 was already ACK'd - I just hadn't separated it yet
for a push and wanted to keep these closer together when/if they were
pushed.
Whether 7/7 is now necessary is debateable - I keep it only for
completeness and environment setup in much the same way the start
code handles aliases.
John Ferlan (7):
virsh: Adjust the text in man page regarding qemu-attach
hotplug: Check for alias in controller detach
hotplug: Check for alias in disk detach
hotplug: Check for alias in hostdev detach
hotplug: Check for alias in chrdev detach
hotplug: Check for alias in net detach
qemu-attach: Assign device aliases
src/qemu/qemu_driver.c | 3 +++
src/qemu/qemu_hotplug.c | 28 +++++++++++++++++++++++++++-
tools/virsh.pod | 5 +++--
3 files changed, 33 insertions(+), 3 deletions(-)
--
1.9.3
7:08 p.m.
New subject: [libvirt] [PATCH v3 1/7] virsh: Adjust the text in man page regarding qemu-attach
Slight adjustment to the qemu-attach man page to note device hotplug
and hot unplug may not work and that the environment should be considered
read-only
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
tools/virsh.pod | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
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>...
--
1.9.3
7:08 p.m.
New subject: [libvirt] [PATCH v3 2/7] hotplug: Check for alias in controller detach
In qemuDomainDetachControllerDevice if the info.alias already exists
a call to qemuAssignDeviceControllerAlias would overwrite the existing
so avoid this possibility.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1e504ec..9c0f6c9 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3226,7 +3226,8 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
goto cleanup;
}
- if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
+ if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) &&
+ !detach->info.alias) {
if (qemuAssignDeviceControllerAlias(detach) < 0)
goto cleanup;
}
--
1.9.3
7:08 p.m.
New subject: [libvirt] [PATCH v3 3/7] hotplug: Check for alias in disk detach
If the QEMU_CAPS_DEVICE is set, then ensure the disk device alias has
been properly set in prior to making the calls to detach the device.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9c0f6c9..3e8cdbf 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2998,6 +2998,12 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
}
}
+ if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) &&
+ !detach->info.alias) {
+ if (qemuAssignDeviceDiskAlias(vm->def, detach, priv->qemuCaps) < 0)
+ goto cleanup;
+ }
+
qemuDomainMarkDeviceForRemoval(vm, &detach->info);
qemuDomainObjEnterMonitor(driver, vm);
--
1.9.3
7:08 p.m.
New subject: [libvirt] [PATCH v3 4/7] hotplug: Check for alias in hostdev detach
If the QEMU_CAPS_DEVICE is set, then ensure the host device alias has
been properly set before making the calls to detach the device
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 3e8cdbf..db39948 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3371,8 +3371,15 @@ qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainHostdevDefPtr detach)
{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
int ret = -1;
+ if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) &&
+ !detach->info->alias) {
+ if (qemuAssignDeviceHostdevAlias(vm->def, detach, -1) < 0)
+ return -1;
+ }
+
switch (detach->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
ret = qemuDomainDetachHostPCIDevice(driver, vm, detach);
--
1.9.3
7:08 p.m.
New subject: [libvirt] [PATCH v3 5/7] hotplug: Check for alias in chrdev detach
If the QEMU_CAPS_DEVICE is set, then ensure the chr device alias has
been properly set before making the calls to detach the device
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index db39948..f79a37a 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3696,6 +3696,12 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
return ret;
}
+ if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) &&
+ !tmpChr->info.alias) {
+ if (qemuAssignDeviceChrAlias(vmdef, tmpChr, -1) < 0)
+ return ret;
+ }
+
if (qemuBuildChrDeviceStr(&devstr, vm->def, chr, priv->qemuCaps) < 0)
return ret;
--
1.9.3
7:08 p.m.
New subject: [libvirt] [PATCH v3 6/7] hotplug: Check for alias in net detach
https://bugzilla.redhat.com/show_bug.cgi?id=1141621
If the QEMU_CAPS_DEVICE is set, then ensure the host device alias has
been properly set before making the calls to detach the device
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f79a37a..33241fb 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3530,6 +3530,12 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver,
}
}
+ if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) &&
+ !detach->info.alias) {
+ if (qemuAssignDeviceNetAlias(vm->def, detach, -1) < 0)
+ goto cleanup;
+ }
+
qemuDomainMarkDeviceForRemoval(vm, &detach->info);
qemuDomainObjEnterMonitor(driver, vm);
--
1.9.3
7:08 p.m.
New subject: [libvirt] [PATCH v3 7/7] qemu-attach: Assign device aliases
https://bugzilla.redhat.com/show_bug.cgi?id=1141621
As part of attach processing, assign the device aliases by calling
qemuAssignDeviceAliases during qemuDomainQemuAttach once all the devices
are found after the qemuParseCommandLinePid processing.
This will alleviate a symptom that caused a libvirtd crash during an
attempted device detach.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4e2b356..57daa67 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14862,6 +14862,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;
--
1.9.3
2:59 a.m.
ping?
Tks,
John
On 10/08/2014 08:08 PM, John Ferlan wrote:
v2 is here:
http://www.redhat.com/archives/libvir-list/2014-September/msg01575.html
As Michal pointed out in his review - other devices could have the
same issue - so take care of each of them separately (I already knew
the answer to my last question...)
Totally different approach this time - rather than error out, follow
the example I from the Controller Detach code which will generate an
alias for the device (although it did miss one minor check to see if
it was already set leading to a potential memory leak since the alias
code would overwrite whatever was there).
Note: In a way 1/7 was already ACK'd - I just hadn't separated it yet
for a push and wanted to keep these closer together when/if they were
pushed.
Whether 7/7 is now necessary is debateable - I keep it only for
completeness and environment setup in much the same way the start
code handles aliases.
John Ferlan (7):
virsh: Adjust the text in man page regarding qemu-attach
hotplug: Check for alias in controller detach
hotplug: Check for alias in disk detach
hotplug: Check for alias in hostdev detach
hotplug: Check for alias in chrdev detach
hotplug: Check for alias in net detach
qemu-attach: Assign device aliases
src/qemu/qemu_driver.c | 3 +++
src/qemu/qemu_hotplug.c | 28 +++++++++++++++++++++++++++-
tools/virsh.pod | 5 +++--
3 files changed, 33 insertions(+), 3 deletions(-)
12:10 p.m.
Ping again
Tks,
John
On 10/08/2014 08:08 PM, John Ferlan wrote:
v2 is here:
http://www.redhat.com/archives/libvir-list/2014-September/msg01575.html
As Michal pointed out in his review - other devices could have the
same issue - so take care of each of them separately (I already knew
the answer to my last question...)
Totally different approach this time - rather than error out, follow
the example I from the Controller Detach code which will generate an
alias for the device (although it did miss one minor check to see if
it was already set leading to a potential memory leak since the alias
code would overwrite whatever was there).
Note: In a way 1/7 was already ACK'd - I just hadn't separated it yet
for a push and wanted to keep these closer together when/if they were
pushed.
Whether 7/7 is now necessary is debateable - I keep it only for
completeness and environment setup in much the same way the start
code handles aliases.
John Ferlan (7):
virsh: Adjust the text in man page regarding qemu-attach
hotplug: Check for alias in controller detach
hotplug: Check for alias in disk detach
hotplug: Check for alias in hostdev detach
hotplug: Check for alias in chrdev detach
hotplug: Check for alias in net detach
qemu-attach: Assign device aliases
src/qemu/qemu_driver.c | 3 +++
src/qemu/qemu_hotplug.c | 28 +++++++++++++++++++++++++++-
tools/virsh.pod | 5 +++--
3 files changed, 33 insertions(+), 3 deletions(-)
6:54 p.m.
On 09.10.2014 02:08, John Ferlan wrote:
v2 is here:
http://www.redhat.com/archives/libvir-list/2014-September/msg01575.html
As Michal pointed out in his review - other devices could have the
same issue - so take care of each of them separately (I already knew
the answer to my last question...)
Totally different approach this time - rather than error out, follow
the example I from the Controller Detach code which will generate an
alias for the device (although it did miss one minor check to see if
it was already set leading to a potential memory leak since the alias
code would overwrite whatever was there).
Note: In a way 1/7 was already ACK'd - I just hadn't separated it yet
for a push and wanted to keep these closer together when/if they were
pushed.
Whether 7/7 is now necessary is debateable - I keep it only for
completeness and environment setup in much the same way the start
code handles aliases.
John Ferlan (7):
virsh: Adjust the text in man page regarding qemu-attach
hotplug: Check for alias in controller detach
hotplug: Check for alias in disk detach
hotplug: Check for alias in hostdev detach
hotplug: Check for alias in chrdev detach
hotplug: Check for alias in net detach
qemu-attach: Assign device aliases
src/qemu/qemu_driver.c | 3 +++
src/qemu/qemu_hotplug.c | 28 +++++++++++++++++++++++++++-
tools/virsh.pod | 5 +++--
3 files changed, 33 insertions(+), 3 deletions(-)
ACK
Michal
8:15 p.m.
On 10/28/2014 07:54 PM, Michal Privoznik wrote:
On 09.10.2014 02:08, John Ferlan wrote:
> v2 is here:
>
> http://www.redhat.com/archives/libvir-list/2014-September/msg01575.html
>
> As Michal pointed out in his review - other devices could have the
> same issue - so take care of each of them separately (I already knew
> the answer to my last question...)
>
> Totally different approach this time - rather than error out, follow
> the example I from the Controller Detach code which will generate an
> alias for the device (although it did miss one minor check to see if
> it was already set leading to a potential memory leak since the alias
> code would overwrite whatever was there).
>
> Note: In a way 1/7 was already ACK'd - I just hadn't separated it yet
> for a push and wanted to keep these closer together when/if they were
> pushed.
>
> Whether 7/7 is now necessary is debateable - I keep it only for
> completeness and environment setup in much the same way the start
> code handles aliases.
>
> John Ferlan (7):
> virsh: Adjust the text in man page regarding qemu-attach
> hotplug: Check for alias in controller detach
> hotplug: Check for alias in disk detach
> hotplug: Check for alias in hostdev detach
> hotplug: Check for alias in chrdev detach
> hotplug: Check for alias in net detach
> qemu-attach: Assign device aliases
>
> src/qemu/qemu_driver.c | 3 +++
> src/qemu/qemu_hotplug.c | 28 +++++++++++++++++++++++++++-
> tools/virsh.pod | 5 +++--
> 3 files changed, 33 insertions(+), 3 deletions(-)
>
ACK
Michal
Pushed - thanks
John
3680
days inactive
3700
days old
11 comments
2 participants
participants (2)
-
John Ferlan -
Michal Privoznik