[PATCH 0/2] storage_file: Refuse qcow2 images with empty string as 'data_file'
See patch 1/2 for justification. Patch 2 is a test case. Peter Krempa (2): storage_file: Refuse qcow2 images with empty string as 'data_file' virstoragetest: Add case for qcow2 image with empty string as 'data_file' src/storage_file/storage_source.c | 10 ++++++++++ tests/virstoragetest.c | 5 +++++ .../images/datafile-emptystr.qcow2 | Bin 0 -> 327680 bytes 3 files changed, 15 insertions(+) create mode 100644 tests/virstoragetestdata/images/datafile-emptystr.qcow2 -- 2.47.1
In certain buggy conditions qemu can create an image which has empty string stored as 'data_file'. While probing libvirt would consider the empty string as a relative file name and construct the path using the path of the parent image stripping the last component and appending the empty string. This results into attempting to using a directory as an image and thus the following error when attempting to start VM with such an image: error: unsupported configuration: storage type 'dir' requires use of storage format 'fat' Reject empty strings passed in as 'data_file'. Note that we do not have the same problem with 'backing store' as an empty string there is interpreted as no backing file both by qemu and libvirt. Resolves: https://issues.redhat.com/browse/RHEL-70627 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/storage_file/storage_source.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/storage_file/storage_source.c b/src/storage_file/storage_source.c index 4612e710b0..fa59949cf2 100644 --- a/src/storage_file/storage_source.c +++ b/src/storage_file/storage_source.c @@ -557,6 +557,16 @@ virStorageSourceNewFromDataFile(virStorageSource *parent) g_autoptr(virStorageSource) dataFile = NULL; int rc; + /* 'qemu-img' records an empty string as 'data_file' field in certain buggy + * cases. Note that it can't happen for 'backing store' as absence of the + * string equals to no backing store. */ + if (STREQ(parent->dataFileRaw, "")) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("invalid empty data-file definition in '%1$s'"), + NULLSTR(parent->path)); + return NULL; + } + if ((rc = virStorageSourceNewFromChild(parent, parent->dataFileRaw, &dataFile)) < 0) -- 2.47.1
Add an example image formatted by: qemu-img create -f qcow2 -o data_file=nbd+unix:///datafile?socket=/tmp/nbd,data_file_raw=true /tmp/nbddatastore.qcow2 10M -u serving as an example when qemu records an empty string as the 'data_file' field. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virstoragetest.c | 5 +++++ .../images/datafile-emptystr.qcow2 | Bin 0 -> 327680 bytes 2 files changed, 5 insertions(+) create mode 100644 tests/virstoragetestdata/images/datafile-emptystr.qcow2 diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 4ec837eefb..78dc644637 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -494,6 +494,11 @@ mymain(void) abs_srcdir "/virstoragetestdata/images/qcow2datafile-datafile.qcow2", VIR_STORAGE_FILE_QCOW2, EXP_PASS); + /* broken qcow2 with a 'data_file' which is an empty string */ + TEST_CHAIN("qcow2-datafile-broken", + abs_srcdir "/virstoragetestdata/images/datafile-emptystr.qcow2", + VIR_STORAGE_FILE_QCOW2, EXP_FAIL); + /* Test various combinations of qcow2 images with missing 'backing_format' */ TEST_CHAIN("qcow2-qcow2_qcow2-qcow2_qcow2-auto", abs_srcdir "/virstoragetestdata/images/qcow2_qcow2-qcow2_qcow2-auto.qcow2", diff --git a/tests/virstoragetestdata/images/datafile-emptystr.qcow2 b/tests/virstoragetestdata/images/datafile-emptystr.qcow2 new file mode 100644 index 0000000000000000000000000000000000000000..18fae8740b4d650252d9a20629ca6e4d04653692 GIT binary patch literal 327680 zcmeIuK}y3w6b9f)+5>omoWUCuL=fBvg6p(NG0>(VouYNsV+(F1sS&bCUEA*q4D;Un znPKMX{^2WxFbiWh=Q16-Nk17<=juF;&BtZGLDy@B{@vnv(am3HpD!UKrx1#&jmK<R z#UU~+S?${P(8Ry~k(TeVZ0mfJ6?x3Fb+suc516fX+omn|`)XHb@z_kKLzs8_*F{-m zkGEM_$F?f>69*(=lYbnuwp_1vhdNG0dudo!aho?6gFn)w&EK-oorfg=0000000000 z00000000000000000000000000000000000000000000000000000000000000000 z0D#NV%Mkzo0000000000000000000000000000000000000000000000000000000 z000000000000000000000RFk;_X7X`00000000000000000000000000000000000 z0000000000000000000000000000000000000000;EJ8PyU(xq0RR910000000000 z00000000000000000000000000000000000000000000000000000000000000000 z=$*U+000000000000000000000000000000000000000000000000000000000000 V0000000000000000000Xgm1%)GK2sC literal 0 HcmV?d00001 -- 2.47.1
On 1/9/25 15:28, Peter Krempa wrote:
See patch 1/2 for justification. Patch 2 is a test case.
Peter Krempa (2): storage_file: Refuse qcow2 images with empty string as 'data_file' virstoragetest: Add case for qcow2 image with empty string as 'data_file'
src/storage_file/storage_source.c | 10 ++++++++++ tests/virstoragetest.c | 5 +++++ .../images/datafile-emptystr.qcow2 | Bin 0 -> 327680 bytes 3 files changed, 15 insertions(+) create mode 100644 tests/virstoragetestdata/images/datafile-emptystr.qcow2
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Prívozník -
Peter Krempa