[libvirt] [PATCH 0/8] storage: separate storage volume creation from building
This series splits the internal storage volume metadata creation from actual volume building and adds support for deletion of guster volumes. Peter Krempa (8): storage: fs: Fix comment for virStorageBackendFileSystemDelete storage: Support deletion of volumes on gluster pools storage: lvm: Avoid forward decl of virStorageBackendLogicalDeleteVol storage: lvm: Separate creating of the volume from building storage: disk: Separate creating of the volume from building storage: RBD: Separate creating of the volume from building storage: Sheepdog: Separate creating of the volume from building storage: Improve error message when a storage backend is missing src/storage/storage_backend.c | 3 +- src/storage/storage_backend_disk.c | 44 ++++++++---- src/storage/storage_backend_fs.c | 6 +- src/storage/storage_backend_gluster.c | 64 +++++++++++++++++ src/storage/storage_backend_logical.c | 127 ++++++++++++++++++--------------- src/storage/storage_backend_rbd.c | 41 +++++++++-- src/storage/storage_backend_sheepdog.c | 29 +++++++- 7 files changed, 226 insertions(+), 88 deletions(-) -- 1.8.5.2
The comment was talking about creating the pool while the function is deleting it. Fix the mismatch. --- src/storage/storage_backend_fs.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 95783be..6ebdd46 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -962,11 +962,9 @@ virStorageBackendFileSystemStop(virConnectPtr conn ATTRIBUTE_UNUSED, /** * @conn connection to report errors against - * @pool storage pool to build - * - * Build a directory or FS based storage pool. + * @pool storage pool to delete * - * - If it is a FS based pool, mounts the unlying source device on the pool + * Delete a directory based storage pool * * Returns 0 on success, -1 on error */ -- 1.8.5.2
On 01/06/2014 09:44 AM, Peter Krempa wrote:
The comment was talking about creating the pool while the function is deleting it. Fix the mismatch. --- src/storage/storage_backend_fs.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
ACK, safe for 1.2.1
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 95783be..6ebdd46 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -962,11 +962,9 @@ virStorageBackendFileSystemStop(virConnectPtr conn ATTRIBUTE_UNUSED,
/** * @conn connection to report errors against - * @pool storage pool to build - * - * Build a directory or FS based storage pool. + * @pool storage pool to delete * - * - If it is a FS based pool, mounts the unlying source device on the pool
While you're deleting this one, could you fix the other three instances in the file to s/unlying/underlying/?
+ * Delete a directory based storage pool * * Returns 0 on success, -1 on error */
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/09/14 22:06, Eric Blake wrote:
On 01/06/2014 09:44 AM, Peter Krempa wrote:
The comment was talking about creating the pool while the function is deleting it. Fix the mismatch. --- src/storage/storage_backend_fs.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
ACK, safe for 1.2.1
Thanks; Pushed.
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 95783be..6ebdd46 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -962,11 +962,9 @@ virStorageBackendFileSystemStop(virConnectPtr conn ATTRIBUTE_UNUSED,
/** * @conn connection to report errors against - * @pool storage pool to build - * - * Build a directory or FS based storage pool. + * @pool storage pool to delete * - * - If it is a FS based pool, mounts the unlying source device on the pool
While you're deleting this one, could you fix the other three instances in the file to s/unlying/underlying/?
Will do as a follow up. Peter
Implement the "deleteVol" storage backend function for gluster volumes. --- src/storage/storage_backend_gluster.c | 64 +++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 2ec2424..c73cf8a 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -382,8 +382,72 @@ cleanup: return ret; } + +static int +virStorageBackendGlusterVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + unsigned int flags) +{ + virStorageBackendGlusterStatePtr state = NULL; + int ret = -1; + + virCheckFlags(0, -1); + + switch ((virStorageVolType) vol->type) { + case VIR_STORAGE_VOL_FILE: + case VIR_STORAGE_VOL_DIR: + case VIR_STORAGE_VOL_BLOCK: + case VIR_STORAGE_VOL_LAST: + virReportError(VIR_ERR_NO_SUPPORT, + _("removing of '%s' volumes is not supported " + "by the gluster backend: %s"), + virStorageVolTypeToString(vol->type), + vol->target.path); + goto cleanup; + break; + + case VIR_STORAGE_VOL_NETWORK: + if (!(state = virStorageBackendGlusterOpen(pool))) + goto cleanup; + + if (glfs_unlink(state->vol, vol->name) < 0) { + if (errno != ENOENT) { + virReportSystemError(errno, + _("cannot remove gluster volume file '%s'"), + vol->target.path); + goto cleanup; + } + } + break; + + case VIR_STORAGE_VOL_NETDIR: + if (!(state = virStorageBackendGlusterOpen(pool))) + goto cleanup; + + if (glfs_rmdir(state->vol, vol->target.path) < 0) { + if (errno != ENOENT) { + virReportSystemError(errno, + _("cannot remove gluster volume dir '%s'"), + vol->target.path); + goto cleanup; + } + } + break; + } + + ret = 0; + +cleanup: + virStorageBackendGlusterClose(state); + return ret; +} + + virStorageBackend virStorageBackendGluster = { .type = VIR_STORAGE_POOL_GLUSTER, .refreshPool = virStorageBackendGlusterRefreshPool, + + .deleteVol = virStorageBackendGlusterVolDelete, }; -- 1.8.5.2
On 01/06/2014 09:44 AM, Peter Krempa wrote:
Implement the "deleteVol" storage backend function for gluster volumes. --- src/storage/storage_backend_gluster.c | 64 +++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+)
ACK, but this starts to get into feature addition, so I'm not sure whether to make it wait until after 1.2.1. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Change code ordering to avoid the need for a forward declaration. --- src/storage/storage_backend_logical.c | 67 ++++++++++++++++------------------- 1 file changed, 31 insertions(+), 36 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 10966cc..15b86dc 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -667,10 +667,38 @@ cleanup: static int -virStorageBackendLogicalDeleteVol(virConnectPtr conn, - virStoragePoolObjPtr pool, +virStorageBackendLogicalDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol, - unsigned int flags); + unsigned int flags) +{ + int ret = -1; + + virCommandPtr lvchange_cmd = NULL; + virCommandPtr lvremove_cmd = NULL; + + virCheckFlags(0, -1); + + virFileWaitForDevices(); + + lvchange_cmd = virCommandNewArgList(LVCHANGE, "-aln", vol->target.path, NULL); + lvremove_cmd = virCommandNewArgList(LVREMOVE, "-f", vol->target.path, NULL); + + if (virCommandRun(lvremove_cmd, NULL) < 0) { + if (virCommandRun(lvchange_cmd, NULL) < 0) { + goto cleanup; + } else { + if (virCommandRun(lvremove_cmd, NULL) < 0) + goto cleanup; + } + } + + ret = 0; +cleanup: + virCommandFree(lvchange_cmd); + virCommandFree(lvremove_cmd); + return ret; +} static int @@ -784,39 +812,6 @@ virStorageBackendLogicalBuildVolFrom(virConnectPtr conn, return build_func(conn, pool, vol, inputvol, flags); } -static int -virStorageBackendLogicalDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, - virStorageVolDefPtr vol, - unsigned int flags) -{ - int ret = -1; - - virCommandPtr lvchange_cmd = NULL; - virCommandPtr lvremove_cmd = NULL; - - virCheckFlags(0, -1); - - virFileWaitForDevices(); - - lvchange_cmd = virCommandNewArgList(LVCHANGE, "-aln", vol->target.path, NULL); - lvremove_cmd = virCommandNewArgList(LVREMOVE, "-f", vol->target.path, NULL); - - if (virCommandRun(lvremove_cmd, NULL) < 0) { - if (virCommandRun(lvchange_cmd, NULL) < 0) { - goto cleanup; - } else { - if (virCommandRun(lvremove_cmd, NULL) < 0) - goto cleanup; - } - } - - ret = 0; -cleanup: - virCommandFree(lvchange_cmd); - virCommandFree(lvremove_cmd); - return ret; -} virStorageBackend virStorageBackendLogical = { .type = VIR_STORAGE_POOL_LOGICAL, -- 1.8.5.2
On 01/06/2014 09:44 AM, Peter Krempa wrote:
Change code ordering to avoid the need for a forward declaration. --- src/storage/storage_backend_logical.c | 67 ++++++++++++++++------------------- 1 file changed, 31 insertions(+), 36 deletions(-)
ACK; fairly mechanical. This one is safe for 1.2.1 -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/09/14 23:32, Eric Blake wrote:
On 01/06/2014 09:44 AM, Peter Krempa wrote:
Change code ordering to avoid the need for a forward declaration. --- src/storage/storage_backend_logical.c | 67 ++++++++++++++++------------------- 1 file changed, 31 insertions(+), 36 deletions(-)
ACK; fairly mechanical. This one is safe for 1.2.1
Pushed; Thanks; Peter
Separate the steps to create libvirt's volume metadata from the actual volume building process. This is already done for regular file based pools to allow job support for storage APIs. --- src/storage/storage_backend_logical.c | 60 +++++++++++++++++++++-------------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 15b86dc..039d962 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -702,32 +702,16 @@ cleanup: static int -virStorageBackendLogicalCreateVol(virConnectPtr conn, - virStoragePoolObjPtr pool, - virStorageVolDefPtr vol) +virStorageBackendLogicalBuildVol(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + unsigned int flags) { int fd = -1; virCommandPtr cmd = NULL; virErrorPtr err; - if (vol->target.encryption != NULL) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("storage pool does not support encrypted " - "volumes")); - return -1; - } - - vol->type = VIR_STORAGE_VOL_BLOCK; - - if (vol->target.path != NULL) { - /* A target path passed to CreateVol has no meaning */ - VIR_FREE(vol->target.path); - } - - if (virAsprintf(&vol->target.path, "%s/%s", - pool->def->target.path, - vol->name) == -1) - return -1; + virCheckFlags(0, -1); cmd = virCommandNewArgList(LVCREATE, "--name", vol->name, @@ -786,7 +770,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, return 0; - error: +error: err = virSaveLastError(); VIR_FORCE_CLOSE(fd); virStorageBackendLogicalDeleteVol(conn, pool, vol, 0); @@ -796,6 +780,36 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, return -1; } + +static int +virStorageBackendLogicalCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol) +{ + if (vol->target.encryption != NULL) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("storage pool does not support encrypted volumes")); + return -1; + } + + vol->type = VIR_STORAGE_VOL_BLOCK; + + VIR_FREE(vol->target.path); + if (virAsprintf(&vol->target.path, "%s/%s", + pool->def->target.path, + vol->name) == -1) + return -1; + + if (virFileExists(vol->target.path)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("volume target path '%s' already exists"), + vol->target.path); + return -1; + } + + return 0; +} + static int virStorageBackendLogicalBuildVolFrom(virConnectPtr conn, virStoragePoolObjPtr pool, @@ -823,7 +837,7 @@ virStorageBackend virStorageBackendLogical = { .refreshPool = virStorageBackendLogicalRefreshPool, .stopPool = virStorageBackendLogicalStopPool, .deletePool = virStorageBackendLogicalDeletePool, - .buildVol = NULL, + .buildVol = virStorageBackendLogicalBuildVol, .buildVolFrom = virStorageBackendLogicalBuildVolFrom, .createVol = virStorageBackendLogicalCreateVol, .deleteVol = virStorageBackendLogicalDeleteVol, -- 1.8.5.2
On 01/06/2014 09:44 AM, Peter Krempa wrote:
Separate the steps to create libvirt's volume metadata from the actual volume building process. This is already done for regular file based pools to allow job support for storage APIs. --- src/storage/storage_backend_logical.c | 60 +++++++++++++++++++++-------------- 1 file changed, 37 insertions(+), 23 deletions(-)
ACK; I'm borderline whether this should wait for the release, though. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/09/14 23:40, Eric Blake wrote:
On 01/06/2014 09:44 AM, Peter Krempa wrote:
Separate the steps to create libvirt's volume metadata from the actual volume building process. This is already done for regular file based pools to allow job support for storage APIs. --- src/storage/storage_backend_logical.c | 60 +++++++++++++++++++++-------------- 1 file changed, 37 insertions(+), 23 deletions(-)
ACK; I'm borderline whether this should wait for the release, though.
Now that 1.2.2 is out I've pushed this one and the rest with the same complaint. Peter
On Thu, 16 Jan 2014, Peter Krempa wrote:
On 01/09/14 23:40, Eric Blake wrote:
On 01/06/2014 09:44 AM, Peter Krempa wrote:
Separate the steps to create libvirt's volume metadata from the actual volume building process. This is already done for regular file based pools to allow job support for storage APIs. --- src/storage/storage_backend_logical.c | 60 +++++++++++++++++++++-------------- 1 file changed, 37 insertions(+), 23 deletions(-)
ACK; I'm borderline whether this should wait for the release, though.
Now that 1.2.2 is out I've pushed this one and the rest with the same complaint.
Hi Peter, This breaks volume creation in an LVM pool: # cat volume.xml <volume type='block'> <name>test</name> <capacity unit='bytes'>10737418240</capacity> <allocation unit='bytes'>10737418240</allocation> </volume> # virsh vol-create vg volume.xml error: Failed to create vol from volume.xml error: key in virGetStorageVol must not be NULL The problem is a storage backend's createVol function is expected to set an appropriate key, but for an LVM volume this isn't done now until buildVol is called. The LV's UUID is used as the volume's key. vol-clone and vol-create-from are even worse: libvirtd crashes since the NULL return from virGetStorageVol isn't handled in storageVolCreateXMLFrom. We probably need: --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1828,6 +1828,10 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, pool->volumes.objs[pool->volumes.count++] = newvol; volobj = virGetStorageVol(obj->conn, pool->def->name, newvol->name, newvol->key, NULL, NULL); + if (!volobj) { + pool->volumes.count--; + goto cleanup; + } /* Drop the pool lock during volume allocation */ pool->asyncjobs++; Even with this fix, the LVM backend's buildVolFrom function doesn't look like it will ever actually create the LV. - Michael
On 02/12/2014 07:29 AM, Michael Chapman wrote:
On Thu, 16 Jan 2014, Peter Krempa wrote:
On 01/09/14 23:40, Eric Blake wrote:
On 01/06/2014 09:44 AM, Peter Krempa wrote:
Separate the steps to create libvirt's volume metadata from the actual volume building process. This is already done for regular file based pools to allow job support for storage APIs. --- src/storage/storage_backend_logical.c | 60 +++++++++++++++++++++-------------- 1 file changed, 37 insertions(+), 23 deletions(-)
ACK; I'm borderline whether this should wait for the release, though.
Now that 1.2.2 is out I've pushed this one and the rest with the same complaint.
Hi Peter,
This breaks volume creation in an LVM pool:
# cat volume.xml <volume type='block'> <name>test</name> <capacity unit='bytes'>10737418240</capacity> <allocation unit='bytes'>10737418240</allocation> </volume> # virsh vol-create vg volume.xml error: Failed to create vol from volume.xml error: key in virGetStorageVol must not be NULL
The problem is a storage backend's createVol function is expected to set an appropriate key, but for an LVM volume this isn't done now until buildVol is called. The LV's UUID is used as the volume's key.
For the disk backend, volume keys are also generated in buildVol after the volume is created. IMO we should revert commits: commit af1fb38f55d4fb87e0fcaee1e973fa9c6713b1e6 storage: lvm: Separate creating of the volume from building commit 67ccf91bf29488783bd1fda46b362450f71a2078 storage: disk: Separate creating of the volume from building unless we have a really good reason not to.
vol-clone and vol-create-from are even worse: libvirtd crashes since the NULL return from virGetStorageVol isn't handled in storageVolCreateXMLFrom. We probably need:
--- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1828,6 +1828,10 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, pool->volumes.objs[pool->volumes.count++] = newvol; volobj = virGetStorageVol(obj->conn, pool->def->name, newvol->name, newvol->key, NULL, NULL); + if (!volobj) { + pool->volumes.count--; + goto cleanup; + }
/* Drop the pool lock during volume allocation */ pool->asyncjobs++;
ACK to this patch. No matter what way we fix the actual bug, virGetStorageVol can still return NULL when we're out of memory. Would you like to write a commit message and send it as a formal patch, or shall I do it? Jan
On Wed, 12 Feb 2014, Ján Tomko wrote:
--- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1828,6 +1828,10 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, pool->volumes.objs[pool->volumes.count++] = newvol; volobj = virGetStorageVol(obj->conn, pool->def->name, newvol->name, newvol->key, NULL, NULL); + if (!volobj) { + pool->volumes.count--; + goto cleanup; + }
/* Drop the pool lock during volume allocation */ pool->asyncjobs++;
ACK to this patch. No matter what way we fix the actual bug, virGetStorageVol can still return NULL when we're out of memory. Would you like to write a commit message and send it as a formal patch, or shall I do it?
I'll send a patch through shortly. Cheers, Michael
Separate the steps to create libvirt's volume metadata from the actual volume building process. --- src/storage/storage_backend_disk.c | 44 ++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index a7a7d0e..aa3b72f 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -617,28 +617,43 @@ virStorageBackendDiskPartBoundries(virStoragePoolObjPtr pool, static int virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, + virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol) { + if (vol->target.encryption != NULL) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("storage pool does not support encrypted volumes")); + return -1; + } + + vol->type = VIR_STORAGE_VOL_BLOCK; + + return 0; +} + + +static int +virStorageBackendDiskBuildVol(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + unsigned int flags) +{ int res = -1; char *partFormat = NULL; unsigned long long startOffset = 0, endOffset = 0; - virCommandPtr cmd = virCommandNewArgList(PARTED, - pool->def->source.devices[0].path, - "mkpart", - "--script", - NULL); + virCommandPtr cmd = NULL; - if (vol->target.encryption != NULL) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("storage pool does not support encrypted " - "volumes")); - goto cleanup; - } + virCheckFlags(0, -1); + + cmd = virCommandNewArgList(PARTED, + pool->def->source.devices[0].path, + "mkpart", + "--script", + NULL); - if (virStorageBackendDiskPartFormat(pool, vol, &partFormat) != 0) { + if (virStorageBackendDiskPartFormat(pool, vol, &partFormat) != 0) goto cleanup; - } + virCommandAddArg(cmd, partFormat); if (virStorageBackendDiskPartBoundries(pool, &startOffset, @@ -768,5 +783,6 @@ virStorageBackend virStorageBackendDisk = { .createVol = virStorageBackendDiskCreateVol, .deleteVol = virStorageBackendDiskDeleteVol, + .buildVol = virStorageBackendDiskBuildVol, .buildVolFrom = virStorageBackendDiskBuildVolFrom, }; -- 1.8.5.2
On 01/06/2014 09:44 AM, Peter Krempa wrote:
Separate the steps to create libvirt's volume metadata from the actual volume building process. --- src/storage/storage_backend_disk.c | 44 ++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 14 deletions(-)
ACK, same story as for 4/8 on whether this is appropriate for the release. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Separate the steps to create libvirt's volume metadata from the actual volume building process. --- src/storage/storage_backend_rbd.c | 41 ++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 4b6f18c..c5f0bc5 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -435,9 +435,35 @@ cleanup: return ret; } -static int virStorageBackendRBDCreateVol(virConnectPtr conn, - virStoragePoolObjPtr pool, - virStorageVolDefPtr vol) + +static int +virStorageBackendRBDCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol) +{ + vol->type = VIR_STORAGE_VOL_NETWORK; + + VIR_FREE(vol->target.path); + if (virAsprintf(&vol->target.path, "%s/%s", + pool->def->source.name, + vol->name) == -1) + return -1; + + VIR_FREE(vol->key); + if (virAsprintf(&vol->key, "%s/%s", + pool->def->source.name, + vol->name) == -1) + return -1; + + return 0; +} + + +static int +virStorageBackendRBDBuildVol(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + unsigned int flags) { virStorageBackendRBDState ptr; ptr.cluster = NULL; @@ -449,9 +475,10 @@ static int virStorageBackendRBDCreateVol(virConnectPtr conn, pool->def->source.name, vol->name, vol->capacity); - if (virStorageBackendRBDOpenRADOSConn(&ptr, conn, pool) < 0) { + virCheckFlags(0, -1); + + if (virStorageBackendRBDOpenRADOSConn(&ptr, conn, pool) < 0) goto cleanup; - } if (rados_ioctx_create(ptr.cluster, pool->def->source.name, &ptr.ioctx) < 0) { @@ -475,9 +502,8 @@ static int virStorageBackendRBDCreateVol(virConnectPtr conn, goto cleanup; } - if (volStorageBackendRBDRefreshVolInfo(vol, pool, &ptr) < 0) { + if (volStorageBackendRBDRefreshVolInfo(vol, pool, &ptr) < 0) goto cleanup; - } ret = 0; @@ -572,6 +598,7 @@ virStorageBackend virStorageBackendRBD = { .refreshPool = virStorageBackendRBDRefreshPool, .createVol = virStorageBackendRBDCreateVol, + .buildVol = virStorageBackendRBDBuildVol, .refreshVol = virStorageBackendRBDRefreshVol, .deleteVol = virStorageBackendRBDDeleteVol, .resizeVol = virStorageBackendRBDResizeVol, -- 1.8.5.2
On 01/06/2014 09:44 AM, Peter Krempa wrote:
Separate the steps to create libvirt's volume metadata from the actual volume building process. --- src/storage/storage_backend_rbd.c | 41 ++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-)
Same as 4/8 on whether to apply now. ACK to the code. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Separate the steps to create libvirt's volume metadata from the actual volume building process. --- src/storage/storage_backend_sheepdog.c | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index a97af1b..a6981ce 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -154,15 +154,37 @@ virStorageBackendSheepdogCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { - - int ret = -1; - if (vol->target.encryption != NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Sheepdog does not support encrypted volumes")); return -1; } + vol->type = VIR_STORAGE_VOL_NETWORK; + + VIR_FREE(vol->key); + if (virAsprintf(&vol->key, "%s/%s", + pool->def->source.name, vol->name) == -1) + return -1; + + VIR_FREE(vol->target.path); + if (VIR_STRDUP(vol->target.path, vol->name) < 0) + return -1; + + return 0; +} + + +static int +virStorageBackendSheepdogBuildVol(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + unsigned int flags) +{ + int ret = -1; + + virCheckFlags(0, -1); + virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "create", vol->name, NULL); virCommandAddArgFormat(cmd, "%llu", vol->capacity); virStorageBackendSheepdogAddHostArg(cmd, pool); @@ -301,6 +323,7 @@ virStorageBackend virStorageBackendSheepdog = { .refreshPool = virStorageBackendSheepdogRefreshPool, .createVol = virStorageBackendSheepdogCreateVol, + .buildVol = virStorageBackendSheepdogBuildVol, .refreshVol = virStorageBackendSheepdogRefreshVol, .deleteVol = virStorageBackendSheepdogDeleteVol, .resizeVol = virStorageBackendSheepdogResizeVol, -- 1.8.5.2
On 01/06/2014 09:44 AM, Peter Krempa wrote:
Separate the steps to create libvirt's volume metadata from the actual volume building process. --- src/storage/storage_backend_sheepdog.c | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-)
ACK; same as 4/8 on whether to apply. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Include the name of the storage backend in the error message instead of just the number. --- src/storage/storage_backend.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index b08d646..19fb1f0 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1146,7 +1146,8 @@ virStorageBackendForType(int type) return backends[i]; virReportError(VIR_ERR_INTERNAL_ERROR, - _("missing backend for pool type %d"), type); + _("missing backend for pool type %d (%s)"), + type, NULLSTR(virStoragePoolTypeToString(type))); return NULL; } -- 1.8.5.2
On 01/06/2014 09:44 AM, Peter Krempa wrote:
Include the name of the storage backend in the error message instead of just the number. --- src/storage/storage_backend.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
ACK; this one is okay for 1.2.1. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/09/14 23:46, Eric Blake wrote:
On 01/06/2014 09:44 AM, Peter Krempa wrote:
Include the name of the storage backend in the error message instead of just the number. --- src/storage/storage_backend.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
ACK; this one is okay for 1.2.1.
Pushed; thanks. Peter
participants (4)
-
Eric Blake -
Ján Tomko -
Michael Chapman -
Peter Krempa