9:35 a.m.
Disk operations can take ages to finish. Therefore users
might want to abort such job, e.g. because host is going
down for maintenance. This patch set is trying to allow
this kind of behaviour. The inspiration was taken from
qemu driver.
How it works:
An API that is known to run for a long time, e.g. volume
allocation, wiping, have to find a pool which volume
belongs to. If the pool was found it is locked. During
this time the storage driver is locked. Now the API is
preparing for taking the operation. Ideally, it would be
sufficient to have only volume locked, but it requires
far more changes to code.
Just before the API is about to take the long term action,
storageBeginJob[WithPool] is called. This wait on a condition
specific for a volume. Similar to QEMU driver, wait is
limited in time. Timeout is set to 30 seconds.
Upon successful job acquire, a counter in the pool
is incremented so the pool is prevented from destroy,
undef; Then the pool is unlocked.
After API finishes it's work, it calls storageEndJob[WithPool]
which reset job structure in the volume, re-locks
the pool, decrement the counter and broadcasts on
the condition.
Meanwhile, if the long term action is done internally,
it should be done in a cycle so we can check if job abort
wasn't signalized. If it is done by external program,
we can virCommandAbort() it.
Several improvements can be made, but I think this is
good for start.
NB, I've implemented job aborting for the both APIs using
streams, however wasn't very successful with aborting
a stream from the daemon site.
Michal Privoznik (4):
storage: Introduce virStorageVolAbortJob
virsh: Expose virStorageVolAbortJob
storage: Implement jobs for storage driver
storage: Implement virStorageVolAbortJob
include/libvirt/libvirt.h.in | 3 +
src/conf/storage_conf.c | 12 ++
src/conf/storage_conf.h | 29 +++-
src/driver.h | 5 +
src/libvirt.c | 49 +++++
src/libvirt_private.syms | 4 +
src/libvirt_public.syms | 1 +
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 8 +-
src/remote_protocol-structs | 5 +
src/storage/storage_backend.c | 53 +++---
src/storage/storage_backend_fs.c | 8 +-
src/storage/storage_driver.c | 408 ++++++++++++++++++++++++++++++--------
src/storage/storage_driver.h | 7 +
tools/virsh.c | 39 ++++
15 files changed, 516 insertions(+), 116 deletions(-)
--
1.7.8.5
9:35 a.m.
New subject: [libvirt] [PATCH 1/4] storage: Introduce virStorageVolAbortJob
This API can be used to terminate long running jobs
on a volume like its building, resizing, wiping.
Moreover, like virDomainAbortJob() calling this API
will block until job has either completed or aborted.
---
include/libvirt/libvirt.h.in | 3 ++
src/driver.h | 5 ++++
src/libvirt.c | 49 ++++++++++++++++++++++++++++++++++++++++++
src/libvirt_public.syms | 1 +
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 8 ++++++-
src/remote_protocol-structs | 5 ++++
7 files changed, 71 insertions(+), 1 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 7d41642..77ec3f0 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2513,6 +2513,9 @@ int virStorageVolResize
(virStorageVolPtr vol,
unsigned long long capacity,
unsigned int flags);
+int virStorageVolAbortJob (virStorageVolPtr vol,
+ unsigned int flags);
+
/**
* virKeycodeSet:
diff --git a/src/driver.h b/src/driver.h
index 03d249b..7845b06 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -1314,6 +1314,10 @@ typedef int
unsigned int flags);
typedef int
+ (*virDrvStorageVolAbortJob) (virStorageVolPtr vol,
+ unsigned int flags);
+
+typedef int
(*virDrvStoragePoolIsActive)(virStoragePoolPtr pool);
typedef int
(*virDrvStoragePoolIsPersistent)(virStoragePoolPtr pool);
@@ -1377,6 +1381,7 @@ struct _virStorageDriver {
virDrvStorageVolResize volResize;
virDrvStoragePoolIsActive poolIsActive;
virDrvStoragePoolIsPersistent poolIsPersistent;
+ virDrvStorageVolAbortJob volAbortJob;
};
# ifdef WITH_LIBVIRTD
diff --git a/src/libvirt.c b/src/libvirt.c
index e916aa0..8ce3234 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -13343,6 +13343,55 @@ error:
}
/**
+ * virStorageVolAbortJob:
+ * @vol: pointer to storage volume
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Requests that the current background job be aborted at the soonest
+ * opportunity. This will block until the job has either completed,
+ * or aborted.
+ *
+ * Returns: 0 in case of success
+ * -1 otherwise
+ */
+int
+virStorageVolAbortJob(virStorageVolPtr vol,
+ unsigned int flags)
+{
+ virConnectPtr conn;
+ VIR_DEBUG("vol=%p flags=%x", vol, flags);
+
+ virResetLastError();
+
+ if (!VIR_IS_STORAGE_VOL(vol)) {
+ virLibStorageVolError(VIR_ERR_INVALID_STORAGE_VOL, __FUNCTION__);
+ virDispatchError(NULL);
+ return -1;
+ }
+
+ conn = vol->conn;
+
+ if (conn->flags & VIR_CONNECT_RO) {
+ virLibConnError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ goto error;
+ }
+
+ if (conn->storageDriver && conn->storageDriver->volAbortJob) {
+ int ret;
+ ret = conn->storageDriver->volAbortJob(vol, flags);
+ if (ret < 0)
+ goto error;
+ return ret;
+ }
+
+ virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+ virDispatchError(vol->conn);
+ return -1;
+}
+
+/**
* virNodeNumOfDevices:
* @conn: pointer to the hypervisor connection
* @cap: capability name
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 46c13fb..cd3e2a6 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -532,6 +532,7 @@ LIBVIRT_0.9.10 {
LIBVIRT_0.9.11 {
global:
virDomainPMWakeup;
+ virStorageVolAbortJob;
} LIBVIRT_0.9.10;
# .... define new API here using predicted next version number ....
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 031167a..3534ac0 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -5013,6 +5013,7 @@ static virStorageDriver storage_driver = {
.volResize = remoteStorageVolResize, /* 0.9.10 */
.poolIsActive = remoteStoragePoolIsActive, /* 0.7.3 */
.poolIsPersistent = remoteStoragePoolIsPersistent, /* 0.7.3 */
+ .volAbortJob = remoteStorageVolAbortJob, /* 0.9.11 */
};
static virSecretDriver secret_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 4d845e7..014eade 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -1754,6 +1754,11 @@ struct remote_storage_vol_resize_args {
unsigned int flags;
};
+struct remote_storage_vol_abort_job_args {
+ remote_nonnull_storage_vol vol;
+ unsigned int flags;
+};
+
/* Node driver calls: */
struct remote_node_num_of_devices_args {
@@ -2765,7 +2770,8 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_SET_METADATA = 264, /* autogen autogen */
REMOTE_PROC_DOMAIN_GET_METADATA = 265, /* autogen autogen */
REMOTE_PROC_DOMAIN_BLOCK_REBASE = 266, /* autogen autogen */
- REMOTE_PROC_DOMAIN_PM_WAKEUP = 267 /* autogen autogen */
+ REMOTE_PROC_DOMAIN_PM_WAKEUP = 267, /* autogen autogen */
+ REMOTE_PROC_STORAGE_VOL_ABORT_JOB = 268 /* autogen autogen */
/*
* Notice how the entries are grouped in sets of 10 ?
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 8f882b7..c80d5f0 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -1317,6 +1317,10 @@ struct remote_storage_vol_resize_args {
uint64_t capacity;
u_int flags;
};
+struct remote_storage_vol_abort_job_args {
+ remote_nonnull_storage_vol vol;
+ u_int flags;
+};
struct remote_node_num_of_devices_args {
remote_string cap;
u_int flags;
@@ -2178,4 +2182,5 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_GET_METADATA = 265,
REMOTE_PROC_DOMAIN_BLOCK_REBASE = 266,
REMOTE_PROC_DOMAIN_PM_WAKEUP = 267,
+ REMOTE_PROC_STORAGE_VOL_ABORT_JOB = 268,
};
--
1.7.8.5
9:48 a.m.
New subject: [libvirt] [PATCH 1/4] storage: Introduce virStorageVolAbortJob
On Tue, Mar 13, 2012 at 03:35:29PM +0100, Michal Privoznik wrote:
This API can be used to terminate long running jobs
on a volume like its building, resizing, wiping.
Moreover, like virDomainAbortJob() calling this API
will block until job has either completed or aborted.
---
include/libvirt/libvirt.h.in | 3 ++
src/driver.h | 5 ++++
src/libvirt.c | 49 ++++++++++++++++++++++++++++++++++++++++++
src/libvirt_public.syms | 1 +
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 8 ++++++-
src/remote_protocol-structs | 5 ++++
7 files changed, 71 insertions(+), 1 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 7d41642..77ec3f0 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2513,6 +2513,9 @@ int virStorageVolResize
(virStorageVolPtr vol,
unsigned long long capacity,
unsigned int flags);
+int virStorageVolAbortJob (virStorageVolPtr vol,
+ unsigned int flags);
+
No, virStorageVolGetJobInfo() API to go with it ? IMHO we should have
both, so we mirror the virDomain job API design.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
9:57 a.m.
New subject: [libvirt] [PATCH 1/4] storage: Introduce virStorageVolAbortJob
On 13.03.2012 15:48, Daniel P. Berrange wrote:
On Tue, Mar 13, 2012 at 03:35:29PM +0100, Michal Privoznik wrote:
> This API can be used to terminate long running jobs
> on a volume like its building, resizing, wiping.
> Moreover, like virDomainAbortJob() calling this API
> will block until job has either completed or aborted.
> ---
> include/libvirt/libvirt.h.in | 3 ++
> src/driver.h | 5 ++++
> src/libvirt.c | 49 ++++++++++++++++++++++++++++++++++++++++++
> src/libvirt_public.syms | 1 +
> src/remote/remote_driver.c | 1 +
> src/remote/remote_protocol.x | 8 ++++++-
> src/remote_protocol-structs | 5 ++++
> 7 files changed, 71 insertions(+), 1 deletions(-)
>
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 7d41642..77ec3f0 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -2513,6 +2513,9 @@ int virStorageVolResize
(virStorageVolPtr vol,
> unsigned long long
capacity,
> unsigned int flags);
>
> +int virStorageVolAbortJob (virStorageVolPtr vol,
> + unsigned int flags);
> +
No, virStorageVolGetJobInfo() API to go with it ? IMHO we should have
both, so we mirror the virDomain job API design.
Regards,
Daniel
yeah, virStorageVolGetJobInfo() is one of the improvements I'm
mentioning in cover letter. But I've decided to not implement it for now
as another huge bunch of code would have to be rewritten make this patch
set unbearable big. But if it is a show stopper I can rewrite and post v2.
9:35 a.m.
New subject: [libvirt] [PATCH 2/4] virsh: Expose virStorageVolAbortJob
via new virsh command 'vol-jobabort'. Currently, it accepts
only volume specification as argument.
---
tools/virsh.c | 39 +++++++++++++++++++++++++++++++++++++++
1 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 630b77f..00668ff 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -12497,6 +12497,44 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
return true;
}
+/*
+ * "vol-jobabort" command
+ */
+static const vshCmdInfo info_vol_jobabort[] = {
+ {"help", N_("abort active volume job")},
+ {"desc", N_("Aborts the currently running volume job")},
+ {NULL, NULL}
+};
+
+static const vshCmdOptDef opts_vol_jobabort[] = {
+ {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("volume name or key")},
+ {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")},
+ {NULL, 0, 0, NULL}
+};
+
+static bool
+cmdVolJobAbort(vshControl *ctl, const vshCmd *cmd)
+{
+ virStorageVolPtr vol;
+ bool ret = false;
+
+ if (!vshConnectionUsability(ctl, ctl->conn))
+ return false;
+
+ if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", NULL))) {
+ return false;
+ }
+
+ if (virStorageVolAbortJob(vol, 0) < 0)
+ goto cleanup;
+
+ vshPrint(ctl, _("Job successfully aborted"));
+ ret = true;
+
+cleanup:
+ virStorageVolFree(vol);
+ return ret;
+}
/*
* "secret-define" command
@@ -17239,6 +17277,7 @@ static const vshCmdDef storageVolCmds[] = {
{"vol-download", cmdVolDownload, opts_vol_download, info_vol_download, 0},
{"vol-dumpxml", cmdVolDumpXML, opts_vol_dumpxml, info_vol_dumpxml, 0},
{"vol-info", cmdVolInfo, opts_vol_info, info_vol_info, 0},
+ {"vol-jobabort", cmdVolJobAbort, opts_vol_jobabort, info_vol_jobabort, 0},
{"vol-key", cmdVolKey, opts_vol_key, info_vol_key, 0},
{"vol-list", cmdVolList, opts_vol_list, info_vol_list, 0},
{"vol-name", cmdVolName, opts_vol_name, info_vol_name, 0},
--
1.7.8.5
9:35 a.m.
New subject: [libvirt] [PATCH 3/4] storage: Implement jobs for storage driver
Simply, when we are about to take an action which might take ages,
like allocating new volumes, wiping, etc. increment a counter of
jobs in pool object and unlock it. We don't want to hold the pool
locked during long term actions.
---
src/conf/storage_conf.c | 12 ++
src/conf/storage_conf.h | 22 +++-
src/libvirt_private.syms | 1 +
src/storage/storage_driver.c | 284 +++++++++++++++++++++++++++++++-----------
4 files changed, 246 insertions(+), 73 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index bdf6218..e378ceb 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -251,6 +251,8 @@ virStorageVolDefFree(virStorageVolDefPtr def) {
if (!def)
return;
+ virMutexDestroy(&def->lock);
+ ignore_value(virCondDestroy(&def->job.cond));
VIR_FREE(def->name);
VIR_FREE(def->key);
@@ -978,6 +980,16 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
return NULL;
}
+ if (virMutexInit(&ret->lock) < 0) {
+ virReportSystemError(errno, "%s", _("Cannot init mutex"));
+ goto cleanup;
+ }
+
+ if (virCondInit(&ret->job.cond) < 0) {
+ virReportSystemError(errno, "%s", _("Cannot init cond"));
+ goto cleanup;
+ }
+
ret->name = virXPathString("string(./name)", ctxt);
if (ret->name == NULL) {
virStorageReportError(VIR_ERR_XML_ERROR,
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 1ef9295..481c806 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -83,6 +83,25 @@ struct _virStorageVolTarget {
};
+enum virStorageVolJob {
+ VIR_STORAGE_VOL_JOB_NONE = 0, /* no job */
+ VIR_STORAGE_VOL_JOB_BUILD,
+ VIR_STORAGE_VOL_JOB_DELETE,
+ VIR_STORAGE_VOL_JOB_REFRESH,
+ VIR_STORAGE_VOL_JOB_WIPE,
+ VIR_STORAGE_VOL_JOB_RESIZE,
+ VIR_STORAGE_VOL_JOB_DOWNLOAD,
+ VIR_STORAGE_VOL_JOB_UPLOAD,
+
+ VIR_STORAGE_VOL_JOB_LAST
+};
+
+struct virStorageVolJobObj {
+ virCond cond;
+ enum virStorageVolJob active;
+ unsigned long long start;
+};
+
typedef struct _virStorageVolDef virStorageVolDef;
typedef virStorageVolDef *virStorageVolDefPtr;
struct _virStorageVolDef {
@@ -90,7 +109,8 @@ struct _virStorageVolDef {
char *key;
int type; /* virStorageVolType enum */
- unsigned int building;
+ virMutex lock;
+ struct virStorageVolJobObj job;
unsigned long long allocation; /* bytes */
unsigned long long capacity; /* bytes */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1f55f5d..fef9d5a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -565,6 +565,7 @@ virFDStreamOpen;
virFDStreamConnectUNIX;
virFDStreamOpenFile;
virFDStreamCreateFile;
+virFDStreamSetInternalCloseCb;
# hash.h
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 66811ce..7f3dfcd 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -48,6 +48,7 @@
#include "virfile.h"
#include "fdstream.h"
#include "configmake.h"
+#include "virtime.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -65,6 +66,111 @@ static void storageDriverUnlock(virStorageDriverStatePtr driver)
}
static void
+storageResetJob(virStorageVolDefPtr vol)
+{
+ struct virStorageVolJobObj *job = &vol->job;
+
+ job->active = VIR_STORAGE_VOL_JOB_NONE;
+ job->start = 0;
+}
+
+/* wait max. 30 seconds for job ackquire */
+#define STORAGE_JOB_WAIT_TIME (1000ull * 30)
+
+static int
+storageBeginJobInternal(virStorageDriverStatePtr driver ATTRIBUTE_UNUSED,
+ virStoragePoolObjPtr pool,
+ bool pool_locked,
+ virStorageVolDefPtr vol,
+ enum virStorageVolJob job)
+{
+ unsigned long long now;
+ unsigned long long then;
+
+ if (virTimeMillisNow(&now) < 0)
+ return -1;
+
+ then = now + STORAGE_JOB_WAIT_TIME;
+
+ while (vol->job.active != VIR_STORAGE_VOL_JOB_NONE) {
+ if (virCondWaitUntil(&vol->job.cond, &vol->lock, then) < 0)
+ goto error;
+ }
+
+ VIR_DEBUG("Starting job %d", job);
+ storageResetJob(vol);
+ vol->job.active = job;
+ vol->job.start = now;
+
+ if (pool_locked) {
+ pool->asyncjobs++;
+ virStoragePoolObjUnlock(pool);
+ }
+
+ return 0;
+
+error:
+ if (errno == ETIMEDOUT)
+ virStorageReportError(VIR_ERR_OPERATION_TIMEOUT, "%s",
+ _("cannot acquire state change lock"));
+ else
+ virReportSystemError(errno, "%s",
+ _("cannot acquire job mutex"));
+ if (pool_locked)
+ virStoragePoolObjLock(pool);
+ return -1;
+}
+
+static int
+storageBeginJob(virStorageDriverStatePtr driver,
+ virStorageVolDefPtr vol,
+ enum virStorageVolJob job)
+{
+ return storageBeginJobInternal(driver, NULL, false, vol, job);
+}
+
+static int
+storageBeginJobWithPool(virStorageDriverStatePtr driver,
+ virStoragePoolObjPtr pool,
+ virStorageVolDefPtr vol,
+ enum virStorageVolJob job)
+{
+ return storageBeginJobInternal(driver, pool, true, vol, job);
+}
+
+static void
+storageEndJobInternal(virStorageDriverStatePtr driver,
+ virStoragePoolObjPtr pool,
+ bool pool_locked,
+ virStorageVolDefPtr vol)
+{
+ VIR_DEBUG("Stopping job %d", vol->job.active);
+ storageResetJob(vol);
+ if (pool_locked) {
+ storageDriverLock(driver);
+ virStoragePoolObjLock(pool);
+ storageDriverUnlock(driver);
+ pool->asyncjobs--;
+ }
+ virCondBroadcast(&vol->job.cond);
+}
+
+static void
+storageEndJob(virStorageDriverStatePtr driver,
+ virStorageVolDefPtr vol)
+{
+ return storageEndJobInternal(driver, NULL, false, vol);
+}
+
+static void
+storageEndJobWithPool(virStorageDriverStatePtr driver,
+ virStoragePoolObjPtr pool,
+ virStorageVolDefPtr vol)
+{
+ return storageEndJobInternal(driver, pool, true, vol);
+}
+
+static void
storageDriverAutostart(virStorageDriverStatePtr driver) {
unsigned int i;
@@ -1365,18 +1471,16 @@ storageVolumeCreateXML(virStoragePoolPtr obj,
memcpy(buildvoldef, voldef, sizeof(*voldef));
/* Drop the pool lock during volume allocation */
- pool->asyncjobs++;
- voldef->building = 1;
- virStoragePoolObjUnlock(pool);
+ if (storageBeginJobWithPool(driver, pool, voldef,
+ VIR_STORAGE_VOL_JOB_BUILD) < 0) {
+ VIR_FREE(buildvoldef);
+ goto cleanup;
+ }
buildret = backend->buildVol(obj->conn, pool, buildvoldef);
- storageDriverLock(driver);
- virStoragePoolObjLock(pool);
- storageDriverUnlock(driver);
- voldef->building = 0;
- pool->asyncjobs--;
+ storageEndJobWithPool(driver, pool, voldef);
voldef = NULL;
VIR_FREE(buildvoldef);
@@ -1416,7 +1520,7 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj,
virStorageBackendPtr backend;
virStorageVolDefPtr origvol = NULL, newvol = NULL;
virStorageVolPtr ret = NULL, volobj = NULL;
- int buildret;
+ int buildret = -1;
virCheckFlags(0, NULL);
@@ -1490,17 +1594,21 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj,
goto cleanup;
}
- if (origvol->building) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
- _("volume '%s' is still being
allocated."),
- origvol->name);
- goto cleanup;
+ if (backend->refreshVol) {
+ int refreshVolRet;
+
+ if (storageBeginJobWithPool(driver, pool, origvol,
+ VIR_STORAGE_VOL_JOB_REFRESH) < 0)
+ goto cleanup;
+
+ refreshVolRet = backend->refreshVol(obj->conn, pool, origvol);
+
+ storageEndJobWithPool(driver, pool, origvol);
+
+ if (refreshVolRet < 0)
+ goto cleanup;
}
- if (backend->refreshVol &&
- backend->refreshVol(obj->conn, pool, origvol) < 0)
- goto cleanup;
-
if (VIR_REALLOC_N(pool->volumes.objs,
pool->volumes.count+1) < 0) {
virReportOOMError();
@@ -1517,34 +1625,23 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj,
newvol->key);
/* Drop the pool lock during volume allocation */
- pool->asyncjobs++;
- origvol->building = 1;
- newvol->building = 1;
- virStoragePoolObjUnlock(pool);
+ if (storageBeginJobWithPool(driver, pool, newvol, VIR_STORAGE_VOL_JOB_BUILD) < 0)
+ goto cleanup;
- if (origpool) {
- origpool->asyncjobs++;
- virStoragePoolObjUnlock(origpool);
- }
+ if ((origpool && storageBeginJobWithPool(driver, origpool, origvol,
VIR_STORAGE_VOL_JOB_BUILD) < 0) ||
+ (!origpool && storageBeginJob(driver, origvol, VIR_STORAGE_VOL_JOB_BUILD)
< 0))
+ goto endjob;
buildret = backend->buildVolFrom(obj->conn, pool, newvol, origvol, flags);
- storageDriverLock(driver);
- virStoragePoolObjLock(pool);
- if (origpool)
- virStoragePoolObjLock(origpool);
- storageDriverUnlock(driver);
-
- origvol->building = 0;
- newvol->building = 0;
newvol = NULL;
- pool->asyncjobs--;
- if (origpool) {
- origpool->asyncjobs--;
- virStoragePoolObjUnlock(origpool);
- origpool = NULL;
- }
+ if (origpool)
+ storageEndJobWithPool(driver, origpool, origvol);
+ else
+ storageEndJob(driver, origvol);
+endjob:
+ storageEndJobWithPool(driver, pool, newvol);
if (buildret < 0) {
virStoragePoolObjUnlock(pool);
@@ -1569,6 +1666,23 @@ cleanup:
return ret;
}
+struct storageVolumeStreamData {
+ virStorageDriverStatePtr driver;
+ virStorageVolDefPtr vol;
+};
+
+static void
+storageVolumeStreamEndJob(virStreamPtr stream ATTRIBUTE_UNUSED,
+ void *opaque)
+{
+ struct storageVolumeStreamData *data = opaque;
+ virStorageDriverStatePtr driver = data->driver;
+ virStorageVolDefPtr vol = data->vol;
+
+ VIR_FREE(data);
+
+ storageEndJob(driver, vol);
+}
static int
storageVolumeDownload(virStorageVolPtr obj,
@@ -1580,6 +1694,7 @@ storageVolumeDownload(virStorageVolPtr obj,
virStorageDriverStatePtr driver = obj->conn->storagePrivateData;
virStoragePoolObjPtr pool = NULL;
virStorageVolDefPtr vol = NULL;
+ struct storageVolumeStreamData *stream_data = NULL;
int ret = -1;
virCheckFlags(0, -1);
@@ -1609,21 +1724,34 @@ storageVolumeDownload(virStorageVolPtr obj,
goto out;
}
- if (vol->building) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
- _("volume '%s' is still being
allocated."),
- vol->name);
+ if (storageBeginJob(driver, vol, VIR_STORAGE_VOL_JOB_DOWNLOAD) < 0)
goto out;
- }
if (virFDStreamOpenFile(stream,
vol->target.path,
offset, length,
O_RDONLY) < 0)
- goto out;
+ goto endjob;
+
+ if (VIR_ALLOC(stream_data) < 0) {
+ virReportOOMError();
+ goto endjob;
+ }
+
+ stream_data->driver = driver;
+ stream_data->vol = vol;
+
+ if (virFDStreamSetInternalCloseCb(stream,
+ storageVolumeStreamEndJob,
+ stream_data, NULL) < 0)
+ goto endjob;
ret = 0;
+ goto out;
+endjob:
+ storageEndJob(driver, vol);
+ VIR_FREE(stream_data);
out:
if (pool)
virStoragePoolObjUnlock(pool);
@@ -1642,6 +1770,7 @@ storageVolumeUpload(virStorageVolPtr obj,
virStorageDriverStatePtr driver = obj->conn->storagePrivateData;
virStoragePoolObjPtr pool = NULL;
virStorageVolDefPtr vol = NULL;
+ struct storageVolumeStreamData *stream_data = NULL;
int ret = -1;
virCheckFlags(0, -1);
@@ -1671,12 +1800,8 @@ storageVolumeUpload(virStorageVolPtr obj,
goto out;
}
- if (vol->building) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
- _("volume '%s' is still being
allocated."),
- vol->name);
+ if (storageBeginJob(driver, vol, VIR_STORAGE_VOL_JOB_DOWNLOAD) < 0)
goto out;
- }
/* Not using O_CREAT because the file is required to
* already exist at this point */
@@ -1684,10 +1809,27 @@ storageVolumeUpload(virStorageVolPtr obj,
vol->target.path,
offset, length,
O_WRONLY) < 0)
- goto out;
+ goto endjob;
+
+ if (VIR_ALLOC(stream_data) < 0) {
+ virReportOOMError();
+ goto endjob;
+ }
+
+ stream_data->driver = driver;
+ stream_data->vol = vol;
+
+ if (virFDStreamSetInternalCloseCb(stream,
+ storageVolumeStreamEndJob,
+ stream_data, NULL) < 0)
+ goto endjob;
ret = 0;
+ goto out;
+endjob:
+ storageEndJob(driver, vol);
+ VIR_FREE(stream_data);
out:
if (pool)
virStoragePoolObjUnlock(pool);
@@ -1737,13 +1879,6 @@ storageVolumeResize(virStorageVolPtr obj,
goto out;
}
- if (vol->building) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
- _("volume '%s' is still being
allocated."),
- vol->name);
- goto out;
- }
-
if (flags & VIR_STORAGE_VOL_RESIZE_DELTA) {
abs_capacity = vol->capacity + capacity;
flags &= ~VIR_STORAGE_VOL_RESIZE_DELTA;
@@ -1771,9 +1906,14 @@ storageVolumeResize(virStorageVolPtr obj,
goto out;
}
+ if (storageBeginJobWithPool(driver, pool, vol, VIR_STORAGE_VOL_JOB_RESIZE) < 0)
+ goto out;
+
if (backend->resizeVol(obj->conn, pool, vol, abs_capacity, flags) < 0)
goto out;
+ storageEndJobWithPool(driver, pool, vol);
+
vol->capacity = abs_capacity;
ret = 0;
@@ -2028,17 +2168,16 @@ storageVolumeWipePattern(virStorageVolPtr obj,
goto out;
}
- if (vol->building) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
- _("volume '%s' is still being
allocated."),
- vol->name);
+ if (storageBeginJobWithPool(driver, pool, vol, VIR_STORAGE_VOL_JOB_WIPE) < 0)
goto out;
- }
if (storageVolumeWipeInternal(vol, algorithm) == -1) {
+ storageEndJobWithPool(driver, pool, vol);
goto out;
}
+ storageEndJobWithPool(driver, pool, vol);
+
ret = 0;
out:
@@ -2066,6 +2205,7 @@ storageVolumeDelete(virStorageVolPtr obj,
virStorageVolDefPtr vol = NULL;
unsigned int i;
int ret = -1;
+ int deleteVolRet;
storageDriverLock(driver);
pool = virStoragePoolObjFindByName(&driver->pools, obj->pool);
@@ -2095,13 +2235,6 @@ storageVolumeDelete(virStorageVolPtr obj,
goto cleanup;
}
- if (vol->building) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
- _("volume '%s' is still being
allocated."),
- vol->name);
- goto cleanup;
- }
-
if (!backend->deleteVol) {
virStorageReportError(VIR_ERR_NO_SUPPORT,
"%s", _("storage pool does not support vol
deletion"));
@@ -2109,7 +2242,14 @@ storageVolumeDelete(virStorageVolPtr obj,
goto cleanup;
}
- if (backend->deleteVol(obj->conn, pool, vol, flags) < 0)
+ if (storageBeginJobWithPool(driver, pool, vol,
+ VIR_STORAGE_VOL_JOB_DELETE) < 0)
+ goto cleanup;
+
+ deleteVolRet = backend->deleteVol(obj->conn, pool, vol, flags);
+
+ storageEndJobWithPool(driver, pool, vol);
+ if (deleteVolRet < 0)
goto cleanup;
for (i = 0 ; i < pool->volumes.count ; i++) {
--
1.7.8.5
9:35 a.m.
New subject: [libvirt] [PATCH 4/4] storage: Implement virStorageVolAbortJob
This implies breaking up some jobs into cycles during which
we check for job abortion request. The virStorageVolAbortJob
API will then just set request and wait until job is released.
If a job was, however, interrupted it should fail with
VIR_ERR_OPERATION_ABORTED error.
---
src/conf/storage_conf.h | 7 ++
src/libvirt_private.syms | 3 +
src/storage/storage_backend.c | 53 +++++++++-------
src/storage/storage_backend_fs.c | 8 ++-
src/storage/storage_driver.c | 128 ++++++++++++++++++++++++++++++++-----
src/storage/storage_driver.h | 7 ++
6 files changed, 162 insertions(+), 44 deletions(-)
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 481c806..ee25e34 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -28,6 +28,7 @@
# include "util.h"
# include "storage_encryption_conf.h"
# include "threads.h"
+# include "command.h"
# include <libxml/tree.h>
@@ -100,6 +101,12 @@ struct virStorageVolJobObj {
virCond cond;
enum virStorageVolJob active;
unsigned long long start;
+
+ bool abort_requested; /* abort was requested */
+ virCommandPtr cmd; /* if we are running external program,
+ store pointer here too, so we can
+ virCommandAbort it if necessary */
+ virStreamPtr stream; /* stream to abort */
};
typedef struct _virStorageVolDef virStorageVolDef;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fef9d5a..cec7a94 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1017,6 +1017,9 @@ virStorageVolDefParseFile;
virStorageVolDefParseNode;
virStorageVolDefParseString;
+# storage_driver.h
+virStorageVolJobSetCommand;
+virStorageVolJobSetStream;
# storage_encryption_conf.h
virStorageEncryptionFormat;
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index caac2f8..2e82c03 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -53,6 +53,7 @@
#include "internal.h"
#include "secret_conf.h"
#include "uuid.h"
+#include "storage_driver.h"
#include "storage_file.h"
#include "storage_backend.h"
#include "logging.h"
@@ -106,8 +107,6 @@ static virStorageBackendPtr backends[] = {
NULL
};
-static int track_allocation_progress = 0;
-
enum {
TOOL_QEMU_IMG,
TOOL_KVM_IMG,
@@ -202,6 +201,15 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
goto cleanup;
}
+
+ if (vol->job.abort_requested ||
+ inputvol->job.abort_requested) {
+ virStorageReportError(VIR_ERR_OPERATION_ABORTED, "%s",
+ _("Job abort requested"));
+ ret = -1;
+ goto cleanup;
+ }
+
} while ((amtleft -= interval) > 0);
}
@@ -325,34 +333,31 @@ createRawFile(int fd, virStorageVolDefPtr vol,
}
if (remain) {
- if (track_allocation_progress) {
+ while (remain) {
+ /* Allocate in chunks of 512MiB: big-enough chunk
+ * size and takes approx. 9s on ext3. A progress
+ * update every 9s is a fair-enough trade-off
+ */
+ unsigned long long bytes = 512 * 1024 * 1024;
- while (remain) {
- /* Allocate in chunks of 512MiB: big-enough chunk
- * size and takes approx. 9s on ext3. A progress
- * update every 9s is a fair-enough trade-off
- */
- unsigned long long bytes = 512 * 1024 * 1024;
-
- if (bytes > remain)
- bytes = remain;
- if (safezero(fd, vol->allocation - remain, bytes) < 0) {
- ret = -errno;
- virReportSystemError(errno, _("cannot fill file
'%s'"),
- vol->target.path);
- goto cleanup;
- }
- remain -= bytes;
- }
- } else { /* No progress bars to be shown */
- if (safezero(fd, 0, remain) < 0) {
+ if (bytes > remain)
+ bytes = remain;
+ if (safezero(fd, vol->allocation - remain, bytes) < 0) {
ret = -errno;
virReportSystemError(errno, _("cannot fill file
'%s'"),
vol->target.path);
goto cleanup;
}
- }
+ remain -= bytes;
+ if (vol->job.abort_requested) {
+ virStorageReportError(VIR_ERR_OPERATION_ABORTED,
+ "%s",
+ _("Job abort requested"));
+ ret = -1;
+ goto cleanup;
+ }
+ }
}
if (fsync(fd) < 0) {
@@ -782,6 +787,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
goto cleanup;
cmd = virCommandNew(create_tool);
+ virStorageVolJobSetCommand(vol, cmd);
if (inputvol) {
virCommandAddArgList(cmd, "convert", "-f", inputType,
"-O", type,
@@ -898,6 +904,7 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn
ATTRIBUTE_UNUSED,
}
cmd = virCommandNewArgList("qcow-create", size, vol->target.path,
NULL);
+ virStorageVolJobSetCommand(vol, cmd);
ret = virStorageBackendCreateExecCommand(pool, vol, cmd);
virCommandFree(cmd);
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 1af12e6..2d16be5 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -44,6 +44,7 @@
#include "virterror_internal.h"
#include "storage_backend_fs.h"
#include "storage_conf.h"
+#include "storage_driver.h"
#include "storage_file.h"
#include "command.h"
#include "memory.h"
@@ -1188,7 +1189,7 @@ virStorageBackendFileSystemVolRefresh(virConnectPtr conn,
}
static int
-virStorageBackendFilesystemResizeQemuImg(const char *path,
+virStorageBackendFilesystemResizeQemuImg(virStorageVolDefPtr vol,
unsigned long long capacity)
{
int ret = -1;
@@ -1207,7 +1208,8 @@ virStorageBackendFilesystemResizeQemuImg(const char *path,
}
cmd = virCommandNew(img_tool);
- virCommandAddArgList(cmd, "resize", path, NULL);
+ virStorageVolJobSetCommand(vol, cmd);
+ virCommandAddArgList(cmd, "resize", vol->target.path, NULL);
virCommandAddArgFormat(cmd, "%llu", capacity);
ret = virCommandRun(cmd, NULL);
@@ -1233,7 +1235,7 @@ virStorageBackendFileSystemVolResize(virConnectPtr conn
ATTRIBUTE_UNUSED,
if (vol->target.format == VIR_STORAGE_FILE_RAW)
return virStorageFileResize(vol->target.path, capacity);
else
- return virStorageBackendFilesystemResizeQemuImg(vol->target.path,
+ return virStorageBackendFilesystemResizeQemuImg(vol,
capacity);
}
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 7f3dfcd..7e11c74 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -72,6 +72,9 @@ storageResetJob(virStorageVolDefPtr vol)
job->active = VIR_STORAGE_VOL_JOB_NONE;
job->start = 0;
+ job->abort_requested = false;
+ job->cmd = NULL;
+ job->stream = NULL;
}
/* wait max. 30 seconds for job ackquire */
@@ -92,11 +95,17 @@ storageBeginJobInternal(virStorageDriverStatePtr driver
ATTRIBUTE_UNUSED,
then = now + STORAGE_JOB_WAIT_TIME;
- while (vol->job.active != VIR_STORAGE_VOL_JOB_NONE) {
+ while (vol->job.active != VIR_STORAGE_VOL_JOB_NONE &&
+ !vol->job.abort_requested) {
if (virCondWaitUntil(&vol->job.cond, &vol->lock, then) < 0)
goto error;
}
+ if (vol->job.abort_requested) {
+ VIR_DEBUG("JobAbort requested. Not starting a job");
+ return -1;
+ }
+
VIR_DEBUG("Starting job %d", job);
storageResetJob(vol);
vol->job.active = job;
@@ -170,6 +179,24 @@ storageEndJobWithPool(virStorageDriverStatePtr driver,
return storageEndJobInternal(driver, pool, true, vol);
}
+void virStorageVolJobSetCommand(virStorageVolDefPtr vol,
+ virCommandPtr cmd)
+{
+ if (!vol || !cmd)
+ return;
+
+ vol->job.cmd = cmd;
+}
+
+void virStorageVolJobSetStream(virStorageVolDefPtr vol,
+ virStreamPtr stream)
+{
+ if (!vol || !stream)
+ return;
+
+ vol->job.stream = stream;
+}
+
static void
storageDriverAutostart(virStorageDriverStatePtr driver) {
unsigned int i;
@@ -1456,35 +1483,19 @@ storageVolumeCreateXML(virStoragePoolPtr obj,
if (backend->buildVol) {
int buildret;
- virStorageVolDefPtr buildvoldef = NULL;
-
- if (VIR_ALLOC(buildvoldef) < 0) {
- virReportOOMError();
- voldef = NULL;
- goto cleanup;
- }
-
- /* Make a shallow copy of the 'defined' volume definition, since the
- * original allocation value will change as the user polls 'info',
- * but we only need the initial requested values
- */
- memcpy(buildvoldef, voldef, sizeof(*voldef));
/* Drop the pool lock during volume allocation */
if (storageBeginJobWithPool(driver, pool, voldef,
VIR_STORAGE_VOL_JOB_BUILD) < 0) {
- VIR_FREE(buildvoldef);
goto cleanup;
}
- buildret = backend->buildVol(obj->conn, pool, buildvoldef);
+ buildret = backend->buildVol(obj->conn, pool, voldef);
storageEndJobWithPool(driver, pool, voldef);
voldef = NULL;
- VIR_FREE(buildvoldef);
-
if (buildret < 0) {
virStoragePoolObjUnlock(pool);
storageVolumeDelete(volobj, 0);
@@ -1727,6 +1738,8 @@ storageVolumeDownload(virStorageVolPtr obj,
if (storageBeginJob(driver, vol, VIR_STORAGE_VOL_JOB_DOWNLOAD) < 0)
goto out;
+ virStorageVolJobSetStream(vol, stream);
+
if (virFDStreamOpenFile(stream,
vol->target.path,
offset, length,
@@ -1803,6 +1816,8 @@ storageVolumeUpload(virStorageVolPtr obj,
if (storageBeginJob(driver, vol, VIR_STORAGE_VOL_JOB_DOWNLOAD) < 0)
goto out;
+ virStorageVolJobSetStream(vol, stream);
+
/* Not using O_CREAT because the file is required to
* already exist at this point */
if (virFDStreamOpenFile(stream,
@@ -2007,6 +2022,13 @@ storageWipeExtent(virStorageVolDefPtr vol,
*bytes_wiped += written;
remaining -= written;
+
+ if (vol->job.abort_requested) {
+ virStorageReportError(VIR_ERR_OPERATION_ABORTED, "%s",
+ _("Job abort requested"));
+ ret = -1;
+ goto out;
+ }
}
if (fdatasync(fd) < 0) {
@@ -2088,6 +2110,7 @@ storageVolumeWipeInternal(virStorageVolDefPtr def,
algorithm);
}
cmd = virCommandNew(SCRUB);
+ virStorageVolJobSetCommand(def, cmd);
virCommandAddArgList(cmd, "-f", "-p", alg_char,
def->target.path, NULL);
@@ -2426,6 +2449,74 @@ cleanup:
return ret;
}
+static int
+storageVolAbortJob(virStorageVolPtr obj,
+ unsigned int flags)
+{
+ virStorageDriverStatePtr driver = obj->conn->storagePrivateData;
+ virStoragePoolObjPtr pool;
+ virStorageVolDefPtr vol;
+ int ret = -1;
+
+ virCheckFlags(0, -1);
+
+ storageDriverLock(driver);
+ pool = virStoragePoolObjFindByName(&driver->pools, obj->pool);
+ storageDriverUnlock(driver);
+ if (!pool) {
+ virStorageReportError(VIR_ERR_NO_STORAGE_POOL,
+ "%s", _("no storage pool with matching
uuid"));
+ goto cleanup;
+ }
+
+ if (!virStoragePoolObjIsActive(pool)) {
+ virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("storage pool is not
active"));
+ goto cleanup;
+ }
+
+ vol = virStorageVolDefFindByName(pool, obj->name);
+
+ if (!vol) {
+ virStorageReportError(VIR_ERR_NO_STORAGE_VOL,
+ _("no storage vol with matching name
'%s'"),
+ obj->name);
+ goto cleanup;
+ }
+
+ if (vol->job.active == VIR_STORAGE_VOL_JOB_NONE) {
+ virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ _("there is no job running on '%s'"),
+ obj->name);
+ goto cleanup;
+ }
+
+ vol->job.abort_requested = true;
+ virStoragePoolObjUnlock(pool);
+
+ /* If we use external program kill it */
+ virCommandAbort(vol->job.cmd);
+ if (vol->job.stream)
+ virStreamAbort(vol->job.stream);
+
+ while (vol->job.active) {
+ if (virCondWait(&vol->job.cond, &vol->lock) < 0) {
+ virReportSystemError(errno, "%s",
+ _("unable to wait on condition"));
+ goto cleanup;
+ }
+ }
+
+ virStoragePoolObjLock(pool);
+
+ ret = 0;
+
+cleanup:
+ if (pool)
+ virStoragePoolObjUnlock(pool);
+ return ret;
+}
+
static virStorageDriver storageDriver = {
.name = "storage",
.open = storageOpen, /* 0.4.0 */
@@ -2467,6 +2558,7 @@ static virStorageDriver storageDriver = {
.volGetXMLDesc = storageVolumeGetXMLDesc, /* 0.4.0 */
.volGetPath = storageVolumeGetPath, /* 0.4.0 */
.volResize = storageVolumeResize, /* 0.9.10 */
+ .volAbortJob = storageVolAbortJob, /* 0.9.11 */
.poolIsActive = storagePoolIsActive, /* 0.7.3 */
.poolIsPersistent = storagePoolIsPersistent, /* 0.7.3 */
diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h
index b3e2554..2220c90 100644
--- a/src/storage/storage_driver.h
+++ b/src/storage/storage_driver.h
@@ -25,7 +25,14 @@
# define __VIR_STORAGE_DRIVER_H__
# include "storage_conf.h"
+# include "command.h"
+# include "fdstream.h"
int storageRegister(void);
+void virStorageVolJobSetCommand(virStorageVolDefPtr vol,
+ virCommandPtr cmd);
+void virStorageVolJobSetStream(virStorageVolDefPtr vol,
+ virStreamPtr stream);
+
#endif /* __VIR_STORAGE_DRIVER_H__ */
--
1.7.8.5
4637
days inactive
4637
days old
6 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Michal Privoznik