On Wed, Apr 21, 2010 at 11:02:33PM +0200, Daniel Veillard wrote:
On Wed, Apr 21, 2010 at 05:56:12PM +0100, Daniel P. Berrange wrote:
The save process was relying on use of the shell >> append operator to ensure the save data was placed after the libvirt header + XML. This doesn't work for block devices though. Replace this code with use of 'dd' and its 'seek' parameter.
The qemuMonitorMigateToCommand() monitor API is used for both save/coredump, and migration via UNIX socket. We can't simply switch this to use 'dd' since this causes problems with the migration usage. Thus, create a dedicated qemuMonitorMigateToFile which can accept an filename + offset, and remove the filename from the current qemuMonitorMigateToCommand() API
* src/qemu/qemu_driver.c: Switch to qemuMonitorMigateToFile for save and core dump * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h, src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h, src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Create a new qemuMonitorMigateToFile, separate from the existing qemuMonitorMigateToCommand to allow handling file offsets --- src/qemu/qemu_driver.c | 172 +++++++++++++++++++++++------------------- src/qemu/qemu_monitor.c | 28 ++++++-- src/qemu/qemu_monitor.h | 9 ++- src/qemu/qemu_monitor_json.c | 35 ++++++++- src/qemu/qemu_monitor_json.h | 9 ++- src/qemu/qemu_monitor_text.c | 35 ++++++++- src/qemu/qemu_monitor_text.h | 9 ++- 7 files changed, 201 insertions(+), 96 deletions(-)
[...]
- if (virAsprintf(&dest, "exec:%s >>%s 2>/dev/null", argstr, safe_target) < 0) { + if (virAsprintf(&dest, "exec:%s | dd of=%s seek=%llub", + argstr, safe_target, offset) < 0) {
hum %llu will be converted to the value and then 'b' is appended. But my reading is that 'b' means block i.e. 512 bytes, and what we need is skeep to offset bytes, i.e. use seek=%lluc since 'c' means characters, or am I mistaken ?
Hmm, yes I think you are correct. Oddly QEMU was happy enough restoring from this.
virReportOOMError(); goto cleanup; }
[...]
@@ -1223,7 +1251,8 @@ int qemuMonitorTextMigrateToCommand(qemuMonitorPtr mon, goto cleanup; }
- if (virAsprintf(&dest, "exec:%s >>%s 2>/dev/null", argstr, safe_target) < 0) { + if (virAsprintf(&dest, "exec:%s | dd of=%s seek=%llub", + argstr, safe_target, offset) < 0) { virReportOOMError(); goto cleanup;
same question.
The main part of the patch is hard to read but I don't spot anything, however I wonder about the dd seek argument.
ACK once resolved one way or another :-)
Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|