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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list