On 11/20/12 19:47, Michal Privoznik wrote:
using qemu guest agent. As said in previous patch,
@mountPoint must be NULL and @flags zero because
qemu guest agent doesn't support these arguments
yet. If qemu learns them, we can start supporting
them as well.
---
src/qemu/qemu_agent.c | 25 ++++++++++++++
src/qemu/qemu_agent.h | 2 +
src/qemu/qemu_driver.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 110 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index ab6dc22..6f517c9 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1448,3 +1448,28 @@ qemuAgentArbitraryCommand(qemuAgentPtr mon,
virJSONValueFree(reply);
return ret;
}
+
+int
+qemuAgentFSTrim(qemuAgentPtr mon,
+ unsigned long long minimum)
+{
+ int ret = -1;
+ virJSONValuePtr cmd;
+ virJSONValuePtr reply = NULL;
+
+ cmd = qemuAgentMakeCommand("guest-fstrim",
+ "U:minimum", minimum,
+ NULL);
+ if (!cmd)
+ return ret;
+
+ ret = qemuAgentCommand(mon, cmd, &reply,
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK);
I didn't check the full call graph of qemuAgentCommand, but most of the
paths already contain error reporting ... (relevance of this command
will be revealed later :))
+
+ if (reply && ret == 0)
+ ret = qemuAgentCheckError(cmd, reply);
+
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+ return ret;
+}
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index c62c438..dad068b 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -83,4 +83,6 @@ int qemuAgentArbitraryCommand(qemuAgentPtr mon,
const char *cmd,
char **result,
int timeout);
+int qemuAgentFSTrim(qemuAgentPtr mon,
+ unsigned long long minimum);
#endif /* __QEMU_AGENT_H__ */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 595c452..3dc5bdd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14733,6 +14733,88 @@ cleanup:
return result;
}
+static int
+qemuDomainFSTrim(virDomainPtr dom,
+ const char *mountPoint,
+ unsigned long long minimum,
+ unsigned int flags)
+{
+ struct qemud_driver *driver = dom->conn->privateData;
+ virDomainObjPtr vm;
+ int ret = -1;
+ qemuDomainObjPrivatePtr priv;
+
+ virCheckFlags(0, -1);
+
+ if (mountPoint) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("Specifying mount point "
+ "is not supported for now"));
+ return -1;
+ }
+
+ qemuDriverLock(driver);
+ vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+ qemuDriverUnlock(driver);
+
+ if (!vm) {
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ virReportError(VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"), uuidstr);
+ goto cleanup;
+ }
You can avoid the two blocks of code above using the
qemuDomObjFromDomain() helper that does that for you.
+
+ priv = vm->privateData;
+
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+ goto cleanup;
+ }
+
+ if (priv->agentError) {
+ virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
+ _("QEMU guest agent is not "
+ "available due to an error"));
+ goto cleanup;
+ }
+
+ if (!priv->agent) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("QEMU guest agent is not configured"));
+ goto cleanup;
+ }
I'd swap the two blocks above, it makes more sense to check if there's
an agent error after you check for the agent.
+
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ goto cleanup;
+
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+ goto endjob;
+ }
+
+ qemuDomainObjEnterAgent(driver, vm);
+ ret = qemuAgentFSTrim(priv->agent, minimum);
+ qemuDomainObjExitAgent(driver, vm);
+ if (ret < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to execute agent command"));
+ goto endjob;
(the relevant part from the comment up)
... this masks the error from the underlying helper function. The goto
statement is redundant here.
+ }
+
+endjob:
+ if (qemuDomainObjEndJob(driver, vm) == 0) {
block quotes are not needed here
+ vm = NULL;
+ }
+
+cleanup:
+ if (vm)
+ virDomainObjUnlock(vm);
+ return ret;
+}
+
static virDriver qemuDriver = {
.no = VIR_DRV_QEMU,
.name = QEMU_DRIVER_NAME,
@@ -14906,6 +14988,7 @@ static virDriver qemuDriver = {
.nodeGetMemoryParameters = nodeGetMemoryParameters, /* 0.10.2 */
.nodeSetMemoryParameters = nodeSetMemoryParameters, /* 0.10.2 */
.nodeGetCPUMap = nodeGetCPUMap, /* 1.0.0 */
+ .domainFSTrim = qemuDomainFSTrim, /* 1.0.1 */
};
I don't like masking errors from underlying layers. I'd rather have it
changed so that more specific errors are shown but it's not a hard
requirement if you think it's not needed.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list