[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Thu, Apr 22, 2010 at 01:16:15PM +0200, Daniel Veillard wrote:
> On Thu, Apr 22, 2010 at 11:36:06AM +0100, Daniel P. Berrange wrote:
> > 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.
> 
> I assume dd will just push at the end if the offset is greater than the
> file size instead of creating a sparse file, so we end up with what we
> expect :-)

It turns out that QEMU wasn't happy at all. I forgot that when QEMU has 
problems loading a save file, it just prints a message to stderr and
carries on with life as if nothing were wrong :-(

Also 'seek' doesn't work in the way I expected. Adding a suffix 'c' does not
mean that it interprets the value as bytes. It always interprets it as the
block size (512), and uses the suffix as a multiplier :-( So there is no
way to seek an arbitrary number of bytes that isn't a multiple of the block
size. We don't want dd reading/writing in 1 byte sizes, so I'm adding padding
instead.


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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]