On Wed, Nov 20, 2024 at 18:48:42 +0300, Nikolai Barybin via Devel wrote:
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com> --- src/storage_file/storage_source.c | 39 +++++++++++++++++++++++++++++++ src/storage_file/storage_source.h | 4 ++++ 2 files changed, 43 insertions(+)
diff --git a/src/storage_file/storage_source.c b/src/storage_file/storage_source.c index 2cfe3bc325..b9d2d71aea 100644 --- a/src/storage_file/storage_source.c +++ b/src/storage_file/storage_source.c @@ -543,6 +543,33 @@ virStorageSourceNewFromBacking(virStorageSource *parent, }
+/** + * virStorageSourceNewFromDataFile: + * @parent: storage source parent + * @dataFileSrc: returned data file source definition + * + * 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. + * + * Return codes are the same as in virStorageSourceNewFromChild. + */ +int
This function is never used outside of this module thus should not be exported.
+virStorageSourceNewFromDataFile(virStorageSource *parent, + virStorageSource **dataFileSrc) +{ + if (virStorageSourceNewFromChild(parent, + parent->dataFileRaw, + dataFileSrc) < 0) + return -1;
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.
+ + (*dataFileSrc)->format = VIR_STORAGE_FILE_RAW; + (*dataFileSrc)->readonly = parent->readonly; + + return 0; +} + + /** * @src: disk source definition structure * @fd: file descriptor @@ -1391,6 +1418,18 @@ virStorageSourceGetMetadataRecurse(virStorageSource *src, } }
+ if (src->dataFileRaw) { + g_autoptr(virStorageSource) dataFileStore = NULL; + if ((rv = virStorageSourceNewFromDataFile(src, &dataFileStore)) < 0) + return -1; + + /* the data file would not be usable for VM usage */ + if (rv == 1) + return 0;
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;