Re: [PATCH 07/15] storage file: fill in src->dataFileStore during file probe

On Wed, Nov 20, 2024 at 18:48:42 +0300, Nikolai Barybin via Devel wrote: This function is never used outside of this module thus should not be exported. This strips the possibility to return '1' which you've explicitly documented above. Thinking about it a bit; the logic which is for backing store regarding the skipping of detection if libvirt can't parse it is a stop-gap measure for pre-existing configs when the blockdev code was being introduced. It meant to simply skip the backing store definition via -blockdev in case libvirt can't do that and offloaded it to qemu. For data files I don't think we need this kind of logic, and can fail instead. Apart from what I wrote above, this would be wrong here as this would also skip the backing store parsing below. Now with the change to report error if 'data-store' can't be fully processed by libvirt the location and logich here will be correct, but the above return statement will be removed. I'll be changing this to: diff --git a/src/storage_file/storage_source.c b/src/storage_file/storage_source.c index 2cfe3bc325..df73f44699 100644 --- a/src/storage_file/storage_source.c +++ b/src/storage_file/storage_source.c @@ -543,6 +543,39 @@ virStorageSourceNewFromBacking(virStorageSource *parent, } +/** + * virStorageSourceNewFromDataFile: + * @parent: storage source parent + * + * Creates a storage source which describes the data file image of @parent. + * Returned storage source format is VIR_STORAGE_FILE_RAW, and, unlike + * backing storage creation, readonly flag is copied from @parent. + */ +static virStorageSource * +virStorageSourceNewFromDataFile(virStorageSource *parent) +{ + g_autoptr(virStorageSource) dataFile = NULL; + int rc; + + if ((rc = virStorageSourceNewFromChild(parent, + parent->dataFileRaw, + &dataFile)) < 0) + return NULL; + + if (rc == 1) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("can't use data file definition '%1$s'"), + parent->dataFileRaw); + return NULL; + } + + dataFile->format = VIR_STORAGE_FILE_RAW; + dataFile->readonly = parent->readonly; + + return g_steal_pointer(&dataFile); +} + + /** * @src: disk source definition structure * @fd: file descriptor @@ -1391,6 +1424,11 @@ virStorageSourceGetMetadataRecurse(virStorageSource *src, } } + if (src->dataFileRaw) { + if (!(src->dataFileStore = virStorageSourceNewFromDataFile(src))) + return -1; + } + if (src->backingStoreRaw) { if ((rv = virStorageSourceNewFromBacking(src, &backingStore)) < 0) return -1;