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

Re: [libvirt] RFE: virsh list improvement (--description and --details)



On Mon, May 08, 2017 at 09:16:26AM +0200, Przemysław Sztoch wrote:
> Dear Michal and Pavel,
> 
> We cover about 100 clients who have their own and simple CentOS KVM 
> installations.
> Their technical skills are far from writing python scripts. They expect 
> simple solutions.

Libvirt is an library that provides unified API to manage different
hypervisors.  Virsh is a simple command line tool to manage one host,
it's not an advanced management application.

> Talking to our helpdesk, I found that 70% of libvirt and virtualization 
> problems are:
> A) lack of autostart activation on critical guests; then occasional 
> failures and reboots affect lack of automatic startup of key services,
> B) frequent overcommiting of allocated virtual processors and memory due 
> to the lack of basic planning and addition operations of local admin 
> stuff :-(,
> C) misconfiguration of qemu-agent, which affects many problems with safe 
> restart, snapshot, backup, etc. (the "Time" column is a perfect 
> diagnostic here)
> D) leaving unnecessary snapshots that lie unused after many months,
> E) live migration attempts that fail to put domain in a transient mode 
> leave the guests disappearing in unexplained circumstances after kvm 
> host restart :-)
> 
> Virtually all the above problems of everyday life, our helpdesk is now 
> able to diagnose by command:
> virsh list --details --managed-save
> By the way, they can easily update the documentation with one compact list.
> 
> I do not understand your dislike for the proposed changes. All the 
> members of our team and teams of our partners have been very 
> enthusiastic about the new functionality.
> You govern, so you have to decide. ;-)

My dislike is that virsh isn't a management application, it's more like
a shell for virtualization.  All the listed problems are things, that
should be handled by management application build on top of libvirt.
This is a common misuse of libvirt.

For simple workstation usage there is virt-manager.  For advanced
management tasks there are different type of applications, see
<http://libvirt.org/apps.html>.

To address the patch itself, as it was already stated by Peter, instead
of adding --details there is command line tool "virt-top" and adding
--description isn't a good idea, because it will break the simple table
printed by "virsh list".  Abusing "virsh list" to provide statistics and
detailed information about domains isn't a way to go.

Pavel

> 
> Przemyslaw Sztoch

> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 0ca53e4..1c3ec37 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -1769,6 +1769,14 @@ static const vshCmdOptDef opts_list[] = {
>       .type = VSH_OT_BOOL,
>       .help = N_("show domain title")
>      },
> +    {.name = "description",
> +     .type = VSH_OT_BOOL,
> +     .help = N_("show domain description")
> +    },
> +    {.name = "details",
> +     .type = VSH_OT_BOOL,
> +     .help = N_("show domain details")
> +    },
>      {.name = NULL}
>  };
>  
> @@ -1780,6 +1788,8 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
>  {
>      bool managed = vshCommandOptBool(cmd, "managed-save");
>      bool optTitle = vshCommandOptBool(cmd, "title");
> +    bool optDescription = vshCommandOptBool(cmd, "description");
> +    bool optDetails = vshCommandOptBool(cmd, "details");
>      bool optTable = vshCommandOptBool(cmd, "table");
>      bool optUUID = vshCommandOptBool(cmd, "uuid");
>      bool optName = vshCommandOptBool(cmd, "name");
> @@ -1822,6 +1832,9 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
>  
>      VSH_EXCLUSIVE_OPTIONS("table", "name");
>      VSH_EXCLUSIVE_OPTIONS("table", "uuid");
> +    VSH_EXCLUSIVE_OPTIONS("description", "title");
> +    VSH_EXCLUSIVE_OPTIONS("details", "title");
> +    VSH_EXCLUSIVE_OPTIONS("details", "description");
>  
>      if (!optUUID && !optName)
>          optTable = true;
> @@ -1831,9 +1844,19 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
>  
>      /* print table header in legacy mode */
>      if (optTable) {
> -        if (optTitle)
> +        if (optTitle || optDescription)
>              vshPrintExtra(ctl, " %-5s %-30s %-10s %-20s\n%s\n",
> -                          _("Id"), _("Name"), _("State"), _("Title"),
> +                          _("Id"), _("Name"), _("State"),
> +                          optTitle ? _("Title") : _("Description"),
> +                          "-----------------------------------------"
> +                          "-----------------------------------------");
> +        else if (optDetails)
> +            vshPrintExtra(ctl, " %-5s %-30s %-10s %-13s %-13s %5s %9s %9s %s\n%s\n",
> +                          _("Id"), _("Name"), _("State"), 
> +                          _("Autostart"), _("Persistent"),
> +                          _("vCPU"), _("Memory"), _("Snapshots"),
> +                          _("Time"),
> +                          "-----------------------------------------"
>                            "-----------------------------------------"
>                            "-----------------------------------------");
>          else
> @@ -1862,8 +1885,8 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
>                  virDomainHasManagedSaveImage(dom, 0) > 0)
>                  state = -2;
>  
> -            if (optTitle) {
> -                if (!(title = virshGetDomainDescription(ctl, dom, true, 0)))
> +            if (optTitle || optDescription) {
> +                if (!(title = virshGetDomainDescription(ctl, dom, optTitle, 0)))
>                      goto cleanup;
>  
>                  vshPrint(ctl, " %-5s %-30s %-10s %-20s\n", id_buf,
> @@ -1873,6 +1896,60 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
>                           title);
>  
>                  VIR_FREE(title);
> +            } else if (optDetails) {
> +                int autostart;
> +                int persistent;
> +                int vcpu = -1;
> +                int memory = -1;
> +                int nsnap;
> +                long long seconds = 0;
> +                unsigned int nseconds = 0;
> +
> +                virDomainInfo info;
> +
> +                if (virDomainGetAutostart(dom, &autostart) < 0) {
> +                    autostart = -1;
> +                }
> +                persistent = virDomainIsPersistent(dom);
> +
> +                if (virDomainGetInfo(dom, &info) == 0) {
> +                    vcpu = info.nrVirtCpu;
> +                    memory = info.memory / 1024;
> +                }
> +
> +                nsnap = virDomainSnapshotNum(dom, 0);
> +
> +                char timestr[100];
> +                if (state == VIR_DOMAIN_RUNNING) {
> +                        if (virDomainGetTime(dom, &seconds, &nseconds, 0) < 0) {
> +                        strcpy(timestr, _("unknown"));
> +                    } else {
> +                        time_t cur_time = seconds;
> +                        struct tm time_info;
> +
> +                        if (!gmtime_r(&cur_time, &time_info)) {
> +                                vshError(ctl, _("Unable to format time"));
> +                                goto cleanup;
> +                    }
> +                        strftime(timestr, sizeof(timestr), "%Y-%m-%d %H:%M:%S", &time_info);
> +                    }
> +                } else {
> +                    strcpy(timestr, "");
> +                }
> +
> +                vshPrint(ctl, " %-5s %-30s %-10s %-13s %-13s %5d %9d %9d %s\n", id_buf,
> +                         virDomainGetName(dom),
> +                         state == -2 ? _("saved")
> +                         : virshDomainStateToString(state),
> +                         autostart == 1 ? _("yes") :
> +                         autostart == 0 ? _("no") : _("unknown"),
> +                         persistent == 1 ? _("yes") :
> +                         persistent == 0 ? _("no") : _("unknown"),
> +                         vcpu, memory,
> +                         nsnap,
> +                         timestr
> +                         );
> +
>              } else {
>                  vshPrint(ctl, " %-5s %-30s %s\n", id_buf,
>                           virDomainGetName(dom),

> --
> libvir-list mailing list
> libvir-list redhat com
> https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: Digital signature


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