[libvirt] [PATCH RFC 00/27] Native external gluster snapshots
This series refactors and fixes stuff needed to support snapshots on gluster-backed vm's. The series is marked as RFC as docs for some of the changes are missing and I'd like to start code review while I'll figure out the docs. Peter Krempa (27): DO NOT APPLY UPSTREAM: Reproducer for disk snapshot crash qemu: snapshot: Avoid libvirtd crash when qemu crashes while snapshotting Revert "DO NOT APPLY UPSTREAM: Reproducer for disk snapshot crash" Revert "storage: fix omitted slash in gluster volume URI" storage: gluster: Properly fix missing slashes in volume paths snapshot: schema: Split out snapshot disk driver definition snapshot: Add support for specifying snapshot disk backing type snapshot: Test snapshot disk type specification storage: Add gluster pool filter and fix virsh pool listing storage: Avoid forward declaration of virStorageVolDelete 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: Introduce internal pool support storage: Add new argument for createVol backend API storage: gluster: Introduce dummy functions for creating a volume storage: Improve error message when a storage backend is missing maint: Fix messy include of libvirt_internal.h storage: Add internal API to create temporary storage pools and vols storage: Implement ephemeral storage APIs for local disk volumes storage: gluster: Support conversion of gluster volumes to temp volumes qemu: snapshot: Switch snapshot file deletion to the new storage API qemu: snapshot: Add support for external active snapshots on gluster docs/hvsupport.pl | 3 + docs/schemas/domainsnapshot.rng | 94 +++- include/libvirt/libvirt.h.in | 1 + src/Makefile.am | 3 +- src/check-aclrules.pl | 3 + src/check-drivername.pl | 1 + src/conf/snapshot_conf.c | 25 +- src/conf/snapshot_conf.h | 15 +- src/conf/storage_conf.c | 7 +- src/conf/storage_conf.h | 1 + src/driver.h | 14 + src/internal.h | 2 - src/libvirt_internal.c | 71 +++ src/libvirt_internal.h | 22 + src/libvirt_private.syms | 3 + src/libxl/libxl_conf.h | 1 + src/lxc/lxc_conf.h | 1 + src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 9 + src/qemu/qemu_driver.c | 274 +++++++++-- src/storage/storage_backend.c | 3 +- src/storage/storage_backend.h | 2 +- src/storage/storage_backend_disk.c | 47 +- src/storage/storage_backend_fs.c | 36 +- src/storage/storage_backend_gluster.c | 139 +++++- src/storage/storage_backend_logical.c | 129 ++--- src/storage/storage_backend_rbd.c | 42 +- src/storage/storage_backend_sheepdog.c | 32 +- src/storage/storage_driver.c | 597 +++++++++++++++++++---- src/uml/uml_conf.h | 1 + tests/domainsnapshotxml2xmlin/disk_snapshot.xml | 18 + tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 18 + tools/virsh-pool.c | 9 +- 33 files changed, 1341 insertions(+), 284 deletions(-) create mode 100644 src/libvirt_internal.c -- 1.8.5.1
Use the following xml to create a disk-only snapshot of a VM with this patch applied to crash libvirtd: <domainsnapshot> <disks> <disk name='vda' type='file'> <driver type='qcow2'/> <source file='/tmp/path.img'/> </disk> </disks> </domainsnapshot> --- src/qemu/qemu_driver.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 45d11cd..9ebfecf 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12225,6 +12225,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, goto cleanup; } + kill(vm->pid, 9); + sleep(1); + /* create the actual snapshot */ if (snap->format) formatStr = virStorageFileFormatTypeToString(snap->format); -- 1.8.5.1
We shouldn't access the domain definition while we are in the monitor section as the domain is unlocked. Additionaly after we exit from the monitor we need to check if the VM is still alive. Not doing so resulted into crash if qemu exits while attempting to do a external VM snapshot. --- src/qemu/qemu_driver.c | 46 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9ebfecf..ab20dfb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12176,7 +12176,8 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, virDomainDiskDefPtr persistDisk, virJSONValuePtr actions, - bool reuse) + bool reuse, + enum qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivatePtr priv = vm->privateData; char *device = NULL; @@ -12231,8 +12232,24 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, /* create the actual snapshot */ if (snap->format) formatStr = virStorageFileFormatTypeToString(snap->format); + + /* The monitor is only accessed if qemu doesn't support transactions. + * Otherwise the following monitor command only constructs the command. + */ + if (!actions && + qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + goto cleanup; ret = qemuMonitorDiskSnapshot(priv->mon, actions, device, source, formatStr, reuse); + if (!actions) { + qemuDomainObjExitMonitor(driver, vm); + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Domain crashed while taking the snapshot")); + goto cleanup; + } + } + virDomainAuditDisk(vm, disk->src, source, "snapshot", ret >= 0); if (ret < 0) goto cleanup; @@ -12338,9 +12355,6 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, * Based on earlier qemuDomainSnapshotPrepare, all * disks in this list are now either SNAPSHOT_NO, or * SNAPSHOT_EXTERNAL with a valid file name and qcow2 format. */ - if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) - goto cleanup; - for (i = 0; i < snap->def->ndisks; i++) { virDomainDiskDefPtr persistDisk = NULL; @@ -12350,23 +12364,31 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, int indx = virDomainDiskIndexByName(vm->newDef, vm->def->disks[i]->dst, false); - if (indx >= 0) { + if (indx >= 0) persistDisk = vm->newDef->disks[indx]; - persist = true; - } } ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm, &snap->def->disks[i], vm->def->disks[i], persistDisk, actions, - reuse); + reuse, asyncJob); if (ret < 0) break; } if (actions) { - if (ret == 0) + if (ret == 0) { + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + goto cleanup; + ret = qemuMonitorTransaction(priv->mon, actions); + qemuDomainObjExitMonitor(driver, vm); + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("domain crashed while taking the snapshot")); + goto cleanup; + } + } virJSONValueFree(actions); if (ret < 0) { /* Transaction failed; undo the changes to vm. */ @@ -12381,8 +12403,11 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, int indx = virDomainDiskIndexByName(vm->newDef, vm->def->disks[i]->dst, false); - if (indx >= 0) + if (indx >= 0) { persistDisk = vm->newDef->disks[indx]; + persist = true; + } + } qemuDomainSnapshotUndoSingleDiskActive(driver, vm, @@ -12393,7 +12418,6 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, } } } - qemuDomainObjExitMonitor(driver, vm); cleanup: -- 1.8.5.1
On 12/16/2013 09:32 AM, Peter Krempa wrote:
We shouldn't access the domain definition while we are in the monitor section as the domain is unlocked. Additionaly after we exit from the
s/Additionaly/Additionally/
monitor we need to check if the VM is still alive. Not doing so resulted into crash if qemu exits while attempting to do a external VM snapshot.
s/into/in a/ s/a external/an external/
--- src/qemu/qemu_driver.c | 46 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 11 deletions(-)
ACK. This one can be applied in spite of the RFC. And patch 1 was indeed helpful in testing this. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 12/18/2013 08:42 PM, Eric Blake wrote:
On 12/16/2013 09:32 AM, Peter Krempa wrote:
We shouldn't access the domain definition while we are in the monitor section as the domain is unlocked. Additionaly after we exit from the
s/Additionaly/Additionally/
monitor we need to check if the VM is still alive. Not doing so resulted into crash if qemu exits while attempting to do a external VM snapshot.
s/into/in a/ s/a external/an external/
--- src/qemu/qemu_driver.c | 46 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 11 deletions(-)
ACK. This one can be applied in spite of the RFC. And patch 1 was indeed helpful in testing this.
Spoke too soon. After trying to redefine and restart my domain, I got the dreaded state change lock error, which means there's still a locking discrepancy in your patch. 2013-12-19 04:09:14.351+0000: 10084: error : qemuMonitorIO:656 : internal error: End of file from monitor 2013-12-19 04:10:44.000+0000: 10100: warning : qemuDomainObjBeginJobInternal:1093 : Cannot start job (modify, none) for domain f18-live; current job is (none, snapshot) owned by (0, 10099) 2013-12-19 04:10:44.000+0000: 10100: error : qemuDomainObjBeginJobInternal:1097 : Timed out during operation: cannot acquire state change lock It's too late for me to try and find it tonight, but I'll try again in the morning if you haven't found it first. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 12/16/2013 09:32 AM, Peter Krempa wrote:
We shouldn't access the domain definition while we are in the monitor section as the domain is unlocked. Additionaly after we exit from the monitor we need to check if the VM is still alive. Not doing so resulted into crash if qemu exits while attempting to do a external VM snapshot. ---
It would also be worth including your 1/27 patch body in the commit message of this patch (since 1/27 isn't being committed upstream, but at least the git log should make it easy to find out how to reliably trigger the problem). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
This reverts commit 96313c60b91197578b313717db154b6bac029c9e. --- src/qemu/qemu_driver.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ab20dfb..2140e1b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12226,9 +12226,6 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, goto cleanup; } - kill(vm->pid, 9); - sleep(1); - /* create the actual snapshot */ if (snap->format) formatStr = virStorageFileFormatTypeToString(snap->format); -- 1.8.5.1
On 12/16/2013 09:32 AM, Peter Krempa wrote:
This reverts commit 96313c60b91197578b313717db154b6bac029c9e.
Also not to be applied upstream :)
--- src/qemu/qemu_driver.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ab20dfb..2140e1b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12226,9 +12226,6 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, goto cleanup; }
- kill(vm->pid, 9); - sleep(1); - /* create the actual snapshot */ if (snap->format) formatStr = virStorageFileFormatTypeToString(snap->format);
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The fix for the missing slash at the beginning of the volume URI prints to a bad variable causing a memory leak and another one if the variable will be fixed. Revert the commit instead as we can just add a slash to the "key" attribute. The fix also caused a regression, where the path of the volume didn't include the actual volume name and thus was invalid. This reverts commit 6cd60b687acd04ea1538690b7d80f2809e0e29d4. --- src/storage/storage_backend_gluster.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 622526b..1be9034 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -227,10 +227,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, goto cleanup; tmp = state->uri->path; - if (virAsprintf(&vol->key, "%s%s", state->uri->path, name) < 0) { - state->uri->path = tmp; - goto cleanup; - } + state->uri->path = vol->key; if (!(vol->target.path = virURIFormat(state->uri))) { state->uri->path = tmp; goto cleanup; -- 1.8.5.1
On 12/16/2013 09:32 AM, Peter Krempa wrote:
The fix for the missing slash at the beginning of the volume URI prints to a bad variable causing a memory leak and another one if the variable will be fixed. Revert the commit instead as we can just add a slash to the "key" attribute.
The fix also caused a regression, where the path of the volume didn't include the actual volume name and thus was invalid.
This reverts commit 6cd60b687acd04ea1538690b7d80f2809e0e29d4. --- src/storage/storage_backend_gluster.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 622526b..1be9034 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -227,10 +227,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, goto cleanup;
tmp = state->uri->path; - if (virAsprintf(&vol->key, "%s%s", state->uri->path, name) < 0) {
Uggh. I see what I did: I swapped vol->key and state->uri->path, and didn't run my patch through valgrind (where I would have seen the problem).
- state->uri->path = tmp; - goto cleanup; - } + state->uri->path = vol->key;
Rather than reverting this patch, I'd prefer to fix the string creation to use the intended order: diff --git i/src/storage/storage_backend_gluster.c w/src/storage/storage_backend_gluster.c index 622526b..aab70ba 100644 --- i/src/storage/storage_backend_gluster.c +++ w/src/storage/storage_backend_gluster.c @@ -227,7 +227,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, goto cleanup; tmp = state->uri->path; - if (virAsprintf(&vol->key, "%s%s", state->uri->path, name) < 0) { + if (virAsprintf(&state->uri->path, "%s%s", vol->key, name) < 0) { state->uri->path = tmp; goto cleanup; } -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 12/18/2013 09:30 PM, Eric Blake wrote:
Rather than reverting this patch, I'd prefer to fix the string creation to use the intended order:
diff --git i/src/storage/storage_backend_gluster.c w/src/storage/storage_backend_gluster.c index 622526b..aab70ba 100644 --- i/src/storage/storage_backend_gluster.c +++ w/src/storage/storage_backend_gluster.c @@ -227,7 +227,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, goto cleanup;
tmp = state->uri->path; - if (virAsprintf(&vol->key, "%s%s", state->uri->path, name) < 0) { + if (virAsprintf(&state->uri->path, "%s%s", vol->key, name) < 0) {
Except that's not right, either. I'll post what I actually tested in a separate thread. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Add a slash to the volume key so that the URI that is generated from it will be formated properly. The extra slash doesn't hurt in the 'key' attribute as we just need it to be unique. --- src/storage/storage_backend_gluster.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 1be9034..36e99e9 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -222,7 +222,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, if (VIR_STRDUP(vol->name, name) < 0) goto cleanup; - if (virAsprintf(&vol->key, "%s%s%s", state->volname, state->dir, + if (virAsprintf(&vol->key, "/%s%s%s", state->volname, state->dir, vol->name) < 0) goto cleanup; -- 1.8.5.1
On 12/16/2013 09:32 AM, Peter Krempa wrote:
Add a slash to the volume key so that the URI that is generated from it will be formated properly. The extra slash doesn't hurt in the 'key' attribute as we just need it to be unique. --- src/storage/storage_backend_gluster.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 1be9034..36e99e9 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -222,7 +222,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
if (VIR_STRDUP(vol->name, name) < 0) goto cleanup; - if (virAsprintf(&vol->key, "%s%s%s", state->volname, state->dir, + if (virAsprintf(&vol->key, "/%s%s%s", state->volname, state->dir,
This does not match the documentation. I'd rather fix the problem with the URI generation (in patch 4/27) and leave the key unchanged, if only because we already released 1.2.0 with documentation that the key does not have a leading slash. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Extract the definition to a new type to allow avoiding of duplication. --- docs/schemas/domainsnapshot.rng | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng index 7b46df1..169fcfb 100644 --- a/docs/schemas/domainsnapshot.rng +++ b/docs/schemas/domainsnapshot.rng @@ -124,16 +124,7 @@ </attribute> </optional> <interleave> - <optional> - <element name='driver'> - <optional> - <attribute name='type'> - <ref name='storageFormat'/> - </attribute> - </optional> - <empty/> - </element> - </optional> + <ref name='disksnapshotdriver'/> <optional> <element name='source'> <optional> @@ -150,4 +141,17 @@ </element> </define> + <define name='disksnapshotdriver'> + <optional> + <element name='driver'> + <optional> + <attribute name='type'> + <ref name='storageFormat'/> + </attribute> + </optional> + <empty/> + </element> + </optional> + </define> + </grammar> -- 1.8.5.1
On 12/16/2013 09:32 AM, Peter Krempa wrote:
Extract the definition to a new type to allow avoiding of duplication. --- docs/schemas/domainsnapshot.rng | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Add support for specifying various types when doing snapshots. This will later allow to do snapshots on network backed volumes. Disks of type 'volume' are not supported by snapshots (yet). --- docs/schemas/domainsnapshot.rng | 76 +++++++++++++++++++++++++++++++++++------ src/conf/snapshot_conf.c | 25 +++++++++++--- src/conf/snapshot_conf.h | 15 ++++---- src/qemu/qemu_driver.c | 59 +++++++++++++++++++++----------- 4 files changed, 134 insertions(+), 41 deletions(-) diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng index 169fcfb..de9e788 100644 --- a/docs/schemas/domainsnapshot.rng +++ b/docs/schemas/domainsnapshot.rng @@ -123,19 +123,73 @@ <value>external</value> </attribute> </optional> - <interleave> - <ref name='disksnapshotdriver'/> - <optional> - <element name='source'> + <choice> + <group> + <optional> + <attribute name='type'> + <value>file</value> + </attribute> + </optional> + <interleave> <optional> - <attribute name='file'> - <ref name='absFilePath'/> - </attribute> + <element name='source'> + <optional> + <attribute name='file'> + <ref name='absFilePath'/> + </attribute> + </optional> + <empty/> + </element> </optional> - <empty/> - </element> - </optional> - </interleave> + <ref name='disksnapshotdriver'/> + </interleave> + </group> + <group> + <attribute name='type'> + <value>block</value> + </attribute> + <interleave> + <optional> + <element name="source"> + <attribute name="dev"> + <ref name="absFilePath"/> + </attribute> + <empty/> + </element> + </optional> + <ref name='disksnapshotdriver'/> + </interleave> + </group> + <group> + <attribute name="type"> + <value>dir</value> + </attribute> + <interleave> + <optional> + <element name="source"> + <attribute name="dir"> + <ref name="absFilePath"/> + </attribute> + <empty/> + </element> + </optional> + <ref name='disksnapshotdriver'/> + </interleave> + </group> + <group> + <attribute name="type"> + <value>network</value> + </attribute> + <interleave> + <optional> + <element name="source"> + <ref name='diskSourceNetwork'/> + </element> + </optional> + <ref name='disksnapshotdriver'/> + </interleave> + </group> + </choice> </group> </choice> </element> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index c18b99b..6ce65f6 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -108,6 +108,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, { int ret = -1; char *snapshot = NULL; + char *type = NULL; xmlNodePtr cur; def->name = virXMLPropString(node, "name"); @@ -129,6 +130,14 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, } def->type = -1; + if ((type = virXMLPropString(node, "type"))) { + if ((def->type = virDomainDiskTypeFromString(type)) < 0 || + def->type == VIR_DOMAIN_DISK_TYPE_VOLUME) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown disk snapshot type '%s'"), type); + goto cleanup; + } + } for (cur = node->children; cur; cur = cur->next) { if (cur->type != XML_ELEMENT_NODE) @@ -145,9 +154,9 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, if (virDomainDiskSourceDefParse(cur, backingtype, &def->file, - NULL, - NULL, - NULL, + &def->protocol, + &def->nhosts, + &def->hosts, NULL) < 0) goto cleanup; @@ -174,6 +183,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, ret = 0; cleanup: VIR_FREE(snapshot); + VIR_FREE(type); if (ret < 0) virDomainSnapshotDiskDefClear(def); return ret; @@ -616,6 +626,9 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, if (type < 0) type = VIR_DOMAIN_DISK_TYPE_FILE; + else + virBufferAsprintf(buf, " type='%s'", + virDomainDiskTypeToString(type)); if (!disk->file && disk->format == 0) { virBufferAddLit(buf, "/>\n"); @@ -630,7 +643,11 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, virDomainDiskSourceDefFormatInternal(buf, type, disk->file, - 0, 0, 0, NULL, 0, NULL, NULL, 0); + 0, + disk->protocol, + disk->nhosts, + disk->hosts, + 0, NULL, NULL, 0); virBufferAddLit(buf, " </disk>\n"); } diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index 241d63c..bcd92dc 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -48,12 +48,15 @@ enum virDomainSnapshotState { typedef struct _virDomainSnapshotDiskDef virDomainSnapshotDiskDef; typedef virDomainSnapshotDiskDef *virDomainSnapshotDiskDefPtr; struct _virDomainSnapshotDiskDef { - char *name; /* name matching the <target dev='...' of the domain */ - int index; /* index within snapshot->dom->disks that matches name */ - int snapshot; /* enum virDomainSnapshotLocation */ - int type; /* enum virDomainDiskType */ - char *file; /* new source file when snapshot is external */ - int format; /* enum virStorageFileFormat */ + char *name; /* name matching the <target dev='...' of the domain */ + int index; /* index within snapshot->dom->disks that matches name */ + int snapshot; /* enum virDomainSnapshotLocation */ + int type; /* enum virDomainDiskType */ + char *file; /* new source file when snapshot is external */ + int format; /* enum virStorageFileFormat */ + int protocol; /* network source protocol */ + size_t nhosts; /* network source hosts count */ + virDomainDiskHostDefPtr hosts; /* network source hosts */ }; /* Stores the complete snapshot metadata */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2140e1b..304c7ee 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12196,33 +12196,48 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, } if (virAsprintf(&device, "drive-%s", disk->info.alias) < 0 || - VIR_STRDUP(source, snap->file) < 0 || (persistDisk && VIR_STRDUP(persistSource, source) < 0)) goto cleanup; - /* create the stub file and set selinux labels; manipulate disk in - * place, in a way that can be reverted on failure. */ - if (!reuse) { - fd = qemuOpenFile(driver, vm, source, O_WRONLY | O_TRUNC | O_CREAT, - &need_unlink, NULL); - if (fd < 0) - goto cleanup; - VIR_FORCE_CLOSE(fd); - } - /* XXX Here, we know we are about to alter disk->backingChain if - * successful, so we nuke the existing chain so that future - * commands will recompute it. Better would be storing the chain - * ourselves rather than reprobing, but this requires modifying - * domain_conf and our XML to fully track the chain across - * libvirtd restarts. */ + * successful, so we nuke the existing chain so that future commands will + * recompute it. Better would be storing the chain ourselves rather than + * reprobing, but this requires modifying domain_conf and our XML to fully + * track the chain across libvirtd restarts. */ virStorageFileFreeMetadata(disk->backingChain); disk->backingChain = NULL; - if (qemuDomainPrepareDiskChainElement(driver, vm, disk, source, - VIR_DISK_CHAIN_READ_WRITE) < 0) { - qemuDomainPrepareDiskChainElement(driver, vm, disk, source, - VIR_DISK_CHAIN_NO_ACCESS); + switch (snap->type) { + case VIR_DOMAIN_DISK_TYPE_BLOCK: + reuse = true; + /* fallthrough */ + case -1: /* type was not provided in snapshot conf */ + case VIR_DOMAIN_DISK_TYPE_FILE: + if (VIR_STRDUP(source, snap->file) < 0) + goto cleanup; + + /* create the stub file and set selinux labels; manipulate disk in + * place, in a way that can be reverted on failure. */ + if (!reuse) { + fd = qemuOpenFile(driver, vm, source, O_WRONLY | O_TRUNC | O_CREAT, + &need_unlink, NULL); + if (fd < 0) + goto cleanup; + VIR_FORCE_CLOSE(fd); + } + + if (qemuDomainPrepareDiskChainElement(driver, vm, disk, source, + VIR_DISK_CHAIN_READ_WRITE) < 0) { + qemuDomainPrepareDiskChainElement(driver, vm, disk, source, + VIR_DISK_CHAIN_NO_ACCESS); + goto cleanup; + } + break; + + default: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("snapshots are not supported on '%s' volumes"), + virDomainDiskTypeToString(snap->type)); goto cleanup; } @@ -12257,11 +12272,13 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, disk->src = source; source = NULL; disk->format = format; + disk->type = snap->type; if (persistDisk) { VIR_FREE(persistDisk->src); persistDisk->src = persistSource; persistSource = NULL; persistDisk->format = format; + persistDisk->type = snap->type; } cleanup: @@ -12303,11 +12320,13 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, disk->src = source; source = NULL; disk->format = origdisk->format; + disk->type = origdisk->type; if (persistDisk) { VIR_FREE(persistDisk->src); persistDisk->src = persistSource; persistSource = NULL; persistDisk->format = origdisk->format; + persistDisk->type = origdisk->type; } cleanup: -- 1.8.5.1
Amend tests to check parsing of the various new disk types that can now be specified. --- tests/domainsnapshotxml2xmlin/disk_snapshot.xml | 18 ++++++++++++++++++ tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/tests/domainsnapshotxml2xmlin/disk_snapshot.xml b/tests/domainsnapshotxml2xmlin/disk_snapshot.xml index ee6b46a..aa1522a 100644 --- a/tests/domainsnapshotxml2xmlin/disk_snapshot.xml +++ b/tests/domainsnapshotxml2xmlin/disk_snapshot.xml @@ -12,5 +12,23 @@ <disk name='hde' snapshot='external'> <source file='/path/to/new'/> </disk> + <disk name='hde' snapshot='external' type='file'> + <source file='/path/to/new2'/> + </disk> + <disk name='hdf' snapshot='external' type='block'> + <source dev='/path/to/new3'/> + </disk> + <disk name='hdg' snapshot='external' type='network'> + <source protocol='gluster' name='volume/path'> + <host name='host' port='1234'/> + </source> + </disk> + <disk name='hdh' snapshot='external' type='network'> + <source protocol='rbd' name='name'> + <host name='host' port='1234'/> + <host name='host2' port='1234' transport='rdma'/> + <host name='host3' port='1234'/> + </source> + </disk> </disks> </domainsnapshot> diff --git a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml index 1a1fc02..316df43 100644 --- a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml +++ b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml @@ -11,5 +11,23 @@ <disk name='hde' snapshot='external'> <source file='/path/to/new'/> </disk> + <disk name='hde' snapshot='external' type='file'> + <source file='/path/to/new2'/> + </disk> + <disk name='hdf' snapshot='external' type='block'> + <source dev='/path/to/new3'/> + </disk> + <disk name='hdg' snapshot='external' type='network'> + <source protocol='gluster' name='volume/path'> + <host name='host' port='1234'/> + </source> + </disk> + <disk name='hdh' snapshot='external' type='network'> + <source protocol='rbd' name='name'> + <host name='host' port='1234'/> + <host name='host2' port='1234' transport='rdma'/> + <host name='host3' port='1234'/> + </source> + </disk> </disks> </domainsnapshot> -- 1.8.5.1
Recent adition of the gluster pool type omitted fixing the virsh and virConnectListAllStoragePool filters. A typecast of the converting function in virsh showed that also the sheepdog pool was omitted in the command parser. This patch adds gluster pool filtering support and fixes virsh to properly convert all supported storage pool types. The added typecast should avoid doing such mistakes in the future. --- include/libvirt/libvirt.h.in | 1 + src/conf/storage_conf.c | 4 +++- tools/virsh-pool.c | 9 +++++++-- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 6f79c49..aa042b4 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3056,6 +3056,7 @@ typedef enum { VIR_CONNECT_LIST_STORAGE_POOLS_MPATH = 1 << 13, VIR_CONNECT_LIST_STORAGE_POOLS_RBD = 1 << 14, VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG = 1 << 15, + VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER = 1 << 16, } virConnectListAllStoragePoolsFlags; int virConnectListAllStoragePools(virConnectPtr conn, diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 22e38c1..ed492cf 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2217,7 +2217,9 @@ virStoragePoolMatch(virStoragePoolObjPtr poolobj, (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_RBD) && (poolobj->def->type == VIR_STORAGE_POOL_RBD)) || (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG) && - (poolobj->def->type == VIR_STORAGE_POOL_SHEEPDOG)))) + (poolobj->def->type == VIR_STORAGE_POOL_SHEEPDOG)) || + (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER) && + (poolobj->def->type == VIR_STORAGE_POOL_GLUSTER)))) return false; } diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 18fe242..b6753c4 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -1006,7 +1006,7 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) return false; } - switch (poolType) { + switch ((enum virStoragePoolType) poolType) { case VIR_STORAGE_POOL_DIR: flags |= VIR_CONNECT_LIST_STORAGE_POOLS_DIR; break; @@ -1034,7 +1034,12 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) case VIR_STORAGE_POOL_RBD: flags |= VIR_CONNECT_LIST_STORAGE_POOLS_RBD; break; - default: + case VIR_STORAGE_POOL_SHEEPDOG: + flags |= VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG; + break; + case VIR_STORAGE_POOL_GLUSTER: + flags |= VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER; + case VIR_STORAGE_POOL_LAST: break; } } -- 1.8.5.1
On 12/16/2013 09:32 AM, Peter Krempa wrote:
Recent adition of the gluster pool type omitted fixing the virsh and
s/adition/addition/
virConnectListAllStoragePool filters. A typecast of the converting function in virsh showed that also the sheepdog pool was omitted in the command parser.
This patch adds gluster pool filtering support and fixes virsh to properly convert all supported storage pool types. The added typecast should avoid doing such mistakes in the future. --- include/libvirt/libvirt.h.in | 1 + src/conf/storage_conf.c | 4 +++- tools/virsh-pool.c | 9 +++++++-- 3 files changed, 11 insertions(+), 3 deletions(-)
Missing a doc change in virsh.pod, to mention that 'gluster' is also a valid type. Code is fine, so ACK if you fix the missing docs. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 12/19/13 05:54, Eric Blake wrote:
On 12/16/2013 09:32 AM, Peter Krempa wrote:
Recent adition of the gluster pool type omitted fixing the virsh and
s/adition/addition/
virConnectListAllStoragePool filters. A typecast of the converting function in virsh showed that also the sheepdog pool was omitted in the command parser.
This patch adds gluster pool filtering support and fixes virsh to properly convert all supported storage pool types. The added typecast should avoid doing such mistakes in the future. --- include/libvirt/libvirt.h.in | 1 + src/conf/storage_conf.c | 4 +++- tools/virsh-pool.c | 9 +++++++-- 3 files changed, 11 insertions(+), 3 deletions(-)
Missing a doc change in virsh.pod, to mention that 'gluster' is also a valid type.
Code is fine, so ACK if you fix the missing docs.
I've fixed the man page and pushed the patch. Thanks. Peter
Move the code around so that the forward declaration isn't needed. --- src/storage/storage_driver.c | 181 ++++++++++++++++++++++--------------------- 1 file changed, 91 insertions(+), 90 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index f08255e..8b1dcae 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1495,7 +1495,97 @@ cleanup: return ret; } -static int storageVolDelete(virStorageVolPtr obj, unsigned int flags); + +static int +storageVolDelete(virStorageVolPtr obj, + unsigned int flags) { + virStorageDriverStatePtr driver = obj->conn->storagePrivateData; + virStoragePoolObjPtr pool; + virStorageBackendPtr backend; + virStorageVolDefPtr vol = NULL; + size_t i; + int ret = -1; + + storageDriverLock(driver); + pool = virStoragePoolObjFindByName(&driver->pools, obj->pool); + storageDriverUnlock(driver); + + if (!pool) { + virReportError(VIR_ERR_NO_STORAGE_POOL, + _("no storage pool with matching name '%s'"), + obj->pool); + goto cleanup; + } + + if (!virStoragePoolObjIsActive(pool)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("storage pool '%s' is not active"), pool->def->name); + goto cleanup; + } + + if ((backend = virStorageBackendForType(pool->def->type)) == NULL) + goto cleanup; + + vol = virStorageVolDefFindByName(pool, obj->name); + + if (!vol) { + virReportError(VIR_ERR_NO_STORAGE_VOL, + _("no storage vol with matching name '%s'"), + obj->name); + goto cleanup; + } + + if (virStorageVolDeleteEnsureACL(obj->conn, pool->def, vol) < 0) + goto cleanup; + + if (vol->building) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("volume '%s' is still being allocated."), + vol->name); + goto cleanup; + } + + if (!backend->deleteVol) { + virReportError(VIR_ERR_NO_SUPPORT, + "%s", _("storage pool does not support vol deletion")); + + goto cleanup; + } + + if (backend->deleteVol(obj->conn, pool, vol, flags) < 0) + goto cleanup; + + /* Update pool metadata */ + pool->def->allocation -= vol->allocation; + pool->def->available += vol->allocation; + + for (i = 0; i < pool->volumes.count; i++) { + if (pool->volumes.objs[i] == vol) { + VIR_INFO("Deleting volume '%s' from storage pool '%s'", + vol->name, pool->def->name); + virStorageVolDefFree(vol); + vol = NULL; + + if (i < (pool->volumes.count - 1)) + memmove(pool->volumes.objs + i, pool->volumes.objs + i + 1, + sizeof(*(pool->volumes.objs)) * (pool->volumes.count - (i + 1))); + + if (VIR_REALLOC_N(pool->volumes.objs, pool->volumes.count - 1) < 0) { + ; /* Failure to reduce memory allocation isn't fatal */ + } + pool->volumes.count--; + + break; + } + } + ret = 0; + +cleanup: + if (pool) + virStoragePoolObjUnlock(pool); + return ret; +} + static virStorageVolPtr storageVolCreateXML(virStoragePoolPtr obj, @@ -2318,95 +2408,6 @@ storageVolWipe(virStorageVolPtr obj, return storageVolWipePattern(obj, VIR_STORAGE_VOL_WIPE_ALG_ZERO, flags); } -static int -storageVolDelete(virStorageVolPtr obj, - unsigned int flags) { - virStorageDriverStatePtr driver = obj->conn->storagePrivateData; - virStoragePoolObjPtr pool; - virStorageBackendPtr backend; - virStorageVolDefPtr vol = NULL; - size_t i; - int ret = -1; - - storageDriverLock(driver); - pool = virStoragePoolObjFindByName(&driver->pools, obj->pool); - storageDriverUnlock(driver); - - if (!pool) { - virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching name '%s'"), - obj->pool); - goto cleanup; - } - - if (!virStoragePoolObjIsActive(pool)) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("storage pool '%s' is not active"), pool->def->name); - goto cleanup; - } - - if ((backend = virStorageBackendForType(pool->def->type)) == NULL) - goto cleanup; - - vol = virStorageVolDefFindByName(pool, obj->name); - - if (!vol) { - virReportError(VIR_ERR_NO_STORAGE_VOL, - _("no storage vol with matching name '%s'"), - obj->name); - goto cleanup; - } - - if (virStorageVolDeleteEnsureACL(obj->conn, pool->def, vol) < 0) - goto cleanup; - - if (vol->building) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("volume '%s' is still being allocated."), - vol->name); - goto cleanup; - } - - if (!backend->deleteVol) { - virReportError(VIR_ERR_NO_SUPPORT, - "%s", _("storage pool does not support vol deletion")); - - goto cleanup; - } - - if (backend->deleteVol(obj->conn, pool, vol, flags) < 0) - goto cleanup; - - /* Update pool metadata */ - pool->def->allocation -= vol->allocation; - pool->def->available += vol->allocation; - - for (i = 0; i < pool->volumes.count; i++) { - if (pool->volumes.objs[i] == vol) { - VIR_INFO("Deleting volume '%s' from storage pool '%s'", - vol->name, pool->def->name); - virStorageVolDefFree(vol); - vol = NULL; - - if (i < (pool->volumes.count - 1)) - memmove(pool->volumes.objs + i, pool->volumes.objs + i + 1, - sizeof(*(pool->volumes.objs)) * (pool->volumes.count - (i + 1))); - - if (VIR_REALLOC_N(pool->volumes.objs, pool->volumes.count - 1) < 0) { - ; /* Failure to reduce memory allocation isn't fatal */ - } - pool->volumes.count--; - - break; - } - } - ret = 0; - -cleanup: - if (pool) - virStoragePoolObjUnlock(pool); - return ret; -} static int storageVolGetInfo(virStorageVolPtr obj, -- 1.8.5.1
On 12/16/2013 09:32 AM, Peter Krempa wrote:
Move the code around so that the forward declaration isn't needed. --- src/storage/storage_driver.c | 181 ++++++++++++++++++++++--------------------- 1 file changed, 91 insertions(+), 90 deletions(-)
ACK; mechanical.
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index f08255e..8b1dcae 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1495,7 +1495,97 @@ cleanup: return ret; }
-static int storageVolDelete(virStorageVolPtr obj, unsigned int flags); + +static int +storageVolDelete(virStorageVolPtr obj, + unsigned int flags) {
This { should be on its own line; but you were doing straight code motion so it's okay if you don't clean it up until a later patch. (Or you could clean it up here, as long as the commit message mentions that you did minor tweaking to fix style issues as part of the code motion) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 12/19/13 05:56, Eric Blake wrote:
On 12/16/2013 09:32 AM, Peter Krempa wrote:
Move the code around so that the forward declaration isn't needed. --- src/storage/storage_driver.c | 181 ++++++++++++++++++++++--------------------- 1 file changed, 91 insertions(+), 90 deletions(-)
ACK; mechanical.
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index f08255e..8b1dcae 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1495,7 +1495,97 @@ cleanup: return ret; }
-static int storageVolDelete(virStorageVolPtr obj, unsigned int flags); + +static int +storageVolDelete(virStorageVolPtr obj, + unsigned int flags) {
This { should be on its own line; but you were doing straight code motion so it's okay if you don't clean it up until a later patch. (Or you could clean it up here, as long as the commit message mentions that you did minor tweaking to fix style issues as part of the code motion)
I've done as you've advised and pushed this one. Thanks. Peter
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 11cf2df..9188ffa 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.1
--- 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 36e99e9..2ea78c7 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -377,8 +377,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.1
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.1
--- 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.1
--- 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.1
--- 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.1
--- 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.1
To allow using the storage driver APIs to do operation on generic domain disks we will need to introduce internal storage pools that will give is a base to support this stuff even on files that weren't originally defined as a part of the pool. This patch introduces the 'internal' flag for a storage pool that will prevent it from being listed along with the user defined storage pools. --- src/conf/storage_conf.c | 3 +++ src/conf/storage_conf.h | 1 + src/storage/storage_driver.c | 12 ++++++++---- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index ed492cf..379bbd6 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2172,6 +2172,9 @@ static bool virStoragePoolMatch(virStoragePoolObjPtr poolobj, unsigned int flags) { + if (poolobj->internal) + return false; + /* filter by active state */ if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE) && !((MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE) && diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 485bdba..62ac749 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -332,6 +332,7 @@ struct _virStoragePoolObj { int active; int autostart; unsigned int asyncjobs; + bool internal; virStoragePoolDefPtr def; virStoragePoolDefPtr newDef; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 8b1dcae..00f2c50 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -377,7 +377,8 @@ storageConnectNumOfStoragePools(virConnectPtr conn) { virStoragePoolObjPtr obj = driver->pools.objs[i]; virStoragePoolObjLock(obj); if (virConnectNumOfStoragePoolsCheckACL(conn, obj->def) && - virStoragePoolObjIsActive(obj)) + virStoragePoolObjIsActive(obj) && + !obj->internal) nactive++; virStoragePoolObjUnlock(obj); } @@ -402,7 +403,8 @@ storageConnectListStoragePools(virConnectPtr conn, virStoragePoolObjPtr obj = driver->pools.objs[i]; virStoragePoolObjLock(obj); if (virConnectListStoragePoolsCheckACL(conn, obj->def) && - virStoragePoolObjIsActive(obj)) { + virStoragePoolObjIsActive(obj) && + !obj->internal) { if (VIR_STRDUP(names[got], obj->def->name) < 0) { virStoragePoolObjUnlock(obj); goto cleanup; @@ -436,7 +438,8 @@ storageConnectNumOfDefinedStoragePools(virConnectPtr conn) { virStoragePoolObjPtr obj = driver->pools.objs[i]; virStoragePoolObjLock(obj); if (virConnectNumOfDefinedStoragePoolsCheckACL(conn, obj->def) && - !virStoragePoolObjIsActive(obj)) + !virStoragePoolObjIsActive(obj) && + !obj->internal) nactive++; virStoragePoolObjUnlock(obj); } @@ -461,7 +464,8 @@ storageConnectListDefinedStoragePools(virConnectPtr conn, virStoragePoolObjPtr obj = driver->pools.objs[i]; virStoragePoolObjLock(obj); if (virConnectListDefinedStoragePoolsCheckACL(conn, obj->def) && - !virStoragePoolObjIsActive(obj)) { + !virStoragePoolObjIsActive(obj) && + !obj->internal) { if (VIR_STRDUP(names[got], obj->def->name) < 0) { virStoragePoolObjUnlock(obj); goto cleanup; -- 1.8.5.1
--- src/storage/storage_backend.h | 2 +- src/storage/storage_backend_disk.c | 3 ++- src/storage/storage_backend_fs.c | 30 +++++++++++++++++++++++------- src/storage/storage_backend_logical.c | 6 ++++-- src/storage/storage_backend_rbd.c | 3 ++- src/storage/storage_backend_sheepdog.c | 3 ++- src/storage/storage_driver.c | 4 ++-- 7 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 9e07dd8..4d0c057 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -41,7 +41,7 @@ typedef int (*virStorageBackendDeletePool)(virConnectPtr conn, virStoragePoolObj typedef int (*virStorageBackendBuildVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags); -typedef int (*virStorageBackendCreateVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol); +typedef int (*virStorageBackendCreateVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, bool internal); typedef int (*virStorageBackendRefreshVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol); typedef int (*virStorageBackendDeleteVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags); typedef int (*virStorageBackendBuildVolFrom)(virConnectPtr conn, virStoragePoolObjPtr pool, diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index aa3b72f..a77c298 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -618,7 +618,8 @@ virStorageBackendDiskPartBoundries(virStoragePoolObjPtr pool, static int virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, - virStorageVolDefPtr vol) + virStorageVolDefPtr vol, + bool internal ATTRIBUTE_UNUSED) { if (vol->target.encryption != NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 9188ffa..f9f7ea3 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -997,8 +997,10 @@ virStorageBackendFileSystemDelete(virConnectPtr conn ATTRIBUTE_UNUSED, static int virStorageBackendFileSystemVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, - virStorageVolDefPtr vol) + virStorageVolDefPtr vol, + bool internal) { + struct stat st; vol->type = VIR_STORAGE_VOL_FILE; @@ -1008,15 +1010,29 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED, 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; + if (internal) { + if (stat(vol->target.path, &st) == 0) { + if (S_ISDIR(st.st_mode)) + vol->type = VIR_STORAGE_VOL_DIR; + else if (S_ISBLK(st.st_mode)) + vol->type = VIR_STORAGE_VOL_BLOCK; + else + vol->type = VIR_STORAGE_VOL_FILE; + } + } else { + if (virFileExists(vol->target.path)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("volume target path '%s' already exists"), + vol->target.path); + return -1; + } } VIR_FREE(vol->key); - return VIR_STRDUP(vol->key, vol->target.path); + if (VIR_STRDUP(vol->key, vol->target.path) < 0) + return -1; + + return 0; } static int createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED, diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 039d962..2e2560f 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -784,7 +784,8 @@ error: static int virStorageBackendLogicalCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, - virStorageVolDefPtr vol) + virStorageVolDefPtr vol, + bool internal) { if (vol->target.encryption != NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -800,7 +801,8 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, vol->name) == -1) return -1; - if (virFileExists(vol->target.path)) { + if (!internal && + virFileExists(vol->target.path)) { virReportError(VIR_ERR_OPERATION_INVALID, _("volume target path '%s' already exists"), vol->target.path); diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index c5f0bc5..75425f4 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -439,7 +439,8 @@ cleanup: static int virStorageBackendRBDCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, - virStorageVolDefPtr vol) + virStorageVolDefPtr vol, + bool internal ATTRIBUTE_UNUSED) { vol->type = VIR_STORAGE_VOL_NETWORK; diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index a6981ce..705451b 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -152,7 +152,8 @@ virStorageBackendSheepdogDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, static int virStorageBackendSheepdogCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, - virStorageVolDefPtr vol) + virStorageVolDefPtr vol, + bool internal ATTRIBUTE_UNUSED) { if (vol->target.encryption != NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 00f2c50..c92e6c2 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1651,7 +1651,7 @@ storageVolCreateXML(virStoragePoolPtr obj, /* Wipe any key the user may have suggested, as volume creation * will generate the canonical key. */ VIR_FREE(voldef->key); - if (backend->createVol(obj->conn, pool, voldef) < 0) { + if (backend->createVol(obj->conn, pool, voldef, false) < 0) { goto cleanup; } @@ -1830,7 +1830,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, * Wipe any key the user may have suggested, as volume creation * will generate the canonical key. */ VIR_FREE(newvol->key); - if (backend->createVol(obj->conn, pool, newvol) < 0) { + if (backend->createVol(obj->conn, pool, newvol, false) < 0) { goto cleanup; } -- 1.8.5.1
The temporary pool code will need to initialize dummy gluster volumes which needs the createVol function of the storage backend. This patch implements it only for that purpose. When an user will get an error message that it is not implemented on an attempt to create a volume in a gluster pool. --- src/storage/storage_backend_gluster.c | 68 +++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 2ea78c7..8ac2b95 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -439,10 +439,78 @@ cleanup: } +static int +virStorageBackendGlusterVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + bool internal) +{ + virStorageBackendGlusterStatePtr state = NULL; + struct stat st; + char *tmp; + int ret = -1; + + if (!(state = virStorageBackendGlusterOpen(pool))) + goto cleanup; + + vol->type = VIR_STORAGE_VOL_NETWORK; + vol->target.format = VIR_STORAGE_FILE_RAW; + + VIR_FREE(vol->key); + if (virAsprintf(&vol->key, "/%s%s%s", state->volname, state->dir, + vol->name) < 0) + goto cleanup; + + tmp = state->uri->path; + state->uri->path = vol->key; + VIR_FREE(vol->target.path); + if (!(vol->target.path = virURIFormat(state->uri))) { + state->uri->path = tmp; + goto cleanup; + } + state->uri->path = tmp; + + if (internal) { + if (glfs_stat(state->vol, vol->name, &st) == 0 && + S_ISDIR(st.st_mode)) { + vol->type = VIR_STORAGE_VOL_NETDIR; + vol->target.format = VIR_STORAGE_FILE_DIR; + } + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("gluster pool backend doesn't " + "yet support volume creation")); + goto cleanup; + } + + ret = 0; + +cleanup: + virStorageBackendGlusterClose(state); + return ret; +} + + +static int +virStorageBackendGlusterVolBuild(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, + virStorageVolDefPtr vol ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virCheckFlags(0, -1); + + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("gluster pool backend doesn't yet support volume building")); + return -1; +} + + virStorageBackend virStorageBackendGluster = { .type = VIR_STORAGE_POOL_GLUSTER, .refreshPool = virStorageBackendGlusterRefreshPool, + .createVol = virStorageBackendGlusterVolCreate, + .buildVol = virStorageBackendGlusterVolBuild, .deleteVol = virStorageBackendGlusterVolDelete, }; -- 1.8.5.1
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.1
The libvirt_internal.h header was included by the internal.h header. This made it painful to add new stuff to the header file that would require some more specific types. Remove inclusion by internal.h and add it to appropriate places manually. --- src/driver.h | 1 + src/internal.h | 2 -- src/libxl/libxl_conf.h | 1 + src/lxc/lxc_conf.h | 1 + src/uml/uml_conf.h | 1 + 5 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/driver.h b/src/driver.h index b6927ea..5f4cd8d 100644 --- a/src/driver.h +++ b/src/driver.h @@ -25,6 +25,7 @@ # include <unistd.h> # include "internal.h" +# include "libvirt_internal.h" # include "viruri.h" /* * List of registered drivers numbers diff --git a/src/internal.h b/src/internal.h index 5e29694..b7c3519 100644 --- a/src/internal.h +++ b/src/internal.h @@ -60,8 +60,6 @@ # include "libvirt/libvirt-qemu.h" # include "libvirt/virterror.h" -# include "libvirt_internal.h" - # include "c-strcase.h" # include "ignore-value.h" diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index ffa93bd..3ab8f20 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -29,6 +29,7 @@ # include <libxl.h> # include "internal.h" +# include "libvirt_internal.h" # include "domain_conf.h" # include "domain_event.h" # include "capabilities.h" diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h index f6cbfc9..e04dcdd 100644 --- a/src/lxc/lxc_conf.h +++ b/src/lxc/lxc_conf.h @@ -26,6 +26,7 @@ # define LXC_CONF_H # include "internal.h" +# include "libvirt_internal.h" # include "domain_conf.h" # include "domain_event.h" # include "capabilities.h" diff --git a/src/uml/uml_conf.h b/src/uml/uml_conf.h index c23a177..a914be0 100644 --- a/src/uml/uml_conf.h +++ b/src/uml/uml_conf.h @@ -25,6 +25,7 @@ # define __UML_CONF_H # include "internal.h" +# include "libvirt_internal.h" # include "capabilities.h" # include "network_conf.h" # include "domain_conf.h" -- 1.8.5.1
If a VM driver wants to access stuff provided by a storage driver the volume needs to be a part of a storage pool. As this wasn't designed in from the beginning we need a way to convert generic domain disk and snapshot disk definitions into temporary pools and volumes. This patch allows that by adding private storage driver APIs that can be used to obtain a pool and vol definition. --- docs/hvsupport.pl | 3 ++ src/Makefile.am | 3 +- src/check-drivername.pl | 1 + src/driver.h | 13 +++++++++ src/libvirt_internal.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_internal.h | 22 +++++++++++++++ src/libvirt_private.syms | 3 ++ 7 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 src/libvirt_internal.c diff --git a/docs/hvsupport.pl b/docs/hvsupport.pl index f8483f9..a7d1f6b 100755 --- a/docs/hvsupport.pl +++ b/docs/hvsupport.pl @@ -176,6 +176,9 @@ $apis{virDomainMigratePerform3Params} = "1.1.0"; $apis{virDomainMigrateFinish3Params} = "1.1.0"; $apis{virDomainMigrateConfirm3Params} = "1.1.0"; +$apis{virStorageEphemeralFree} = "1.2.1"; +$apis{virStorageEphemeralFromDiskDef} = "1.2.1"; +$apis{virStorageEphemeralFromSnapshotDiskDef} = "1.2.1"; # Now we want to get the mapping between public APIs diff --git a/src/Makefile.am b/src/Makefile.am index 57e163f..c0c535d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -176,7 +176,8 @@ DRIVER_SOURCES = \ $(DATATYPES_SOURCES) \ fdstream.c fdstream.h \ $(NODE_INFO_SOURCES) \ - libvirt.c libvirt_internal.h \ + libvirt.c \ + libvirt_internal.c libvirt_internal.h \ locking/lock_manager.c locking/lock_manager.h \ locking/lock_driver.h \ locking/lock_driver_nop.h locking/lock_driver_nop.c \ diff --git a/src/check-drivername.pl b/src/check-drivername.pl index 5c8de0a..50b3480 100755 --- a/src/check-drivername.pl +++ b/src/check-drivername.pl @@ -50,6 +50,7 @@ while (<DRVFILE>) { next if $drv =~ /virDrvState/; next if $drv =~ /virDrvDomainMigrate(Prepare|Perform|Confirm|Begin|Finish)/; + next if $drv =~ /virDrvStorageEphemeral/; my $sym = $drv; $sym =~ s/virDrv/vir/; diff --git a/src/driver.h b/src/driver.h index 5f4cd8d..b72c5e9 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1754,6 +1754,16 @@ typedef int typedef int (*virDrvStoragePoolIsPersistent)(virStoragePoolPtr pool); +typedef void +(*virDrvStorageEphemeralFree)(virStorageEphemeralPtr def); + +typedef virStorageEphemeralPtr +(*virDrvStorageEphemeralFromDiskDef)(virConnectPtr conn, + virDomainDiskDefPtr def); + +typedef virStorageEphemeralPtr +(*virDrvStorageEphemeralFromSnapshotDiskDef)(virConnectPtr conn, + virDomainSnapshotDiskDefPtr def); typedef struct _virStorageDriver virStorageDriver; @@ -1813,6 +1823,9 @@ struct _virStorageDriver { virDrvStorageVolResize storageVolResize; virDrvStoragePoolIsActive storagePoolIsActive; virDrvStoragePoolIsPersistent storagePoolIsPersistent; + virDrvStorageEphemeralFree storageEphemeralFree; + virDrvStorageEphemeralFromDiskDef storageEphemeralFromDiskDef; + virDrvStorageEphemeralFromSnapshotDiskDef storageEphemeralFromSnapshotDiskDef; }; # ifdef WITH_LIBVIRTD diff --git a/src/libvirt_internal.c b/src/libvirt_internal.c new file mode 100644 index 0000000..ea10885 --- /dev/null +++ b/src/libvirt_internal.c @@ -0,0 +1,71 @@ +/* + * libvirt_internal.c: internally exported APIs, not for public use + * + * Copyright (C) 2013 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> +#include "libvirt_internal.h" + +#include "datatypes.h" +#include "viralloc.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +void +virStorageEphemeralFree(virStorageEphemeralPtr def) +{ + if (!def) + return; + + if (!def->pool) { + VIR_FREE(def); + return; + } + + if (def->pool->conn->storageDriver->storageEphemeralFree) { + def->pool->conn->storageDriver->storageEphemeralFree(def); + return; + } + + return; +} + + +virStorageEphemeralPtr +virStorageEphemeralFromDiskDef(virConnectPtr conn, + virDomainDiskDefPtr def) +{ + if (conn->storageDriver->storageEphemeralFromDiskDef) + return conn->storageDriver->storageEphemeralFromDiskDef(conn, def); + + virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return NULL; +} + + +virStorageEphemeralPtr +virStorageEphemeralFromSnapshotDiskDef(virConnectPtr conn, + virDomainSnapshotDiskDefPtr def) +{ + if (conn->storageDriver->storageEphemeralFromSnapshotDiskDef) + return conn->storageDriver->storageEphemeralFromSnapshotDiskDef(conn, def); + + virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return NULL; + +} diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index 115d8d1..0db46d8 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -27,6 +27,9 @@ # include "internal.h" +# include "conf/domain_conf.h" +# include "conf/snapshot_conf.h" + typedef void (*virStateInhibitCallback)(bool inhibit, void *opaque); @@ -280,4 +283,23 @@ int virDomainMigrateConfirm3Params(virDomainPtr domain, int cookieinlen, unsigned int flags, int cancelled); + +/* storage related internal stuff */ +typedef struct _virStorageEphemeral virStorageEphemeral; +typedef virStorageEphemeral *virStorageEphemeralPtr; + +struct _virStorageEphemeral +{ + bool existing; + virStorageVolPtr vol; + virStoragePoolPtr pool; +}; + +void virStorageEphemeralFree(virStorageEphemeralPtr def); + +virStorageEphemeralPtr virStorageEphemeralFromDiskDef(virConnectPtr conn, + virDomainDiskDefPtr def); + +virStorageEphemeralPtr virStorageEphemeralFromSnapshotDiskDef(virConnectPtr conn, + virDomainSnapshotDiskDefPtr def); #endif diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2dbb8f8..7c75531 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -811,6 +811,9 @@ virRegisterNodeDeviceDriver; virRegisterNWFilterDriver; virRegisterSecretDriver; virRegisterStorageDriver; +virStorageEphemeralFree; +virStorageEphemeralFromDiskDef; +virStorageEphemeralFromSnapshotDiskDef; # locking/domain_lock.h -- 1.8.5.1
This patch implements the APIs for getting temporary storage pools for the local filesystem driver using storage_backend_fs. --- src/check-aclrules.pl | 3 + src/storage/storage_driver.c | 253 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 256 insertions(+) diff --git a/src/check-aclrules.pl b/src/check-aclrules.pl index 057517e..7174868 100755 --- a/src/check-aclrules.pl +++ b/src/check-aclrules.pl @@ -73,6 +73,9 @@ my %implwhitelist = ( "xenUnifiedDomainIsPersistent" => 1, "xenUnifiedDomainIsUpdated" => 1, "xenUnifiedDomainOpenConsole" => 1, + "storageEphemeralFree" => 1, # internal API, no RPC + "storageEphemeralFromDiskDef" => 1, # internal API, no RPC + "storageEphemeralFromSnapshotDiskDef" => 1, # internal API, no RPC ); my %filterimplwhitelist = ( "xenUnifiedConnectListDomains" => 1, diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index c92e6c2..436b8e0 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -50,6 +50,8 @@ #include "virstring.h" #include "viraccessapicheck.h" +#include "dirname.h" + #define VIR_FROM_THIS VIR_FROM_STORAGE static virStorageDriverStatePtr driverState; @@ -2592,6 +2594,253 @@ cleanup: return ret; } + +static void +storageEphemeralFree(virStorageEphemeralPtr def) +{ + virConnectPtr conn; + virStorageDriverStatePtr driver; + virStoragePoolObjPtr pool; + + if (!def) + return; + + if (!def->existing && + def->pool) { + conn = def->pool->conn; + driver = conn->storagePrivateData; + + if ((pool = virStoragePoolObjFindByUUID(&driver->pools, + def->pool->uuid))) { + virStoragePoolObjRemove(&driver->pools, pool); + } + } + + if (def->vol) + virStorageVolFree(def->vol); + + if (def->pool) + virStoragePoolFree(def->pool); +} + + +static void +storageEphemeralGenerateUniquePoolID(virStoragePoolDefPtr def) +{ + virUUIDGenerate(def->uuid); + virUUIDFormat(def->uuid, def->name); +} + + +static virStoragePoolPtr +storageEphemeralCreateDirPool(virConnectPtr conn, + const char *source) +{ + virStorageDriverStatePtr driver = conn->storagePrivateData; + virStoragePoolDefPtr def = NULL; + virStoragePoolObjPtr pool = NULL; + virStorageBackendPtr backend; + virStoragePoolPtr ret = NULL; + + if (VIR_ALLOC(def) < 0) + goto cleanup; + + if (VIR_ALLOC_N(def->name, VIR_UUID_STRING_BUFLEN) < 0) + goto cleanup; + + if (VIR_STRDUP(def->target.path, source) < 0) + goto cleanup; + + if (VIR_STRDUP(def->source.dir, def->target.path) < 0) + goto cleanup; + + def->type = VIR_STORAGE_POOL_DIR; + + storageDriverLock(driver); + + /* generate a unique name */ + do { + if (pool) { + virStoragePoolObjUnlock(pool); + pool = NULL; + } + + storageEphemeralGenerateUniquePoolID(def); + + if ((pool = virStoragePoolObjFindByUUID(&driver->pools, def->uuid))) + continue; + + pool = virStoragePoolObjFindByName(&driver->pools, def->name); + } while (pool); + + if (!(backend = virStorageBackendForType(def->type))) + goto cleanup; + + if (!(pool = virStoragePoolObjAssignDef(&driver->pools, def))) + goto cleanup; + def = NULL; + + if (backend->startPool && + backend->startPool(conn, pool) < 0) { + virStoragePoolObjRemove(&driver->pools, pool); + pool = NULL; + goto cleanup; + } + + pool->active = 1; + pool->internal = true; + + ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid, + NULL, NULL); + +cleanup: + virStoragePoolDefFree(def); + if (pool) + virStoragePoolObjUnlock(pool); + storageDriverUnlock(driver); + return ret; +} + + +static virStorageVolPtr +storageEphemeralCreateVol(virStoragePoolPtr obj, + const char *source) +{ + + virStorageDriverStatePtr driver = obj->conn->storagePrivateData; + virStoragePoolObjPtr pool; + virStorageBackendPtr backend; + virStorageVolDefPtr vol = NULL; + virStorageVolPtr ret = NULL; + + storageDriverLock(driver); + pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); + storageDriverUnlock(driver); + + if (!pool) { + virReportError(VIR_ERR_NO_STORAGE_POOL, + _("no storage pool with matching uuid %s"), obj->uuid); + goto cleanup; + } + + if ((backend = virStorageBackendForType(pool->def->type)) == NULL) + goto cleanup; + + if (VIR_ALLOC(vol) < 0) + goto cleanup; + + if (VIR_STRDUP(vol->name, source) < 0) + goto cleanup; + + if (backend->createVol(obj->conn, pool, vol, true) < 0) + goto cleanup; + + if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) < 0) + goto cleanup; + + if (!(ret = virGetStorageVol(obj->conn, pool->def->name, + pool->volumes.objs[pool->volumes.count - 1]->name, + pool->volumes.objs[pool->volumes.count - 1]->key, + NULL, NULL))) { + vol = pool->volumes.objs[pool->volumes.count--]; + goto cleanup; + } + +cleanup: + virStorageVolDefFree(vol); + if (pool) + virStoragePoolObjUnlock(pool); + return ret; +} + + +static virStorageEphemeralPtr +storageEphemeralCreate(virConnectPtr conn, + int type, + const char *source, + virDomainDiskSourcePoolDefPtr srcpool) +{ + virStorageEphemeralPtr ret = NULL; + char *dirname = NULL; + char *filename = NULL; + + if (VIR_ALLOC(ret) < 0) + goto error; + + switch ((enum virDomainDiskType) type) { + case VIR_DOMAIN_DISK_TYPE_LAST: + case VIR_DOMAIN_DISK_TYPE_BLOCK: + case VIR_DOMAIN_DISK_TYPE_FILE: + case VIR_DOMAIN_DISK_TYPE_DIR: + if (!(dirname = mdir_name(source))) { + virReportOOMError(); + goto error; + } + + if (VIR_STRDUP(filename, last_component(source)) < 1) + goto error; + + if (!(ret->pool = storageEphemeralCreateDirPool(conn, dirname))) + goto error; + + if (!(ret->vol = storageEphemeralCreateVol(ret->pool, filename))) + goto error; + + break; + + case VIR_DOMAIN_DISK_TYPE_NETWORK: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("ephemeral network based volumes are not yet supported")); + goto error; + break; + + case VIR_DOMAIN_DISK_TYPE_VOLUME: + ret->existing = true; + + if (!(ret->pool = virStoragePoolLookupByName(conn, srcpool->pool))) + goto error; + + if (!(ret->vol = virStorageVolLookupByName(ret->pool, srcpool->volume))) + goto error; + + break; + } + +cleanup: + VIR_FREE(dirname); + VIR_FREE(filename); + + return ret; + +error: + storageEphemeralFree(ret); + ret = NULL; + goto cleanup; +} + + +static virStorageEphemeralPtr +storageEphemeralFromDiskDef(virConnectPtr conn, + virDomainDiskDefPtr def) +{ + return storageEphemeralCreate(conn, + def->type, + def->src, + def->srcpool); + +} + + +static virStorageEphemeralPtr +storageEphemeralFromSnapshotDiskDef(virConnectPtr conn, + virDomainSnapshotDiskDefPtr def) +{ + return storageEphemeralCreate(conn, + def->type, + def->file, + NULL); +} + static virStorageDriver storageDriver = { .name = "storage", .storageOpen = storageOpen, /* 0.4.0 */ @@ -2638,6 +2887,10 @@ static virStorageDriver storageDriver = { .storagePoolIsActive = storagePoolIsActive, /* 0.7.3 */ .storagePoolIsPersistent = storagePoolIsPersistent, /* 0.7.3 */ + + .storageEphemeralFree = storageEphemeralFree, /* 1.2.1 */ + .storageEphemeralFromDiskDef = storageEphemeralFromDiskDef, /* 1.2.1 */ + .storageEphemeralFromSnapshotDiskDef = storageEphemeralFromSnapshotDiskDef, /* 1.2.1 */ }; -- 1.8.5.1
Add code to convert an regular guest volume into a ephemeral pool. --- src/storage/storage_driver.c | 153 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 150 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 436b8e0..c0637b0 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2633,6 +2633,106 @@ storageEphemeralGenerateUniquePoolID(virStoragePoolDefPtr def) static virStoragePoolPtr +storageEphemeralCreateGlusterPool(virConnectPtr conn, + const char *source, + virDomainDiskHostDefPtr host) +{ + virStorageDriverStatePtr driver = conn->storagePrivateData; + virStoragePoolDefPtr def = NULL; + virStoragePoolObjPtr pool = NULL; + virStorageBackendPtr backend; + virStoragePoolPtr ret = NULL; + + char *volume = NULL; + char *path = NULL; + char *tmp; + + if (VIR_STRDUP(volume, source) < 0) + goto cleanup; + + if ((tmp = strchr(volume, '/'))) + *tmp++ = '0'; + + if (tmp && virAsprintf(&path, "/%s", tmp) < 0) + goto cleanup; + + if (VIR_ALLOC(def) < 0) + goto cleanup; + + if (VIR_ALLOC_N(def->source.hosts, 1)) + goto cleanup; + + def->source.nhost = 1; + + if (VIR_STRDUP(def->source.hosts->name, host->name) < 0) + goto cleanup; + + if (host->port && + virStrToLong_i(host->port, NULL, 10, &def->source.hosts->port) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("failed to parse port number '%s'"), + host->port); + goto cleanup; + } + + if (VIR_ALLOC_N(def->name, VIR_UUID_STRING_BUFLEN) < 0) + goto cleanup; + + def->source.name = volume; + volume = NULL; + + def->source.dir = path; + path = NULL; + + def->type = VIR_STORAGE_POOL_GLUSTER; + + storageDriverLock(driver); + + /* generate a unique name */ + do { + if (pool) { + virStoragePoolObjUnlock(pool); + pool = NULL; + } + + storageEphemeralGenerateUniquePoolID(def); + + if ((pool = virStoragePoolObjFindByUUID(&driver->pools, def->uuid))) + continue; + + pool = virStoragePoolObjFindByName(&driver->pools, def->name); + } while (pool); + + if (!(backend = virStorageBackendForType(def->type))) + goto cleanup; + + if (!(pool = virStoragePoolObjAssignDef(&driver->pools, def))) + goto cleanup; + def = NULL; + + if (backend->startPool && + backend->startPool(conn, pool) < 0) { + virStoragePoolObjRemove(&driver->pools, pool); + pool = NULL; + goto cleanup; + } + + pool->active = 1; + + ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid, + NULL, NULL); + +cleanup: + virStoragePoolDefFree(def); + if (pool) + virStoragePoolObjUnlock(pool); + storageDriverUnlock(driver); + return ret; +} + + + +static virStoragePoolPtr storageEphemeralCreateDirPool(virConnectPtr conn, const char *source) { @@ -2758,6 +2858,9 @@ static virStorageEphemeralPtr storageEphemeralCreate(virConnectPtr conn, int type, const char *source, + int protocol, + size_t nhosts, + virDomainDiskHostDefPtr hosts, virDomainDiskSourcePoolDefPtr srcpool) { virStorageEphemeralPtr ret = NULL; @@ -2789,9 +2892,47 @@ storageEphemeralCreate(virConnectPtr conn, break; case VIR_DOMAIN_DISK_TYPE_NETWORK: - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("ephemeral network based volumes are not yet supported")); - goto error; + switch ((enum virDomainDiskProtocol) protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: + if (nhosts != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Gluster disk supports only 1 source")); + goto error; + } + + if (!(dirname = mdir_name(source))) { + virReportOOMError(); + goto error; + } + + if (VIR_STRDUP(filename, last_component(source)) < 1) + goto error; + + if (!(ret->pool = storageEphemeralCreateGlusterPool(conn, dirname, hosts))) + goto error; + + if (!(ret->vol = storageEphemeralCreateVol(ret->pool, filename))) + goto error; + + break; + + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + case VIR_DOMAIN_DISK_PROTOCOL_RBD: + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + case VIR_DOMAIN_DISK_PROTOCOL_ISCSI: + case VIR_DOMAIN_DISK_PROTOCOL_HTTP: + case VIR_DOMAIN_DISK_PROTOCOL_HTTPS: + case VIR_DOMAIN_DISK_PROTOCOL_FTP: + case VIR_DOMAIN_DISK_PROTOCOL_FTPS: + case VIR_DOMAIN_DISK_PROTOCOL_TFTP: + case VIR_DOMAIN_DISK_PROTOCOL_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("ephemeral network based volumes using '%s' " + "protocol are not yet supported"), + virDomainDiskProtocolTypeToString(protocol)); + goto error; + break; + } break; case VIR_DOMAIN_DISK_TYPE_VOLUME: @@ -2826,6 +2967,9 @@ storageEphemeralFromDiskDef(virConnectPtr conn, return storageEphemeralCreate(conn, def->type, def->src, + def->protocol, + def->nhosts, + def->hosts, def->srcpool); } @@ -2838,6 +2982,9 @@ storageEphemeralFromSnapshotDiskDef(virConnectPtr conn, return storageEphemeralCreate(conn, def->type, def->file, + def->protocol, + def->nhosts, + def->hosts, NULL); } -- 1.8.5.1
Use the new storage conversion APIs to delete garbage left behind after a failed snapshot attempt using the storage driver. --- src/qemu/qemu_driver.c | 60 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 46 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 304c7ee..fad8c60 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12170,7 +12170,8 @@ cleanup: /* The domain is expected to hold monitor lock. */ static int -qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, +qemuDomainSnapshotCreateSingleDiskActive(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainSnapshotDiskDefPtr snap, virDomainDiskDefPtr disk, @@ -12188,6 +12189,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, int ret = -1; int fd = -1; bool need_unlink = false; + virStorageEphemeralPtr temppool = NULL; if (snap->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -12207,6 +12209,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, virStorageFileFreeMetadata(disk->backingChain); disk->backingChain = NULL; + if (!(temppool = virStorageEphemeralFromSnapshotDiskDef(conn, snap))) + goto cleanup; + switch (snap->type) { case VIR_DOMAIN_DISK_TYPE_BLOCK: reuse = true; @@ -12282,8 +12287,16 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, } cleanup: - if (need_unlink && unlink(source)) - VIR_WARN("unable to unlink just-created %s", source); + if (need_unlink) { + if (virStorageVolDelete(temppool->vol, 0) < 0) { + char *path = virStorageVolGetPath(temppool->vol); + VIR_WARN("unable to remove just-created snapshot storage '%s'", + NULLSTR(path)); + VIR_FREE(path); + } + } + + virStorageEphemeralFree(temppool); VIR_FREE(device); VIR_FREE(source); VIR_FREE(persistSource); @@ -12294,7 +12307,8 @@ cleanup: * counterpart to qemuDomainSnapshotCreateSingleDiskActive, called * only on a failed transaction. */ static void -qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, +qemuDomainSnapshotUndoSingleDiskActive(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr origdisk, virDomainDiskDefPtr disk, @@ -12303,17 +12317,33 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, { char *source = NULL; char *persistSource = NULL; - struct stat st; + + virStorageEphemeralPtr temppool = NULL; if (VIR_STRDUP(source, origdisk->src) < 0 || (persistDisk && VIR_STRDUP(persistSource, source) < 0)) goto cleanup; - qemuDomainPrepareDiskChainElement(driver, vm, disk, disk->src, - VIR_DISK_CHAIN_NO_ACCESS); - if (need_unlink && stat(disk->src, &st) == 0 && - S_ISREG(st.st_mode) && unlink(disk->src) < 0) - VIR_WARN("Unable to remove just-created %s", disk->src); + switch (origdisk->type) { + case VIR_DOMAIN_DISK_TYPE_BLOCK: + case -1: /* type was not provided in snapshot conf */ + case VIR_DOMAIN_DISK_TYPE_FILE: + qemuDomainPrepareDiskChainElement(driver, vm, disk, disk->src, + VIR_DISK_CHAIN_NO_ACCESS); + break; + + default: + break; + } + + temppool = virStorageEphemeralFromDiskDef(conn, disk); + + if (need_unlink && temppool && + virStorageVolDelete(temppool->vol, 0) < 0) { + char *path = virStorageVolGetPath(temppool->vol); + VIR_WARN("Unable to remove just-created '%s'", NULLSTR(path)); + VIR_FREE(path); + } /* Update vm in place to match changes. */ VIR_FREE(disk->src); @@ -12330,13 +12360,15 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, } cleanup: + virStorageEphemeralFree(temppool); VIR_FREE(source); VIR_FREE(persistSource); } /* The domain is expected to be locked and active. */ static int -qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, +qemuDomainSnapshotCreateDiskActive(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainSnapshotObjPtr snap, unsigned int flags, @@ -12384,7 +12416,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, persistDisk = vm->newDef->disks[indx]; } - ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm, + ret = qemuDomainSnapshotCreateSingleDiskActive(conn, driver, vm, &snap->def->disks[i], vm->def->disks[i], persistDisk, actions, @@ -12426,7 +12458,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, } - qemuDomainSnapshotUndoSingleDiskActive(driver, vm, + qemuDomainSnapshotUndoSingleDiskActive(conn, driver, vm, snap->def->dom->disks[i], vm->def->disks[i], persistDisk, @@ -12568,7 +12600,7 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn, * * Next we snapshot the disks. */ - if ((ret = qemuDomainSnapshotCreateDiskActive(driver, vm, snap, flags, + if ((ret = qemuDomainSnapshotCreateDiskActive(conn, driver, vm, snap, flags, QEMU_ASYNC_JOB_SNAPSHOT)) < 0) goto endjob; -- 1.8.5.1
--- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 9 ++++ src/qemu/qemu_driver.c | 113 +++++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 116 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a80559e..d742248 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3826,7 +3826,7 @@ cleanup: } -static int +int qemuGetDriveSourceString(int type, const char *src, int protocol, diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 66c23cc..ec944ea 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -312,4 +312,13 @@ qemuParseKeywords(const char *str, int *retnkeywords, int allowEmptyValue); +int qemuGetDriveSourceString(int type, + const char *src, + int protocol, + size_t nhosts, + virDomainDiskHostDefPtr hosts, + const char *username, + const char *secret, + char **path); + #endif /* __QEMU_COMMAND_H__*/ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fad8c60..872634f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11492,6 +11492,24 @@ cleanup: return ret; } + +static int +qemuDomainSnapshotDiskGetSourceString(virDomainSnapshotDiskDefPtr disk, + char **source) +{ + *source = NULL; + + return qemuGetDriveSourceString(qemuSnapshotDiskGetActualType(disk), + disk->file, + disk->protocol, + disk->nhosts, + disk->hosts, + NULL, + NULL, + source); +} + + typedef enum { VIR_DISK_CHAIN_NO_ACCESS, VIR_DISK_CHAIN_READ_ONLY, @@ -11872,6 +11890,29 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d return 0; case VIR_DOMAIN_DISK_TYPE_NETWORK: + switch ((enum virDomainDiskProtocol) disk->protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: + return 0; + + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + case VIR_DOMAIN_DISK_PROTOCOL_RBD: + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + case VIR_DOMAIN_DISK_PROTOCOL_ISCSI: + case VIR_DOMAIN_DISK_PROTOCOL_HTTP: + case VIR_DOMAIN_DISK_PROTOCOL_HTTPS: + case VIR_DOMAIN_DISK_PROTOCOL_FTP: + case VIR_DOMAIN_DISK_PROTOCOL_FTPS: + case VIR_DOMAIN_DISK_PROTOCOL_TFTP: + case VIR_DOMAIN_DISK_PROTOCOL_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("external active snapshots are not supported on " + "'network' disks using '%s' protocol"), + virDomainDiskProtocolTypeToString(disk->protocol)); + return -1; + + } + break; + case VIR_DOMAIN_DISK_TYPE_DIR: case VIR_DOMAIN_DISK_TYPE_VOLUME: case VIR_DOMAIN_DISK_TYPE_LAST: @@ -12183,6 +12224,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virConnectPtr conn, qemuDomainObjPrivatePtr priv = vm->privateData; char *device = NULL; char *source = NULL; + char *newsource = NULL; + virDomainDiskHostDefPtr newhosts = NULL; + virDomainDiskHostDefPtr persistHosts = NULL; int format = snap->format; const char *formatStr = NULL; char *persistSource = NULL; @@ -12197,8 +12241,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virConnectPtr conn, return -1; } - if (virAsprintf(&device, "drive-%s", disk->info.alias) < 0 || - (persistDisk && VIR_STRDUP(persistSource, source) < 0)) + if (virAsprintf(&device, "drive-%s", disk->info.alias) < 0) goto cleanup; /* XXX Here, we know we are about to alter disk->backingChain if @@ -12212,14 +12255,22 @@ qemuDomainSnapshotCreateSingleDiskActive(virConnectPtr conn, if (!(temppool = virStorageEphemeralFromSnapshotDiskDef(conn, snap))) goto cleanup; + if (qemuDomainSnapshotDiskGetSourceString(snap, &source) < 0) + goto cleanup; + + if (VIR_STRDUP(newsource, snap->file) < 0) + goto cleanup; + + if (persistDisk && + VIR_STRDUP(persistSource, snap->file) < 0) + goto cleanup; + switch (snap->type) { case VIR_DOMAIN_DISK_TYPE_BLOCK: reuse = true; /* fallthrough */ case -1: /* type was not provided in snapshot conf */ case VIR_DOMAIN_DISK_TYPE_FILE: - if (VIR_STRDUP(source, snap->file) < 0) - goto cleanup; /* create the stub file and set selinux labels; manipulate disk in * place, in a way that can be reverted on failure. */ @@ -12239,6 +12290,27 @@ qemuDomainSnapshotCreateSingleDiskActive(virConnectPtr conn, } break; + case VIR_DOMAIN_DISK_TYPE_NETWORK: + switch (snap->protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: + if (!(newhosts = virDomainDiskHostDefCopy(snap->nhosts, snap->hosts))) + goto cleanup; + + if (persistDisk && + !(persistHosts = virDomainDiskHostDefCopy(snap->nhosts, snap->hosts))) + goto cleanup; + + break; + + default: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("snapshots on volumes using '%s' protocol " + "are not supported"), + virDomainDiskProtocolTypeToString(snap->protocol)); + goto cleanup; + } + break; + default: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("snapshots are not supported on '%s' volumes"), @@ -12273,17 +12345,33 @@ qemuDomainSnapshotCreateSingleDiskActive(virConnectPtr conn, /* Update vm in place to match changes. */ need_unlink = false; + VIR_FREE(disk->src); - disk->src = source; - source = NULL; + virDomainDiskHostDefFree(disk->nhosts, disk->hosts); + + disk->src = newsource; disk->format = format; disk->type = snap->type; + disk->protocol = snap->protocol; + disk->nhosts = snap->nhosts; + disk->hosts = newhosts; + + newsource = NULL; + newhosts = NULL; + if (persistDisk) { VIR_FREE(persistDisk->src); + virDomainDiskHostDefFree(persistDisk->nhosts, persistDisk->hosts); + persistDisk->src = persistSource; - persistSource = NULL; persistDisk->format = format; persistDisk->type = snap->type; + persistDisk->protocol = snap->protocol; + persistDisk->nhosts = snap->nhosts; + persistDisk->hosts = persistHosts; + + persistSource = NULL; + persistHosts = NULL; } cleanup: @@ -12299,7 +12387,10 @@ cleanup: virStorageEphemeralFree(temppool); VIR_FREE(device); VIR_FREE(source); + VIR_FREE(newsource); VIR_FREE(persistSource); + virDomainDiskHostDefFree(snap->nhosts, newhosts); + virDomainDiskHostDefFree(snap->nhosts, persistHosts); return ret; } @@ -12351,12 +12442,20 @@ qemuDomainSnapshotUndoSingleDiskActive(virConnectPtr conn, source = NULL; disk->format = origdisk->format; disk->type = origdisk->type; + disk->protocol = origdisk->protocol; + virDomainDiskHostDefFree(disk->nhosts, disk->hosts); + disk->nhosts = origdisk->nhosts; + disk->hosts = virDomainDiskHostDefCopy(origdisk->nhosts, origdisk->hosts); if (persistDisk) { VIR_FREE(persistDisk->src); persistDisk->src = persistSource; persistSource = NULL; persistDisk->format = origdisk->format; persistDisk->type = origdisk->type; + persistDisk->protocol = origdisk->protocol; + virDomainDiskHostDefFree(persistDisk->nhosts, persistDisk->hosts); + persistDisk->nhosts = origdisk->nhosts; + persistDisk->hosts = virDomainDiskHostDefCopy(origdisk->nhosts, origdisk->hosts); } cleanup: -- 1.8.5.1
participants (2)
-
Eric Blake -
Peter Krempa