Re: [libvirt] [PATCH 3/4] Fix QEMU save/restore with block devices

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 ?

> 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 :-)