Allow the callers to pass down libvirt-internal flags. --- src/conf/storage_conf.c | 25 ++++++++++++++++--------- src/conf/storage_conf.h | 9 ++++++--- src/esx/esx_storage_backend_vmfs.c | 4 ++-- src/parallels/parallels_storage.c | 6 +++--- src/phyp/phyp_driver.c | 2 +- src/storage/storage_driver.c | 4 ++-- src/test/test_driver.c | 6 +++--- src/vbox/vbox_storage.c | 2 +- tests/storagebackendsheepdogtest.c | 2 +- tests/storagevolxml2argvtest.c | 4 ++-- tests/storagevolxml2xmltest.c | 13 ++++++++----- 11 files changed, 45 insertions(+), 32 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index e9aaa8a..c8a860b 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1245,7 +1245,8 @@ virStorageSize(const char *unit, static virStorageVolDefPtr virStorageVolDefParseXML(virStoragePoolDefPtr pool, - xmlXPathContextPtr ctxt) + xmlXPathContextPtr ctxt, + unsigned long flags) { virStorageVolDefPtr ret; virStorageVolOptionsPtr options; @@ -1259,6 +1260,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, size_t i; int n; + virCheckFlags(0, NULL); + options = virStorageVolOptionsForPoolType(pool->type); if (options == NULL) return NULL; @@ -1429,7 +1432,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, virStorageVolDefPtr virStorageVolDefParseNode(virStoragePoolDefPtr pool, xmlDocPtr xml, - xmlNodePtr root) + xmlNodePtr root, + unsigned long flags) { xmlXPathContextPtr ctxt = NULL; virStorageVolDefPtr def = NULL; @@ -1449,7 +1453,7 @@ virStorageVolDefParseNode(virStoragePoolDefPtr pool, } ctxt->node = root; - def = virStorageVolDefParseXML(pool, ctxt); + def = virStorageVolDefParseXML(pool, ctxt, flags); cleanup: xmlXPathFreeContext(ctxt); return def; @@ -1458,13 +1462,14 @@ virStorageVolDefParseNode(virStoragePoolDefPtr pool, static virStorageVolDefPtr virStorageVolDefParse(virStoragePoolDefPtr pool, const char *xmlStr, - const char *filename) + const char *filename, + unsigned long flags) { virStorageVolDefPtr ret = NULL; xmlDocPtr xml; if ((xml = virXMLParse(filename, xmlStr, _("(storage_volume_definition)")))) { - ret = virStorageVolDefParseNode(pool, xml, xmlDocGetRootElement(xml)); + ret = virStorageVolDefParseNode(pool, xml, xmlDocGetRootElement(xml), flags); xmlFreeDoc(xml); } @@ -1473,16 +1478,18 @@ virStorageVolDefParse(virStoragePoolDefPtr pool, virStorageVolDefPtr virStorageVolDefParseString(virStoragePoolDefPtr pool, - const char *xmlStr) + const char *xmlStr, + unsigned long flags) { - return virStorageVolDefParse(pool, xmlStr, NULL); + return virStorageVolDefParse(pool, xmlStr, NULL, flags); } virStorageVolDefPtr virStorageVolDefParseFile(virStoragePoolDefPtr pool, - const char *filename) + const char *filename, + unsigned long flags) { - return virStorageVolDefParse(pool, NULL, filename); + return virStorageVolDefParse(pool, NULL, filename, flags); } static void diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 2c9eaed..2162426 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -347,14 +347,17 @@ char *virStoragePoolDefFormat(virStoragePoolDefPtr def); virStorageVolDefPtr virStorageVolDefParseString(virStoragePoolDefPtr pool, - const char *xml); + const char *xml, + unsigned long flags); virStorageVolDefPtr virStorageVolDefParseFile(virStoragePoolDefPtr pool, - const char *filename); + const char *filename, + unsigned long flags); virStorageVolDefPtr virStorageVolDefParseNode(virStoragePoolDefPtr pool, xmlDocPtr xml, - xmlNodePtr root); + xmlNodePtr root, + unsigned long flags); char *virStorageVolDefFormat(virStoragePoolDefPtr pool, virStorageVolDefPtr def); diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index fa423e9..0dcf419 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -864,7 +864,7 @@ esxStorageVolCreateXML(virStoragePoolPtr pool, } /* Parse config */ - def = virStorageVolDefParseString(&poolDef, xmldesc); + def = virStorageVolDefParseString(&poolDef, xmldesc, 0); if (!def) goto cleanup; @@ -1085,7 +1085,7 @@ esxStorageVolCreateXMLFrom(virStoragePoolPtr pool, goto cleanup; /* Parse config */ - def = virStorageVolDefParseString(&poolDef, xmldesc); + def = virStorageVolDefParseString(&poolDef, xmldesc, 0); if (!def) goto cleanup; diff --git a/src/parallels/parallels_storage.c b/src/parallels/parallels_storage.c index d2c5bf2..a6980a4 100644 --- a/src/parallels/parallels_storage.c +++ b/src/parallels/parallels_storage.c @@ -1208,9 +1208,9 @@ parallelsStorageVolDefineXML(virStoragePoolObjPtr pool, char *xml_path = NULL; if (xmlfile) - privvol = virStorageVolDefParseFile(pool->def, xmlfile); + privvol = virStorageVolDefParseFile(pool->def, xmlfile, 0); else - privvol = virStorageVolDefParseString(pool->def, xmldesc); + privvol = virStorageVolDefParseString(pool->def, xmldesc, 0); if (privvol == NULL) goto cleanup; @@ -1335,7 +1335,7 @@ parallelsStorageVolCreateXMLFrom(virStoragePoolPtr pool, goto cleanup; } - privvol = virStorageVolDefParseString(privpool->def, xmldesc); + privvol = virStorageVolDefParseString(privpool->def, xmldesc, 0); if (privvol == NULL) goto cleanup; diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index d05f897..dbf76e2 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -2026,7 +2026,7 @@ phypStorageVolCreateXML(virStoragePoolPtr pool, goto err; } - if ((voldef = virStorageVolDefParseString(spdef, xml)) == NULL) { + if ((voldef = virStorageVolDefParseString(spdef, xml, 0)) == NULL) { VIR_ERROR(_("Error parsing volume XML.")); goto err; } diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index ad92c9b..bc16e87 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1649,7 +1649,7 @@ storageVolCreateXML(virStoragePoolPtr obj, if ((backend = virStorageBackendForType(pool->def->type)) == NULL) goto cleanup; - voldef = virStorageVolDefParseString(pool->def, xmldesc); + voldef = virStorageVolDefParseString(pool->def, xmldesc, 0); if (voldef == NULL) goto cleanup; @@ -1810,7 +1810,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, goto cleanup; } - newvol = virStorageVolDefParseString(pool->def, xmldesc); + newvol = virStorageVolDefParseString(pool->def, xmldesc, 0); if (newvol == NULL) goto cleanup; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index a386270..718e84a 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1230,7 +1230,7 @@ testOpenVolumesForPool(const char *file, if (!node) goto error; - def = virStorageVolDefParseNode(pool->def, ctxt->doc, node); + def = virStorageVolDefParseNode(pool->def, ctxt->doc, node, 0); if (!def) goto error; @@ -5430,7 +5430,7 @@ testStorageVolCreateXML(virStoragePoolPtr pool, goto cleanup; } - privvol = virStorageVolDefParseString(privpool->def, xmldesc); + privvol = virStorageVolDefParseString(privpool->def, xmldesc, 0); if (privvol == NULL) goto cleanup; @@ -5504,7 +5504,7 @@ testStorageVolCreateXMLFrom(virStoragePoolPtr pool, goto cleanup; } - privvol = virStorageVolDefParseString(privpool->def, xmldesc); + privvol = virStorageVolDefParseString(privpool->def, xmldesc, 0); if (privvol == NULL) goto cleanup; diff --git a/src/vbox/vbox_storage.c b/src/vbox/vbox_storage.c index 599f917..4367009 100644 --- a/src/vbox/vbox_storage.c +++ b/src/vbox/vbox_storage.c @@ -430,7 +430,7 @@ vboxStorageVolCreateXML(virStoragePoolPtr pool, memset(&poolDef, 0, sizeof(poolDef)); poolDef.type = VIR_STORAGE_POOL_DIR; - if ((def = virStorageVolDefParseString(&poolDef, xml)) == NULL) + if ((def = virStorageVolDefParseString(&poolDef, xml, 0)) == NULL) goto cleanup; if (!def->name || diff --git a/tests/storagebackendsheepdogtest.c b/tests/storagebackendsheepdogtest.c index 14fc76d..c85a9b4 100644 --- a/tests/storagebackendsheepdogtest.c +++ b/tests/storagebackendsheepdogtest.c @@ -100,7 +100,7 @@ test_vdi_list_parser(collie_test test, char *poolxml, char *volxml) if (!(pool = virStoragePoolDefParseString(poolXmlData))) goto cleanup; - if (!(vol = virStorageVolDefParseString(pool, volXmlData))) + if (!(vol = virStorageVolDefParseString(pool, volXmlData, 0))) goto cleanup; if (VIR_STRDUP(output, test.output) < 0) diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index f9d2d2d..c1f8a9d 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -84,11 +84,11 @@ testCompareXMLToArgvFiles(bool shouldFail, goto cleanup; } - if (!(vol = virStorageVolDefParseString(pool, volXmlData))) + if (!(vol = virStorageVolDefParseString(pool, volXmlData, 0))) goto cleanup; if (inputvolxml && - !(inputvol = virStorageVolDefParseString(inputpool, inputvolXmlData))) + !(inputvol = virStorageVolDefParseString(inputpool, inputvolXmlData, 0))) goto cleanup; testSetVolumeType(vol, pool); diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c index cf4d401..ee0495b 100644 --- a/tests/storagevolxml2xmltest.c +++ b/tests/storagevolxml2xmltest.c @@ -18,7 +18,7 @@ static int testCompareXMLToXMLFiles(const char *poolxml, const char *inxml, - const char *outxml) + const char *outxml, unsigned long flags) { char *poolXmlData = NULL; char *inXmlData = NULL; @@ -38,7 +38,7 @@ testCompareXMLToXMLFiles(const char *poolxml, const char *inxml, if (!(pool = virStoragePoolDefParseString(poolXmlData))) goto fail; - if (!(dev = virStorageVolDefParseString(pool, inXmlData))) + if (!(dev = virStorageVolDefParseString(pool, inXmlData, flags))) goto fail; if (!(actual = virStorageVolDefFormat(pool, dev))) @@ -64,6 +64,7 @@ testCompareXMLToXMLFiles(const char *poolxml, const char *inxml, struct testInfo { const char *pool; const char *name; + unsigned long flags; }; static int @@ -84,7 +85,7 @@ testCompareXMLToXMLHelper(const void *data) goto cleanup; } - result = testCompareXMLToXMLFiles(poolxml, inxml, outxml); + result = testCompareXMLToXMLFiles(poolxml, inxml, outxml, info->flags); cleanup: VIR_FREE(poolxml); @@ -100,15 +101,17 @@ mymain(void) { int ret = 0; -#define DO_TEST(pool, name) \ +#define DO_TEST_FULL(pool, name, flags) \ do { \ - struct testInfo info = { pool, name }; \ + struct testInfo info = { pool, name, flags }; \ if (virtTestRun("Storage Vol XML-2-XML " name, \ testCompareXMLToXMLHelper, &info) < 0) \ ret = -1; \ } \ while (0); +#define DO_TEST(pool, name) DO_TEST_FULL(pool, name, 0) + DO_TEST("pool-dir", "vol-file"); DO_TEST("pool-dir", "vol-file-naming"); DO_TEST("pool-dir", "vol-file-backing"); -- 2.0.5

On Wed, Feb 25, 2015 at 09:17:26AM +0100, Peter Krempa wrote: I have absolutely no excuse for the blatant waste of memory and screen space. I have changed the type to unsigned int in my local branch. Jan

On Thu, Feb 19, 2015 at 15:59:12 +0100, Ján Tomko wrote: I had to trace the code to understand what this patch was about ... Note that this change is on the "!convert" (create) path ... [1] Few lines below this change there's the following hunk: /* Is there ever a valid case for this? */ if (newvol->target.capacity < origvol->target.capacity) newvol->target.capacity = origvol->target.capacity; Which sets the capacity to the capacity of the original volume. While this is probably semantically OK it might be worth tweaking the comment. [1] ... while the test you are adding is testing the convert path. I think that the hunk [1] belongs to a different patch as the size argument was added only on the "create" path anyways. ACK after the release with hunk [1] removed. Peter

Do not use relPath, it has not been filled by virStorageFileMetadataNew. --- src/util/virstoragefile.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 8d3d1f5..029fe17 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1030,12 +1030,12 @@ virStorageFileGetMetadataFromFD(const char *path, } if (lseek(fd, 0, SEEK_SET) == (off_t)-1) { - virReportSystemError(errno, _("cannot seek to start of '%s'"), meta->relPath); + virReportSystemError(errno, _("cannot seek to start of '%s'"), meta->path); goto cleanup; } if ((len = virFileReadHeaderFD(fd, len, &buf)) < 0) { - virReportSystemError(errno, _("cannot read header '%s'"), meta->relPath); + virReportSystemError(errno, _("cannot read header '%s'"), meta->path); goto cleanup; } -- 2.0.5

On Wed, Feb 25, 2015 at 09:49:31AM +0100, Peter Krempa wrote: I have pushed this one out of the (arbitrary) order. Jan

On Thu, Feb 19, 2015 at 15:59:14 +0100, Ján Tomko wrote: This function has a parameter named 'updateCapacity' ... ... that is obeyed when updating the capacity via the stat call ... ... but this new code ignores it. Is there a specific reason for that? Peter

This reverts commit f1856eb622fde2e6c3a6a932d1dded7f1691d205. Now that we can update capacity from image metadata, we don't need to skip the update. --- src/storage/storage_backend.c | 20 +++++--------------- src/storage/storage_backend.h | 5 +---- src/storage/storage_backend_disk.c | 4 ++-- src/storage/storage_backend_fs.c | 10 ++++------ src/storage/storage_backend_gluster.c | 2 +- src/storage/storage_backend_logical.c | 2 +- src/storage/storage_backend_mpath.c | 2 +- src/storage/storage_backend_scsi.c | 2 +- 8 files changed, 16 insertions(+), 31 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 8471744..ea5cabe 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1508,7 +1508,6 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, int virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, - bool updateCapacity, bool withBlockVolFormat, unsigned int openflags) { @@ -1522,8 +1521,7 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, goto cleanup; fd = ret; - if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb, - updateCapacity)) < 0) + if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb)) < 0) goto cleanup; if (target->type == VIR_STORAGE_VOL_FILE && @@ -1566,21 +1564,18 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, - bool updateCapacity, bool withBlockVolFormat, unsigned int openflags) { int ret; if ((ret = virStorageBackendUpdateVolTargetInfo(&vol->target, - updateCapacity, withBlockVolFormat, openflags)) < 0) return ret; if (vol->target.backingStore && (ret = virStorageBackendUpdateVolTargetInfo(vol->target.backingStore, - updateCapacity, withBlockVolFormat, VIR_STORAGE_VOL_OPEN_DEFAULT | VIR_STORAGE_VOL_OPEN_NOERROR) < 0)) @@ -1594,15 +1589,13 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, * @target: target definition ptr of volume to update * @fd: fd of storage volume to update, via virStorageBackendOpenVol*, or -1 * @sb: details about file (must match @fd, if that is provided) - * @updateCapacity: If true, updated capacity info will be stored * * Returns 0 for success, -1 on a legitimate error condition. */ int virStorageBackendUpdateVolTargetInfoFD(virStorageSourcePtr target, int fd, - struct stat *sb, - bool updateCapacity) + struct stat *sb) { #if WITH_SELINUX security_context_t filecon = NULL; @@ -1618,12 +1611,10 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageSourcePtr target, /* Regular files may be sparse, so logical size (capacity) is not same * as actual allocation above */ - if (updateCapacity) - target->capacity = sb->st_size; + target->capacity = sb->st_size; } else if (S_ISDIR(sb->st_mode)) { target->allocation = 0; - if (updateCapacity) - target->capacity = 0; + target->capacity = 0; } else if (fd >= 0) { off_t end; /* XXX this is POSIX compliant, but doesn't work for CHAR files, @@ -1639,8 +1630,7 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageSourcePtr target, return -1; } target->allocation = end; - if (updateCapacity) - target->capacity = end; + target->capacity = end; } if (!target->perms && VIR_ALLOC(target->perms) < 0) diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index a93df5d..9f1db60 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -179,17 +179,14 @@ int virStorageBackendVolOpen(const char *path, struct stat *sb, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, - bool updateCapacity, bool withBlockVolFormat, unsigned int openflags); int virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, - bool updateCapacity, bool withBlockVolFormat, unsigned int openflags); int virStorageBackendUpdateVolTargetInfoFD(virStorageSourcePtr target, int fd, - struct stat *sb, - bool updateCapacity); + struct stat *sb); char *virStorageBackendStablePath(virStoragePoolObjPtr pool, const char *devpath, diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 9f4d76a..f62b23e 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -152,14 +152,14 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool, * -1 was returned indicating some other error than an open error. */ if (vol->source.partType == VIR_STORAGE_VOL_DISK_TYPE_EXTENDED) { - if (virStorageBackendUpdateVolInfo(vol, true, false, + if (virStorageBackendUpdateVolInfo(vol, false, VIR_STORAGE_VOL_OPEN_DEFAULT | VIR_STORAGE_VOL_OPEN_NOERROR) == -1) return -1; vol->target.allocation = vol->target.capacity = (vol->source.extents[0].end - vol->source.extents[0].start); } else { - if (virStorageBackendUpdateVolInfo(vol, true, false, + if (virStorageBackendUpdateVolInfo(vol, false, VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) return -1; } diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 2af5dd7..77d894c 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -80,7 +80,7 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, return rc; /* Take care to propagate rc, it is not always -1 */ fd = rc; - if (virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb, true) < 0) + if (virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb) < 0) goto cleanup; if (S_ISDIR(sb.st_mode)) { @@ -902,7 +902,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, if (vol->target.backingStore) { ignore_value(virStorageBackendUpdateVolTargetInfo(vol->target.backingStore, - true, false, + false, VIR_STORAGE_VOL_OPEN_DEFAULT)); /* If this failed, the backing file is currently unavailable, * the capacity, allocation, owner, group and mode are unknown. @@ -1183,10 +1183,8 @@ virStorageBackendFileSystemVolRefresh(virConnectPtr conn, { int ret; - /* Refresh allocation / permissions info in case its changed - * don't update the capacity value for this pass - */ - ret = virStorageBackendUpdateVolInfo(vol, false, false, + /* Refresh allocation / capacity / permissions info in case its changed */ + ret = virStorageBackendUpdateVolInfo(vol, false, VIR_STORAGE_VOL_FS_OPEN_FLAGS); if (ret < 0) return ret; diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 53e4632..d2e79bc 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -268,7 +268,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, if (VIR_ALLOC(vol) < 0) goto cleanup; - if (virStorageBackendUpdateVolTargetInfoFD(&vol->target, -1, st, true) < 0) + if (virStorageBackendUpdateVolTargetInfoFD(&vol->target, -1, st) < 0) goto cleanup; if (virStorageBackendGlusterSetMetadata(state, vol, name) < 0) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 8aa68a6..7ba8ded 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -161,7 +161,7 @@ virStorageBackendLogicalMakeVol(char **const groups, if (!vol->key && VIR_STRDUP(vol->key, groups[2]) < 0) goto cleanup; - if (virStorageBackendUpdateVolInfo(vol, true, false, + if (virStorageBackendUpdateVolInfo(vol, false, VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) goto cleanup; diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index b67dc2a..44bcd60 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -60,7 +60,7 @@ virStorageBackendMpathNewVol(virStoragePoolObjPtr pool, if (virAsprintf(&vol->target.path, "/dev/%s", dev) < 0) goto cleanup; - if (virStorageBackendUpdateVolInfo(vol, true, true, + if (virStorageBackendUpdateVolInfo(vol, true, VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) { goto cleanup; } diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index c4879b0..58e7e6d 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -206,7 +206,7 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, goto free_vol; } - if (virStorageBackendUpdateVolInfo(vol, true, true, + if (virStorageBackendUpdateVolInfo(vol, true, VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) { retval = -1; goto free_vol; -- 2.0.5

Add VIR_VOL_XML_PARSE_OPT_CAPACITY flag to virStorageVolDefParseXML. With this flag, no error is reported when the capacity is missing if there is a backing store. --- src/conf/storage_conf.c | 6 ++++-- src/conf/storage_conf.h | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index ca8fc9b..9723696 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1260,7 +1260,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, size_t i; int n; - virCheckFlags(VIR_VOL_XML_PARSE_NO_CAPACITY, NULL); + virCheckFlags(VIR_VOL_XML_PARSE_NO_CAPACITY | + VIR_VOL_XML_PARSE_OPT_CAPACITY, NULL); options = virStorageVolOptionsForPoolType(pool->type); if (options == NULL) @@ -1325,7 +1326,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (capacity) { if (virStorageSize(unit, capacity, &ret->target.capacity) < 0) goto error; - } else if (!(flags & VIR_VOL_XML_PARSE_NO_CAPACITY)) { + } else if (!(flags & VIR_VOL_XML_PARSE_NO_CAPACITY) && + !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) && ret->target.backingStore)) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing capacity element")); goto error; } diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index cd3d09c..21cd584 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -348,6 +348,8 @@ char *virStoragePoolDefFormat(virStoragePoolDefPtr def); typedef enum { /* do not require volume capacity at all */ VIR_VOL_XML_PARSE_NO_CAPACITY = 1 << 0, + /* do not require volume capacity if the volume has a backing store */ + VIR_VOL_XML_PARSE_OPT_CAPACITY = 1 << 1, } virStorageVolDefParseFlags; virStorageVolDefPtr virStorageVolDefParseString(virStoragePoolDefPtr pool, -- 2.0.5

The tool creating the image can get the capacity from the backing storage. Just refresh the volume afterwards. https://bugzilla.redhat.com/show_bug.cgi?id=958510 --- src/storage/storage_backend.c | 6 ++++++ src/storage/storage_backend_fs.c | 7 +++++++ src/storage/storage_backend_rbd.c | 6 ++++++ src/storage/storage_backend_sheepdog.c | 6 ++++++ src/storage/storage_driver.c | 14 ++++++++++++- tests/storagevolxml2argvdata/qcow2-nocapacity.argv | 5 +++++ tests/storagevolxml2argvtest.c | 20 +++++++++++-------- .../vol-qcow2-nocapacity-backing.xml | 23 ++++++++++++++++++++++ 8 files changed, 78 insertions(+), 9 deletions(-) create mode 100644 tests/storagevolxml2argvdata/qcow2-nocapacity.argv create mode 100644 tests/storagevolxml2xmlin/vol-qcow2-nocapacity-backing.xml diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index ea5cabe..a4e75f5a5 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -487,6 +487,12 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } + if (vol->target.backingStore) { + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("backing storage not supported for raw volumes")); + goto cleanup; + } + if (flags & VIR_STORAGE_VOL_CREATE_REFLINK) reflink_copy = true; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 77d894c..35385db 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1037,6 +1037,13 @@ static int createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED, return -1; } + if (vol->target.backingStore) { + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("backing storage not supported for directories volumes")); + return -1; + } + + if ((err = virDirCreate(vol->target.path, vol->target.perms->mode, vol->target.perms->uid, vol->target.perms->gid, diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 57182de..ae4bcb3 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -509,6 +509,12 @@ virStorageBackendRBDBuildVol(virConnectPtr conn, virCheckFlags(0, -1); + if (!vol->target.capacity) { + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("volume capacity required for this storage pool")); + goto cleanup; + } + if (virStorageBackendRBDOpenRADOSConn(&ptr, conn, &pool->def->source) < 0) goto cleanup; diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index 9419859..f389d9b 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -266,6 +266,12 @@ virStorageBackendSheepdogBuildVol(virConnectPtr conn, virCheckFlags(0, -1); + if (!vol->target.capacity) { + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("volume capacity required for this pool")); + goto cleanup; + } + virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "create", vol->name, NULL); virCommandAddArgFormat(cmd, "%llu", vol->target.capacity); virStorageBackendSheepdogAddHostArg(cmd, pool); diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 409b486..e8af90f 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1649,10 +1649,18 @@ storageVolCreateXML(virStoragePoolPtr obj, if ((backend = virStorageBackendForType(pool->def->type)) == NULL) goto cleanup; - voldef = virStorageVolDefParseString(pool->def, xmldesc, 0); + voldef = virStorageVolDefParseString(pool->def, xmldesc, + VIR_VOL_XML_PARSE_OPT_CAPACITY); if (voldef == NULL) goto cleanup; + if (!voldef->target.capacity && !backend->buildVol) { + virReportError(VIR_ERR_NO_SUPPORT, + "%s", _("volume capacity required for this " + "storage pool")); + goto cleanup; + } + if (virStorageVolCreateXMLEnsureACL(obj->conn, pool->def, voldef) < 0) goto cleanup; @@ -1725,6 +1733,10 @@ storageVolCreateXML(virStoragePoolPtr obj, } + if (backend->refreshVol && + backend->refreshVol(obj->conn, pool, voldef) < 0) + goto cleanup; + /* Update pool metadata */ pool->def->allocation += buildvoldef->target.allocation; pool->def->available -= buildvoldef->target.allocation; diff --git a/tests/storagevolxml2argvdata/qcow2-nocapacity.argv b/tests/storagevolxml2argvdata/qcow2-nocapacity.argv new file mode 100644 index 0000000..1198cba --- /dev/null +++ b/tests/storagevolxml2argvdata/qcow2-nocapacity.argv @@ -0,0 +1,5 @@ +qemu-img create \ +-f qcow2 \ +-b /dev/null \ +-o backing_fmt=raw,encryption=on \ +/var/lib/libvirt/images/OtherDemo.img diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 52bb856..696659c 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -40,7 +40,8 @@ testCompareXMLToArgvFiles(bool shouldFail, const char *inputvolxml, const char *cmdline, unsigned int flags, - int imgformat) + int imgformat, + unsigned long parse_flags) { char *volXmlData = NULL; char *poolXmlData = NULL; @@ -49,7 +50,6 @@ testCompareXMLToArgvFiles(bool shouldFail, char *expectedCmdline = NULL; char *actualCmdline = NULL; int ret = -1; - unsigned long parse_flags = 0; int len; @@ -149,6 +149,7 @@ struct testInfo { const char *cmdline; unsigned int flags; int imgformat; + unsigned long parseflags; }; static int @@ -183,7 +184,7 @@ testCompareXMLToArgvHelper(const void *data) result = testCompareXMLToArgvFiles(info->shouldFail, poolxml, volxml, inputpoolxml, inputvolxml, cmdline, info->flags, - info->imgformat); + info->imgformat, info->parseflags); cleanup: VIR_FREE(poolxml); @@ -210,11 +211,11 @@ mymain(void) int ret = 0; unsigned int flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; -#define DO_TEST_FULL(shouldFail, pool, vol, inputpool, inputvol, cmdline, \ - flags, imgformat) \ +#define DO_TEST_FULL(shouldFail, parseflags, pool, vol, inputpool, inputvol, \ + cmdline, flags, imgformat) \ do { \ struct testInfo info = { shouldFail, pool, vol, inputpool, inputvol, \ - cmdline, flags, imgformat }; \ + cmdline, flags, imgformat, parseflags }; \ if (virtTestRun("Storage Vol XML-2-argv " cmdline, \ testCompareXMLToArgvHelper, &info) < 0) \ ret = -1; \ @@ -222,10 +223,10 @@ mymain(void) while (0); #define DO_TEST(pool, ...) \ - DO_TEST_FULL(false, pool, __VA_ARGS__) + DO_TEST_FULL(false, 0, pool, __VA_ARGS__) #define DO_TEST_FAIL(pool, ...) \ - DO_TEST_FULL(true, pool, __VA_ARGS__) + DO_TEST_FULL(true, 0, pool, __VA_ARGS__) DO_TEST("pool-dir", "vol-qcow2", NULL, NULL, @@ -312,6 +313,9 @@ mymain(void) DO_TEST("pool-dir", "vol-qcow2-nocapacity", "pool-dir", "vol-file", "qcow2-nocapacity-convert-prealloc", flags, FMT_OPTIONS); + DO_TEST_FULL(false, VIR_VOL_XML_PARSE_OPT_CAPACITY, + "pool-dir", "vol-qcow2-nocapacity-backing", NULL, NULL, + "qcow2-nocapacity", 0, FMT_OPTIONS); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/storagevolxml2xmlin/vol-qcow2-nocapacity-backing.xml b/tests/storagevolxml2xmlin/vol-qcow2-nocapacity-backing.xml new file mode 100644 index 0000000..f8e439b --- /dev/null +++ b/tests/storagevolxml2xmlin/vol-qcow2-nocapacity-backing.xml @@ -0,0 +1,23 @@ +<volume> + <name>OtherDemo.img</name> + <key>/var/lib/libvirt/images/OtherDemo.img</key> + <source> + </source> + <target> + <path>/var/lib/libvirt/images/OtherDemo.img</path> + <format type='qcow2'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + <encryption format='qcow'> + <secret type='passphrase' uuid='e78d4b51-a2af-485f-b0f5-afca709a80f4'/> + </encryption> + </target> + <backingStore> + <path>/dev/null</path> + <format type='raw'/> + </backingStore> +</volume> -- 2.0.5