9:14 a.m.
From: Wim ten Have <wim.ten.have(a)oracle.com>
When tools like virt-install request to create a fully allocated
filesystem object storage by setting the parameter sparse=no, libvirt
doesn't allow that to happen for qcow2 formatted files.
Regardless of its XML instuction request libvirt always targets its
filesystem object storage with preallocation=metadata if format=qcow2
is in effect. This results in sparse files which could cause problems
since total image storage potentially can overrun actual filesystem
available space.
(NOTE: This is a RESEND holding corrected cover letter and commit
messages / Subject headers.)
Wim ten Have (2):
storage: extend preallocation flags support for qemu-img
tests: add qemu-img test for preallocation=falloc
include/libvirt/libvirt-storage.h | 5 ++++-
src/storage/storage_util.c | 21 +++++++++++++++++----
.../qcow2-nocapacity-convert-prealloc.argv | 2 +-
3 files changed, 22 insertions(+), 6 deletions(-)
--
2.14.3
9:14 a.m.
New subject: [libvirt] [PATCH v1 1/2] storage: extend preallocation flags support for qemu-img
From: Wim ten Have <wim.ten.have(a)oracle.com>
This patch adds support to qcow2 formatted filesystem object storage by
instructing qemu-img to build them with preallocation=falloc whenever the
XML described storage <allocation> matches its <capacity>. For all other
cases the filesystem stored objects are built with preallocation=metadata.
Signed-off-by: Wim ten Have <wim.ten.have(a)oracle.com>
---
include/libvirt/libvirt-storage.h | 5 ++++-
src/storage/storage_util.c | 21 +++++++++++++++++----
2 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h
index 413d9f6c4..2f22b388c 100644
--- a/include/libvirt/libvirt-storage.h
+++ b/include/libvirt/libvirt-storage.h
@@ -337,8 +337,11 @@ const char* virStorageVolGetName
(virStorageVolPtr vol);
const char* virStorageVolGetKey (virStorageVolPtr vol);
typedef enum {
+ VIR_STORAGE_VOL_CREATE_PREALLOC_NONE = 0 << 0,
VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA = 1 << 0,
- VIR_STORAGE_VOL_CREATE_REFLINK = 1 << 1, /* perform a btrfs lightweight copy
*/
+ VIR_STORAGE_VOL_CREATE_PREALLOC_FALLOC = 1 << 1,
+ VIR_STORAGE_VOL_CREATE_PREALLOC_FULL = 1 << 2,
+ VIR_STORAGE_VOL_CREATE_REFLINK = 1 << 3, /* perform a btrfs
lightweight copy */
} virStorageVolCreateFlags;
virStorageVolPtr virStorageVolCreateXML (virStoragePoolPtr pool,
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index b4aed0f70..7728fb63e 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -852,7 +852,7 @@ struct _virStorageBackendQemuImgInfo {
const char *path;
unsigned long long size_arg;
bool encryption;
- bool preallocate;
+ unsigned int preallocate;
const char *compat;
virBitmapPtr features;
bool nocow;
@@ -884,8 +884,15 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc,
virStorageFileFormatTypeToString(info.backingFormat));
if (info.encryption)
virBufferAddLit(&buf, "encryption=on,");
- if (info.preallocate)
+
+ /* Handle various types of file-system storage pre-allocate sets.
+ */
+ if (info.preallocate & VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA)
virBufferAddLit(&buf, "preallocation=metadata,");
+ else if (info.preallocate & VIR_STORAGE_VOL_CREATE_PREALLOC_FALLOC)
+ virBufferAddLit(&buf, "preallocation=falloc,");
+ else if (info.preallocate & VIR_STORAGE_VOL_CREATE_PREALLOC_FULL)
+ virBufferAddLit(&buf, "preallocation=full,");
}
if (info.nocow)
@@ -1183,7 +1190,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
.format = vol->target.format,
.path = vol->target.path,
.encryption = vol->target.encryption != NULL,
- .preallocate = !!(flags & VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA),
+ .preallocate = VIR_STORAGE_VOL_CREATE_PREALLOC_NONE,
.compat = vol->target.compat,
.features = vol->target.features,
.nocow = vol->target.nocow,
@@ -1192,7 +1199,13 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr
pool,
};
virStorageEncryptionInfoDefPtr enc = NULL;
- virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL);
+ if (flags) {
+ info.preallocate = (vol->target.capacity == vol->target.allocation) ?
+ VIR_STORAGE_VOL_CREATE_PREALLOC_FALLOC :
VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
+ virCheckFlags((VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA|
+ VIR_STORAGE_VOL_CREATE_PREALLOC_FALLOC|
+ VIR_STORAGE_VOL_CREATE_PREALLOC_FULL), NULL);
+ }
/* Treat output block devices as 'raw' format */
if (vol->type == VIR_STORAGE_VOL_BLOCK)
--
2.14.3
6:31 a.m.
New subject: [libvirt] [PATCH v1 1/2] storage: extend preallocation flags support for qemu-img
On 04/03/2018 04:14 PM, Wim Ten Have wrote:
From: Wim ten Have <wim.ten.have(a)oracle.com>
This patch adds support to qcow2 formatted filesystem object storage by
instructing qemu-img to build them with preallocation=falloc whenever the
XML described storage <allocation> matches its <capacity>. For all other
cases the filesystem stored objects are built with preallocation=metadata.
Signed-off-by: Wim ten Have <wim.ten.have(a)oracle.com>
---
include/libvirt/libvirt-storage.h | 5 ++++-
src/storage/storage_util.c | 21 +++++++++++++++++----
2 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h
index 413d9f6c4..2f22b388c 100644
--- a/include/libvirt/libvirt-storage.h
+++ b/include/libvirt/libvirt-storage.h
@@ -337,8 +337,11 @@ const char* virStorageVolGetName
(virStorageVolPtr vol);
const char* virStorageVolGetKey (virStorageVolPtr vol);
typedef enum {
+ VIR_STORAGE_VOL_CREATE_PREALLOC_NONE = 0 << 0,
This is okay.
VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA = 1 << 0,
- VIR_STORAGE_VOL_CREATE_REFLINK = 1 << 1, /* perform a btrfs lightweight copy
*/
+ VIR_STORAGE_VOL_CREATE_PREALLOC_FALLOC = 1 << 1,
+ VIR_STORAGE_VOL_CREATE_PREALLOC_FULL = 1 << 2,
+ VIR_STORAGE_VOL_CREATE_REFLINK = 1 << 3, /* perform a btrfs
lightweight copy */
This is not. Imagine there's a mgmt application already written and
compiled which calls:
virStorageVolCreateXML(flags = VIR_STORAGE_VOL_CREATE_REFLINK);
Because it is already compiled it is effectively calling:
virStorageVolCreateXML(flags = 1);
and everything works. However, if this change would be merged, the mgmt
application would be still making the same call but now it would have
different semantic, because you are changing the numbering. So
effectively mgmt app would be calling
virStorageVolCreateXML(flags = VIR_STORAGE_VOL_CREATE_PREALLOC_FALLOC)
which is obviously wrong. We can not expect mgmt applications to be
recompiled every time there's a new release of libvirt.
} virStorageVolCreateFlags;
virStorageVolPtr virStorageVolCreateXML (virStoragePoolPtr pool,
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index b4aed0f70..7728fb63e 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -852,7 +852,7 @@ struct _virStorageBackendQemuImgInfo {
const char *path;
unsigned long long size_arg;
bool encryption;
- bool preallocate;
+ unsigned int preallocate;
const char *compat;
virBitmapPtr features;
bool nocow;
@@ -884,8 +884,15 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc,
virStorageFileFormatTypeToString(info.backingFormat));
if (info.encryption)
virBufferAddLit(&buf, "encryption=on,");
- if (info.preallocate)
+
+ /* Handle various types of file-system storage pre-allocate sets.
+ */
+ if (info.preallocate & VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA)
virBufferAddLit(&buf, "preallocation=metadata,");
+ else if (info.preallocate & VIR_STORAGE_VOL_CREATE_PREALLOC_FALLOC)
+ virBufferAddLit(&buf, "preallocation=falloc,");
+ else if (info.preallocate & VIR_STORAGE_VOL_CREATE_PREALLOC_FULL)
+ virBufferAddLit(&buf, "preallocation=full,");
}
if (info.nocow)
@@ -1183,7 +1190,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr
pool,
.format = vol->target.format,
.path = vol->target.path,
.encryption = vol->target.encryption != NULL,
- .preallocate = !!(flags & VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA),
+ .preallocate = VIR_STORAGE_VOL_CREATE_PREALLOC_NONE,
.compat = vol->target.compat,
.features = vol->target.features,
.nocow = vol->target.nocow,
@@ -1192,7 +1199,13 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr
pool,
};
virStorageEncryptionInfoDefPtr enc = NULL;
- virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL);
+ if (flags) {
+ info.preallocate = (vol->target.capacity == vol->target.allocation) ?
+ VIR_STORAGE_VOL_CREATE_PREALLOC_FALLOC :
VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
+ virCheckFlags((VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA|
+ VIR_STORAGE_VOL_CREATE_PREALLOC_FALLOC|
+ VIR_STORAGE_VOL_CREATE_PREALLOC_FULL), NULL);
+ }
virCheckFlags() should be checked outside of this if() statement.
But hold on. How can PREALLOC_FULL be used? How can any flag be
specified in the first place? I mean
virStorageVolCreateXML(flags = VIR_STORAGE_VOL_CREATE_PREALLOC_FULL)
boils down to
virStorageBackendCreateQemuImgCmdFromVol(flags =
VIR_STORAGE_VOL_CREATE_PREALLOC_FULL)
which completely ignores the flag. This is wrong.
Also, storagevolxml2argvtest is failing after this change. See 'make check'.
Michal
1:14 p.m.
New subject: [libvirt] [PATCH v1 1/2] storage: extend preallocation flags support for qemu-img
On Wed, 4 Apr 2018 13:31:39 +0200
Michal Privoznik <mprivozn(a)redhat.com> wrote:
On 04/03/2018 04:14 PM, Wim Ten Have wrote:
> From: Wim ten Have <wim.ten.have(a)oracle.com>
>
> This patch adds support to qcow2 formatted filesystem object storage by
> instructing qemu-img to build them with preallocation=falloc whenever the
> XML described storage <allocation> matches its <capacity>. For all
other
> cases the filesystem stored objects are built with preallocation=metadata.
>
> Signed-off-by: Wim ten Have <wim.ten.have(a)oracle.com>
...
> VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA = 1 << 0,
> - VIR_STORAGE_VOL_CREATE_REFLINK = 1 << 1, /* perform a btrfs lightweight
copy */
> + VIR_STORAGE_VOL_CREATE_PREALLOC_FALLOC = 1 << 1,
> + VIR_STORAGE_VOL_CREATE_PREALLOC_FULL = 1 << 2,
> + VIR_STORAGE_VOL_CREATE_REFLINK = 1 << 3, /* perform a btrfs
lightweight copy */
This is not. Imagine there's a mgmt application already written and
compiled which calls:
virStorageVolCreateXML(flags = VIR_STORAGE_VOL_CREATE_REFLINK);
Because it is already compiled it is effectively calling:
virStorageVolCreateXML(flags = 1);
and everything works. However, if this change would be merged, the mgmt
application would be still making the same call but now it would have
different semantic, because you are changing the numbering. So
effectively mgmt app would be calling
virStorageVolCreateXML(flags = VIR_STORAGE_VOL_CREATE_PREALLOC_FALLOC)
which is obviously wrong. We can not expect mgmt applications to be
recompiled every time there's a new release of libvirt.
...
right, let me send v2. (applying and propagating XML target.sparse)
Also, storagevolxml2argvtest is failing after this change. See
'make check'.
?! ... sure I ran 'make check'. Anyways ... corrections under v2 coming forth.
Rgds,
- Wim.
Michal
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
9:14 a.m.
New subject: [libvirt] [PATCH v1 2/2] tests: add qemu-img test for preallocation=falloc
From: Wim ten Have <wim.ten.have(a)oracle.com>
Signed-off-by: Wim ten Have <wim.ten.have(a)oracle.com>
---
tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
b/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
index 9073b1b16..b151b9401 100644
--- a/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
+++ b/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
@@ -1,4 +1,4 @@
qemu-img convert -f raw -O qcow2 \
--o encryption=on,preallocation=metadata \
+-o encryption=on,preallocation=falloc \
/var/lib/libvirt/images/sparse.img \
/var/lib/libvirt/images/OtherDemo.img
--
2.14.3
6:31 a.m.
New subject: [libvirt] [PATCH v1 2/2] tests: add qemu-img test for preallocation=falloc
On 04/03/2018 04:14 PM, Wim Ten Have wrote:
From: Wim ten Have <wim.ten.have(a)oracle.com>
Signed-off-by: Wim ten Have <wim.ten.have(a)oracle.com>
---
tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
b/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
index 9073b1b16..b151b9401 100644
--- a/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
+++ b/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
@@ -1,4 +1,4 @@
qemu-img convert -f raw -O qcow2 \
--o encryption=on,preallocation=metadata \
+-o encryption=on,preallocation=falloc \
/var/lib/libvirt/images/sparse.img \
/var/lib/libvirt/images/OtherDemo.img
Ah, if anything, this should be merged into the previous patch.
Michal
2422
days inactive
2428
days old
5 comments
3 participants
participants (3)
-
Michal Privoznik -
Wim Ten Have -
Wim ten Have