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@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
