On Wed, Nov 28, 2012 at 11:56:23AM +0100, Peter Krempa wrote:
> 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;
> >+
> >+cleanup:
> >+ 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.
Agreed, we should not cripple virsh to suit a particular libvirtd
feature set, because you have to bear in mind that we have an RPC
system that allows an old virsh to talk to a new libvirtd.
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list