On Mon, 2011-04-04 at 16:39 -0600, Eric Blake wrote:
On 03/27/2011 07:30 PM, Jesse Cook wrote:
This patch enables the relative backing file path support provided by qemu-img create.
If a relative path is specified for the backing file, it is converted to an absolute path using the storage pool path. The absolute path is used to verify that the backing file exists. If the backing file exists, the relative path is allowed and will be provided to qemu-img create.
This patch takes the place of a previous patch: http://www.redhat.com/archives/libvir-list/2011-March/msg00179.html --- src/storage/storage_backend.c | 18 +++++++++++++++++- 1 files changed, 17 insertions(+), 1 deletions(-)
Sorry for not reviewing this in time for 0.9.0.
No Problem. It took me a while to get around to the fix.
@@ -686,7 +689,20 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, vol->backingStore.format); return -1; } - if (access(vol->backingStore.path, R_OK) != 0) { + + /* Convert relative backing store paths to absolute paths for access + * validation. + */ + if ('/' != *(vol->backingStore.path)) { + virAsprintf(&absolutePath, "%s/%s", pool->def->target.path, + vol->backingStore.path); + + } else { + virAsprintf(&absolutePath, "%s", vol->backingStore.path);
strdup is more efficient here, and avoiding malloc in the first place even more so.
+ } + accessRetCode = access(absolutePath, R_OK);
This could segfault on OOM.
+ VIR_FREE(absolutePath);
I believe there needs to be a NULL check here or absolute paths and virAsprintf errors will segfault. I can patch if you don't beat me to it.
+ if (accessRetCode != 0) { virReportSystemError(errno, _("inaccessible backing store volume %s"), vol->backingStore.path);
ACK with nits fixed; so here's what I squashed in before pushing. I also updated AUTHORS to pass 'make syntax-check'; feel free to let me know off-list if you prefer any alternate spelling there (especially since you sent v1 and v2 under different email aliases).
diff --git i/src/storage/storage_backend.c w/src/storage/storage_backend.c index 5f5565b..8af2878 100644 --- i/src/storage/storage_backend.c +++ w/src/storage/storage_backend.c @@ -693,7 +693,6 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
if (vol->backingStore.path) { int accessRetCode = -1; - char *absolutePath = NULL;
/* XXX: Not strictly required: qemu-img has an option a different @@ -719,14 +718,14 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, /* Convert relative backing store paths to absolute paths for access * validation. */ - if ('/' != *(vol->backingStore.path)) { + if ('/' != *(vol->backingStore.path) && virAsprintf(&absolutePath, "%s/%s", pool->def->target.path, - vol->backingStore.path); - - } else { - virAsprintf(&absolutePath, "%s", vol->backingStore.path); + vol->backingStore.path) < 0) { + virReportOOMError(); + return -1; } - accessRetCode = access(absolutePath, R_OK); + accessRetCode = access(absolutePath ? absolutePath + : vol->backingStore.path, R_OK); VIR_FREE(absolutePath); if (accessRetCode != 0) { virReportSystemError(errno,
-- Jesse Cook Research Scientist EADS NA Defense Security & Systems Solutions, Inc. (DS3) Email: jesse.cook@eads-na-security.com