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@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