On Thu, Jan 05, 2023 at 05:30:21PM +0100, Peter Krempa wrote:
We assume that FD passed images already exist so all existance
checks
are skipped.
For the case that a FD-passed image is passed without a terminated
backing chain (thus forcing us to detect) we attempt to read the header
from the FD.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 23 ++++++++++++++---------
src/storage_file/storage_source.c | 14 ++++++++++++++
2 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7dc4ef4ddb..38883a57d8 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7679,16 +7679,20 @@ qemuDomainDetermineDiskChain(virQEMUDriver *driver,
disksrc->format > VIR_STORAGE_FILE_NONE &&
disksrc->format < VIR_STORAGE_FILE_BACKING) {
+ /* terminate the chain for such images as the code below would do */
+ if (!disksrc->backingStore)
+ disksrc->backingStore = virStorageSourceNew();
+
+ /* we assume that FD-passed disks always exist */
+ if (virStorageSourceIsFD(disksrc))
+ return 0;
+
if (!virFileExists(disksrc->path)) {
virStorageSourceReportBrokenChain(errno, disksrc, disksrc);
return -1;
}
- /* terminate the chain for such images as the code below would do */
- if (!disksrc->backingStore)
- disksrc->backingStore = virStorageSourceNew();
-
/* host cdrom requires special treatment in qemu, so we need to check
* whether a block device is a cdrom */
if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM &&
@@ -7700,12 +7704,14 @@ qemuDomainDetermineDiskChain(virQEMUDriver *driver,
return 0;
}
- src = disksrc;
/* skip to the end of the chain if there is any */
- while (virStorageSourceHasBacking(src)) {
- int rv = virStorageSourceSupportsAccess(src);
+ for (src = disksrc; virStorageSourceHasBacking(src); src = src->backingStore) {
+ int rv;
+
+ if (virStorageSourceIsFD(src))
+ continue;
- if (rv < 0)
+ if ((rv = virStorageSourceSupportsAccess(src)) < 0)
return -1;
if (rv > 0) {
@@ -7720,7 +7726,6 @@ qemuDomainDetermineDiskChain(virQEMUDriver *driver,
virStorageSourceDeinit(src);
}
- src = src->backingStore;
}
/* We skipped to the end of the chain. Skip detection if there's the
diff --git a/src/storage_file/storage_source.c b/src/storage_file/storage_source.c
index ab0cdf2b12..00b28f73a6 100644
--- a/src/storage_file/storage_source.c
+++ b/src/storage_file/storage_source.c
@@ -1264,6 +1264,20 @@ virStorageSourceGetMetadataRecurseReadHeader(virStorageSource
*src,
int ret = -1;
ssize_t len;
+ if (virStorageSourceIsFD(src)) {
+ if (!src->fdtuple) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("fd passed image source not initialized"));
+ return -1;
+ }
+
+ return virFileReadHeaderFD(src->fdtuple->fds[0], VIR_STORAGE_MAX_HEADER,
+ buf);
+ }
This change makes the compiler complain with the following error:
../src/storage_file/storage_source.c: In function ‘virStorageSourceGetMetadataRecurse’:
../src/storage_file/storage_source.c:1343:9: error: ‘headerLen’ may be used uninitialized
[-Werror=maybe-uninitialized]
1343 | if (virStorageFileProbeGetMetadata(src, buf, headerLen) < 0)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/storage_file/storage_source.c:1311:12: note: ‘headerLen’ was declared here
1311 | size_t headerLen;
The issue is that at the end of this function we set `*headerLen = len;`
but it doesn't happen with FD storage source, instead we return the len
and the return value of virStorageSourceGetMetadataRecurseReadHeader()
is only used in if condition.
+
+ if (src->fdgroup)
+ return 0;
This seems like no-op as virStorageSourceIsFD() does the same.
Pavel
if (virStorageSourceInitAs(src, uid, gid) < 0)
return -1;
--
2.38.1