On 07/04/2014 05:29 AM, Peter Krempa wrote: > When pivotting to a new disk source after a block commit (and possibly s/pivotting/pivoting/ here and in subject > after a soon-to-be-added active block commit) we changed just a few > fields to the new target. In case we'd copy a network disk to a local > file we'd not change the type properly. > > To avoid such problems, switch to tracking of the source via changing of > the complete source struct to the one tracking the mirroring info. > --- > src/qemu/qemu_driver.c | 49 +++++++++++++++++++------------------------------ > 1 file changed, 19 insertions(+), 30 deletions(-) > > @@ -14942,9 +14928,8 @@ qemuDomainBlockPivot(virConnectPtr conn, > * portion of the chain, and is made more difficult by the > * fact that we aren't tracking the full chain ourselves; so > * for now, we leak the access to the original. */ We're closer to nuking that fixme comment. :) > @@ -14954,16 +14939,17 @@ qemuDomainBlockPivot(virConnectPtr conn, > * 'query-block', to see what state we really got left in > * before killing the mirroring job? And just as on the > * success case, there's security labeling to worry about. */ > - disk->src->path = oldsrc; > - disk->src->format = oldformat; > - virStorageSourceFree(disk->src->backingStore); > - disk->src->backingStore = oldchain; > + virStorageSourceFree(disk->mirror); > } > - virStorageSourceFree(disk->mirror); > + > disk->mirror = NULL; > disk->mirroring = false; > > cleanup: > + /* revert to original disk def on failure */ > + if (oldsrc) > + disk->src = oldsrc; It took me a couple reads to track this, but I'm fairly sure that you got the aliasing correct (no double frees, no leaked memory, and the correct end results on both success and failure). ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list