[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 4/4] virsh: Expose new virDomainFSTrim API

On 11/20/12 19:47, Michal Privoznik wrote:
under domfstrim command. Since API doesn't support all
parameters (some must be NULL or zero), don't expose
these in virsh neither.
  tools/virsh-domain.c |   40 ++++++++++++++++++++++++++++++++++++++++
  tools/virsh.pod      |   12 ++++++++++++
  2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index cc47383..4ea08f4 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8300,6 +8300,45 @@ cleanup:
      return ret;

+static const vshCmdInfo info_domfstrim[] = {
+    {"help", N_("Invoke fstrim on domain's mounted filesystems.")},
+    {"desc", N_("Invoke fstrim on domain's mounted filesystems.")},
+    {NULL, NULL}
+static const vshCmdOptDef opts_domfstrim[] = {
+    {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
+    {"minimum", VSH_OT_INT, 0, N_("Just a hint to ignore contiguous "
+                                  "free ranges smaller than this (Bytes)")},

Wouldn't it make sense to expose the mount point argument right now? I know it's not implemented yet, but when somebody does implement that we will need to change this later.

+    {NULL, 0, 0, NULL}
+static bool
+cmdDomFSTrim(vshControl *ctl, const vshCmd *cmd)
+    virDomainPtr dom = NULL;
+    bool ret = false;
+    unsigned long long minimum = 0;
+    unsigned int flags = 0;
+    if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+        goto cleanup;
+    if (vshCommandOptULongLong(cmd, "minimum", &minimum) < 0) {
+        vshError(ctl, _("Unable to parse integer parameter"));

As you know what's the name of the parameter print it in the error message so the user is not left guessing where the error is.

+        goto cleanup;
+    }
+    if (virDomainFSTrim(dom, NULL, minimum, flags) < 0) {
+        vshError(ctl, _("Unable to invoke fstrim"));
+        goto cleanup;
+    }
+    ret = true;
+    return ret;
  const vshCmdDef domManagementCmds[] = {
      {"attach-device", cmdAttachDevice, opts_attach_device,
       info_attach_device, 0},
@@ -8332,6 +8371,7 @@ const vshCmdDef domManagementCmds[] = {
      {"detach-interface", cmdDetachInterface, opts_detach_interface,
       info_detach_interface, 0},
      {"domdisplay", cmdDomDisplay, opts_domdisplay, info_domdisplay, 0},
+    {"domfstrim", cmdDomFSTrim, opts_domfstrim, info_domfstrim, 0},
      {"domhostname", cmdDomHostname, opts_domhostname, info_domhostname, 0},
      {"domid", cmdDomid, opts_domid, info_domid, 0},
      {"domif-setlink", cmdDomIfSetLink, opts_domif_setlink, info_domif_setlink, 0},
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 29be39e..c3fdcb7 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -878,6 +878,18 @@ Output a URI which can be used to connect to the graphical display of the
  domain via VNC, SPICE or RDP. If I<--include-password> is specified, the
  SPICE channel password will be included in the URI.

+=item B<domfstrim> I<domain> [I<--minimum> B<bytes>]
+Issue a fstrim command on all mounted filesystems within a running
+domain. It discards blocks which are not in use by the filesystem.
+If I<--minimum> B<bytes> is specified, it tells guest kernel length
+of contiguous free range. Smaller than this may be ignored (this is
+a hint and the guest may not respect it). By increasing this value,
+the fstrim operation will complete more quickly for filesystems
+with badly fragmented free space, although not all blocks will
+be discarded.  The default value is zero, meaning "discard
+every free block".
  =item B<domhostname> I<domain>

  Returns the hostname of a domain, if the hypervisor makes it available.

I'd rather see this with the ability to specify mountpoint to the api even if it still isn't supported.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]