9:59 a.m.
According to qemu-kvm/qerror.c all messages start with a capital
"Device ", but the current code only scans for the lower case "device
".
This results in "virDomainUpdateDeviceFlags()" to not detect locked
CD-ROMs and reporting success even in the case of a failure:
# virsh qemu-monitor-command "$VM" change\ drive-ide0-0-0\
\"/var/lib/libvirt/images/ucs_2.4-0-sec4-20110714145916-dvd-amd64.iso\"
Device 'drive-ide0-0-0' is locked
# virsh update-device "$VM" /dev/stdin <<<"<disk
type='file' device='cdrom'><driver name='qemu'
type='raw'/><source
file='/var/lib/libvirt/images/ucs_2.4-0-sec4-20110714145916-dvd-amd64.iso'/><target
dev='hda' bus='ide'/><readonly/><alias
name='ide0-0-0'/><address type='drive' controller='0'
bus='0' unit='0'/></disk>"
Device updated successfully
Signed-off-by: Philipp Hahn <hahn(a)univention.de>
---
src/qemu/qemu_monitor_text.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 52d924a..98d9169 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -1064,7 +1064,7 @@ int qemuMonitorTextChangeMedia(qemuMonitorPtr mon,
/* If the command failed qemu prints:
* device not found, device is locked ...
* No message is printed on success it seems */
- if (strstr(reply, "device ")) {
+ if (c_strcasestr(reply, "device ")) {
qemuReportError(VIR_ERR_OPERATION_FAILED,
_("could not change media on %s: %s"), devname,
reply);
goto cleanup;
--
1.7.1
9:59 a.m.
New subject: [libvirt] [PATCH 2/2] Reload snapshots on SIGHUP
On receiving SIGHUP, libvirtd currently only reloads all persistent
configs for qemu domains, but fails to reload the associated snapshot
data.
Copy code from qemudStartup() to qemudReload() to iterate over all
domains and call qemuDomainSnapshotLoad() for every domain.
Signed-off-by: Philipp Hahn <hahn(a)univention.de>
---
This depends on commits 9e093f0b4cc5a5fc455a4893d73dc0f2c5355161 to fix
SIGHUP and on 839a5295ef568727b025f9546c1720279f49fe62 to fix a memory
leak.
---
src/qemu/qemu_driver.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c8dda73..fcb56ac 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -702,6 +702,9 @@ qemudReload(void) {
qemu_driver->autostartDir,
0, QEMU_EXPECTED_VIRT_TYPES,
qemudNotifyLoadDomain, qemu_driver);
+
+ virHashForEach(qemu_driver->domains.objs, qemuDomainSnapshotLoad,
+ qemu_driver->snapshotDir);
qemuDriverUnlock(qemu_driver);
qemuAutostartDomains(qemu_driver);
--
1.7.1
12:02 p.m.
New subject: [libvirt] [PATCH 2/2] Reload snapshots on SIGHUP
On 08/30/2011 08:59 AM, Philipp Hahn wrote:
On receiving SIGHUP, libvirtd currently only reloads all persistent
configs for qemu domains, but fails to reload the associated snapshot
data.
Copy code from qemudStartup() to qemudReload() to iterate over all
domains and call qemuDomainSnapshotLoad() for every domain.
Signed-off-by: Philipp Hahn<hahn(a)univention.de>
---
Are you sure this is still relevant? In my testing, after this was
applied, I got lots of errors:
10:58:09.987: 15794: error : virDomainSnapshotAssignDef:11113 : internal
error unexpected domain snapshot 1308171632 already exists
and without the patch, it looked to me like the snapshots were still all
present in memory across SIGHUP without reloading them from disk. I
have to wonder if commit 6766ff10d made the difference in how things are
behaving? Perhaps your patch is still needed, but if so, can you give
me the test scenario you used that shows the difference in behavior pre-
and post-patch?
Were you hand-modifying the contents of
/var/lib/libvirt/qemu/snapshot/dom/*.xml behind libvirt's back and
expecting the SIGHUP to have libvirt incorporate the new xml contents
from the disk? Are you missing a step that nukes all existing snapshot
data in memory before re-reading snapshot data from the disk?
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
12:18 a.m.
New subject: [libvirt] [PATCH 2/2] Reload snapshots on SIGHUP
Hello Eric,
Am Dienstag 30 August 2011 19:02:50 schrieb Eric Blake:
On 08/30/2011 08:59 AM, Philipp Hahn wrote:
> On receiving SIGHUP, libvirtd currently only reloads all persistent
> configs for qemu domains, but fails to reload the associated snapshot
> data.
...
Are you sure this is still relevant? In my testing, after this was
applied, I got lots of errors:
10:58:09.987: 15794: error : virDomainSnapshotAssignDef:11113 : internal
error unexpected domain snapshot 1308171632 already exists
and without the patch, it looked to me like the snapshots were still all
present in memory across SIGHUP without reloading them from disk. I
have to wonder if commit 6766ff10d made the difference in how things are
behaving? Perhaps your patch is still needed, but if so, can you give
me the test scenario you used that shows the difference in behavior pre-
and post-patch?
I posted that patch more for consistency, since sending a SIGHUP to libvirtd
re-scans for changes in the domain XMl files under /etc/libvirt/qemu/, but
would miss new or updates snapshots under /var/lib/libvirt/qemu/snapshot/.
Were you hand-modifying the contents of
/var/lib/libvirt/qemu/snapshot/dom/*.xml behind libvirt's back and
expecting the SIGHUP to have libvirt incorporate the new xml contents
from the disk?
Yes, see my other post on snapshots-on-a-shared-storage: I had to get that
working with 0.8.7 and just forward-ported my patch to HEAD.
Are you missing a step that nukes all existing snapshot
data in memory before re-reading snapshot data from the disk?
Might be, will re-check.
Sincerely
Philipp
--
Philipp Hahn Open Source Software Engineer hahn(a)univention.de
Univention GmbH Linux for Your Business fon: +49 421 22 232- 0
Mary-Somerville-Str.1 D-28359 Bremen fax: +49 421 22 232-99
http://www.univention.de/
----------------------------------------------------------------------------
Treffen Sie Univention auf der IT&Business vom 20. bis 22. September 2011
auf dem Gemeinschaftsstand der Open Source Business Alliance in Stuttgart in
Halle 3 Stand 3D27-7.
11:33 a.m.
On 08/30/2011 08:59 AM, Philipp Hahn wrote:
According to qemu-kvm/qerror.c all messages start with a capital
"Device ", but the current code only scans for the lower case "device
".
This results in "virDomainUpdateDeviceFlags()" to not detect locked
CD-ROMs and reporting success even in the case of a failure:
# virsh qemu-monitor-command "$VM" change\ drive-ide0-0-0\
\"/var/lib/libvirt/images/ucs_2.4-0-sec4-20110714145916-dvd-amd64.iso\"
Device 'drive-ide0-0-0' is locked
# virsh update-device "$VM" /dev/stdin<<<"<disk
type='file' device='cdrom'><driver name='qemu'
type='raw'/><source
file='/var/lib/libvirt/images/ucs_2.4-0-sec4-20110714145916-dvd-amd64.iso'/><target
dev='hda' bus='ide'/><readonly/><alias
name='ide0-0-0'/><address type='drive' controller='0'
bus='0' unit='0'/></disk>"
Device updated successfully
Signed-off-by: Philipp Hahn<hahn(a)univention.de>
---
src/qemu/qemu_monitor_text.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 52d924a..98d9169 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -1064,7 +1064,7 @@ int qemuMonitorTextChangeMedia(qemuMonitorPtr mon,
/* If the command failed qemu prints:
* device not found, device is locked ...
* No message is printed on success it seems */
- if (strstr(reply, "device ")) {
+ if (c_strcasestr(reply, "device ")) {
ACK and pushed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
4859
days inactive
4863
days old
4 comments
2 participants
participants (2)
-
Eric Blake -
Philipp Hahn