I am trying to use a qcow image with libvirt where the backing 'file' is a qemu-nbd server. Unfortunately virDomainDiskDefForeachPath() assumes that backingStore is always a real file so something like 'nbd:0:3333' is rejected because a file with that name cannot be accessed. Note that I am not worried about directly using nbd images. That would require a new disk type with XML markup, etc. I only want it to be permitted as a backingStore The following patch implements danpb's suggestion:

I think I'm inclined to push the logic for skipping NBD one stage higher. I'd rather expect virStorageFileGetMetadata() to return all backing stores, even if not files. The virDomainDiskDefForeachPath() method should definitely ignore non-file backing stores though.

So what I'm thinking is to extend the virStorageFileMetadata struct and just add a 'bool isFile' field to it. Default this field to true, unless you see the prefix of nbd: in which case set it to false. The virDomainDiskDefForeachPath() method can then skip over any backing store with isFile == false

Signed-off-by: Adam Litke <agl@us.ibm.com> Cc: Daniel P. Berrange <berrange@redhat.com> --- src/conf/domain_conf.c | 7 +++++++ src/util/storage_file.c | 16 +++++++++++++++- src/util/storage_file.h | 1 + 3 files changed, 23 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fe93711..2d11785 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7813,6 +7813,13 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, depth++; nextpath = meta.backingStore; + /* Stop iterating if we reach a non-file backing store */ + if (nextpath && meta.backingStoreIsFile == false) { + VIR_DEBUG("Stopping iteration on non-file backing store: %s", + nextpath); + break; + } + format = meta.backingStoreFormat; if (format == VIR_STORAGE_FILE_AUTO && diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 6f48b10..6e099ed 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -509,6 +509,14 @@ virStorageFileMatchesVersion(int format, return true; } +static bool +virBackingStoreIsFile(const char *backing) +{ + /* Backing store is a network block device */ + if (STRPREFIX(backing, "nbd:")) + return false; + return true; +} static int virStorageFileGetMetadataFromBuf(int format, @@ -579,8 +587,14 @@ virStorageFileGetMetadataFromBuf(int format, if (ret == BACKING_STORE_ERROR) return -1; + meta->backingStoreIsFile = false; if (backing != NULL) { - meta->backingStore = absolutePathFromBaseFile(path, backing); + if (virBackingStoreIsFile(backing)) { + meta->backingStoreIsFile = true; + meta->backingStore = absolutePathFromBaseFile(path, backing); + } else { + meta->backingStore = strdup(backing); + } VIR_FREE(backing); if (meta->backingStore == NULL) { virReportOOMError(); diff --git a/src/util/storage_file.h b/src/util/storage_file.h index fb0c005..c4d4650 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -49,6 +49,7 @@ VIR_ENUM_DECL(virStorageFileFormat); typedef struct _virStorageFileMetadata { char *backingStore; int backingStoreFormat; + bool backingStoreIsFile; unsigned long long capacity; bool encrypted; } virStorageFileMetadata; -- 1.7.1 -- Thanks, Adam