[libvirt] [PATCH 1/2] Storage: Suppress metadata refresh for volumes being built.

[Ján Tomko]

On Tue, May 05, 2015 at 03:24:31PM +0530, Prerna Saxena wrote:
> 
> On Tuesday 05 May 2015 03:20 PM, Prerna Saxena wrote:
> > On Tuesday 05 May 2015 01:52 PM, Ján Tomko wrote:
> >> On Tue, May 05, 2015 at 08:43:21AM +0530, Prerna Saxena wrote:
> >>> Libvirt periodically calls 'stat' on all volumes in a storage pool,
> >>> to update fields such as 'target.allocation'.
> >>>
> >>> The operation doesnt make sense for a volume which is curently being allocated.
> >> From the comments in the storage driver, the point of allowing refresh
> >> for a volume that is currently being allocated is to track the progress
> >> of the allocation.
> >>
> >>> Also, the 'target.allocation' sub-field is taken into account while copying a raw image.
> >>> To suppress any (potential) corruption, libvirt must not attempt to refresh a volume currently being built.
> >> What would be the corruption?
> >>
> >> We do not allow using a volume that is currently building as a
> >> source for cloning the volume - storageVolCreateXMLFrom checks for
> >> origvol->building:
> >>
> >>     if (origvol->building) {
> >>         virReportError(VIR_ERR_OPERATION_INVALID,
> >>                        _("volume '%s' is still being allocated."),
> >>                        origvol->name);
> >>         goto cleanup;
> >>     }
> >>
> > While running libvirt on PowerPC, I saw an interesting scenario. The 'target.allocation' field seemed to change for a volume getting allocated, and this would lead to incomplete copy. This would
> > happen at random intervals, not deterministically. While looking through the code, I found this to be the other place in code where the same field seemed to change without a lock. Hence the patch.
> >

Oh, I was thinking of the soure volume for some reason.

We correctly lock the pool before calling refreshVol, so changing the
object should not be an issue.
I think the bug is in storageVolCreateXMLFrom - it drops all the locks,
but expects the allocation not to change.

In storageVolCreateXML we work around this by creating a shallow copy of
the volume.

> > I have sent the second patch which fixes the erring code too :
> >
> > -    remain = vol->target.allocation;
> > +    remain = inputvol->target.capacity;
> >
> More fundamental question -- why do we offload the copying of non-raw images to qemu-img tool, but make libvirt responsible for copying raw volumes ?
> Would it not be better if libvirt called on 'qemu-img' to copy all types of volumes, including raw ones ?
> 

This way, libvirt can create raw volumes even without qemu-img
installed. I don't know if there's any other reason.

Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150505/4e625bf6/attachment-0001.sig>