Re: [PATCH v3] qemu: read backing chain names from qemu

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

 



On Mon, Mar 16, 2015 at 12:17:58 -0600, Eric Blake wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1199182 documents that
> after a series of disk snapshots into existing destination images,
> followed by active commits of the top image, it is possible for
> qemu 2.2 and earlier to end up tracking a different name for the
> image than what it would have had when opening the chain afresh.
> That is, when starting with the chain 'a <- b <- c', the name
> associated with 'b' is how it was spelled in the metadata of 'c',
> but when starting with 'a', taking two snapshots into 'a <- b <- c',
> then committing 'c' back into 'b', the name associated with 'b' is
> now the name used when taking the first snapshot.
> 
> Sadly, older qemu doesn't know how to treat different spellings of
> the same filename as identical files (it uses strcmp() instead of
> checking for the same inode), which means libvirt's attempt to
> commit an image using solely the names learned from qcow2 metadata
> fails with a cryptic:
> 
> error: internal error: unable to execute QEMU command 'block-commit': Top image file /tmp/images/c/../b/b not found
> 
> even though the file exists.  Trying to teach libvirt the rules on
> which name qemu will expect is not worth the effort (besides, we'd
> have to remember it across libvirtd restarts, and track whether a
> file was opened via metadata or via snapshot creation for a given
> qemu process); it is easier to just always directly ask qemu what
> string it expects to see in the first place.
> 
> As a safety valve, we validate that any name returned by qemu
> still maps to the same local file as we have tracked it, so that
> a compromised qemu cannot accidentally cause us to act on an
> incorrect file.
> 
> * src/qemu/qemu_monitor.h (qemuMonitorDiskNameLookup): New
> prototype.
> * src/qemu/qemu_monitor_json.h (qemuMonitorJSONDiskNameLookup):
> Likewise.
> * src/qemu/qemu_monitor.c (qemuMonitorDiskNameLookup): New function.
> * src/qemu/qemu_monitor_json.c (qemuMonitorJSONDiskNameLookup)
> (qemuMonitorJSONDiskNameLookupOne): Likewise.
> * src/qemu/qemu_driver.c (qemuDomainBlockCommit)
> (qemuDomainBlockJobImpl): Use it.
> 
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
> 
> v3: better sanity checks in qemu_monitor_json, rebase
> and retested atop Peter's fixes for interlocking block jobs
> 
>  src/qemu/qemu_driver.c       |  28 ++++++------
>  src/qemu/qemu_monitor.c      |  20 ++++++++-
>  src/qemu/qemu_monitor.h      |   8 +++-
>  src/qemu/qemu_monitor_json.c | 102 ++++++++++++++++++++++++++++++++++++++++++-
>  src/qemu/qemu_monitor_json.h |   9 +++-
>  5 files changed, 149 insertions(+), 18 deletions(-)

ACK,

Peter

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]