Re: [libvirt] [PATCH 1/2] Storage: Suppress metadata refresh for volumes being built.
On 05/05/2015 10:44 PM, Prerna Saxena wrote:
On Tuesday 05 May 2015 04:22 PM, Ján Tomko wrote:
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.
Yes, and so I sent this patch that blocks a refresh for volumes being created.
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.
I'm sorry, libvirt does not copy raw volumes as a 'fallback mechanism'.
Libvirt chooses to copy raw volumes on its own, but calls on qemu-img to copy
all other volume types.
Is it not better to let qemu-img copy all volume types , including raw ones ?
I wanted to check if there are reasons for this decision ? I'll be happy if
the community could throw some light.
Also cc'ing Cole, who had put in some fixes in this area.
My recollection is that like Jan says we implement raw volume creation
ourselves so that we could work without qemu-img in that case, like if
qemu-img isn't installed. And then raw cloning was kind of built upon the raw
creation code.
But IMO we should drop all that stuff and just use qemu-img unconditionally,
it's ubiquitous at this point.
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Storage: Suppress metadata refresh for volumes being built.
On Tuesday 05 May 2015 04:22 PM, Ján Tomko wrote:
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.
Yes, and so I sent this patch that blocks a refresh for volumes being created.
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.
I'm sorry, libvirt does not copy raw volumes as a 'fallback mechanism'.
Libvirt chooses to copy raw volumes on its own, but calls on qemu-img to copy
all other volume types.
Is it not better to let qemu-img copy all volume types , including raw ones ?
I wanted to check if there are reasons for this decision ? I'll be happy if the
community could throw some light.
Also cc'ing Cole, who had put in some fixes in this area.
Regards,
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Storage: Suppress metadata refresh for volumes being built.
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;
}
Jan
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Storage: Suppress metadata refresh for volumes being built.
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.
I have sent the second patch which fixes the erring code too :
-remain = vol-target.allocation;
+remain = inputvol-target.capacity;
Regards,
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Storage: Suppress metadata refresh for volumes being built.
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.
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 ?
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Storage: Suppress metadata refresh for volumes being built.
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
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Storage: Suppress metadata refresh for volumes being built.
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.
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.
Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
---
src/storage/storage_backend.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 289f454..355fc7f 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1576,6 +1576,9 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
{
int ret;
+if (vol-building)
+return 0;
+
if ((ret = virStorageBackendUpdateVolTargetInfo(vol-target,
withBlockVolFormat,
openflags)) 0)
--
1.8.3.1
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
