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

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



> On 4 May 2017, at 17:03, Michal Privoznik <mprivozn redhat com> wrote:
>> 2. Please add new columns to virsh list (for —details):
>> a) CPUs
>> b) MaxMem
>> c) Flags: p-persistent, t-transient, a-autostart, m-managed save, s-with snapshot(s)
>> and total sum for column vCPUs and Mem.
>> 
>> In my opinion it is very popular use case. It will help you control vCPU and memory overcommiting in very simple and fast way.
>> Put flags on list will prevents many additional commands from being executed. You will get all important information in one place.
> 
> Frankly, this one looks at the edge of the scope of virsh list. I mean,
> I view virsh as a simple CLI utility that you can build your own scripts
> on the top of.
> Also, you want 5 flags. Cool. But tomorrow somebody else wants another 5
> (title, description, has given device, etc.). The possibilities are
> endless. I suggest writing your own management application on the top of
> libvirt.

In my opinion you are wrong. It is very natural place for more information about your domains.
Of course, everybody can build his own scripts, but bash script will be too much slow and too much complicated for typical tech stuff.

My patch is very simple:
virsh list —details

Please, audit attached diff, because I have not any skill about C:

index 0ca53e4..c98160d 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,7 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
 
     VSH_EXCLUSIVE_OPTIONS("table", "name");
     VSH_EXCLUSIVE_OPTIONS("table", "uuid");
+    VSH_EXCLUSIVE_OPTIONS("title", "description");
 
     if (!optUUID && !optName)
         optTable = true;
@@ -1836,6 +1847,18 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
                           _("Id"), _("Name"), _("State"), _("Title"),
                           "-----------------------------------------"
                           "-----------------------------------------");
+        else if (optDescription)
+            vshPrintExtra(ctl, " %-5s %-30s %-10s %-20s\n%s\n",
+                          _("Id"), _("Name"), _("State"), _("Description"),
+                          "-----------------------------------------"
+                          "-----------------------------------------");
+        else if (optDetails)
+            vshPrintExtra(ctl, " %-5s %-30s %-10s %5s %6s %9s %-5s %-20s\n%s\n",
+                          _("Id"), _("Name"), _("State"), _("vCPU"), 
+                          _("Memory"), _("Snapshots"), _("Flags"), _("Time"),
+                          "-----------------------------------------"
+                          "-----------------------------------------"
+                          "----------------------");
         else
             vshPrintExtra(ctl, " %-5s %-30s %s\n%s\n",
                           _("Id"), _("Name"), _("State"),
@@ -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,62 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
                          title);
 
                 VIR_FREE(title);
+            } else if (optDetails) {
+        	int vcpu = -1;
+        	int memory = -1;
+		int persistent;
+		int autostart;
+		int nsnap;
+		int mansave;
+		long long seconds = 0;
+		unsigned int nseconds = 0;
+		
+		virDomainInfo info;
+
+		if (virDomainGetInfo(dom, &info) == 0) {
+		    vcpu = info.nrVirtCpu;
+		    memory = info.memory / 1024;
+		}
+
+        	persistent = virDomainIsPersistent(dom);
+        	if (virDomainGetAutostart(dom, &autostart) < 0) {
+        	    autostart = -1;
+        	}
+        	nsnap = virDomainSnapshotNum(dom, 0);
+        	mansave = virDomainHasManagedSaveImage(dom, 0);
+		
+        	char timestr[100];
+		if (state == VIR_DOMAIN_RUNNING) {		
+    		    if (virDomainGetTime(dom, &seconds, &nseconds, false) < 0) {
+        		strcpy(timestr, "??");
+        	    } 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 %5d %6d %9d %s%s%s%s%s %s\n", id_buf,
+                         virDomainGetName(dom),
+                         state == -2 ? _("saved")
+                         : virshDomainStateToString(state),
+                         vcpu, memory,
+                         nsnap,
+                         persistent == 1 ? "p" : persistent == 0 ? "t" : "?",
+                         autostart == 1 ? "a" : autostart == 0 ? " " : "?",
+                         mansave == 1 ? "m" : mansave == 0 ? " " : "?",
+                         nsnap > 0 ? "s" : nsnap == 0 ? " " : "?",
+                         " ",
+                         timestr
+                         );
+
             } else {
                 vshPrint(ctl, " %-5s %-30s %s\n", id_buf,
                          virDomainGetName(dom),


> 
>> 
>> 3. Please add —domtime parameter to virsh list command. It should show column with domain’s system time similar to virsh domtime domname --pretty.
>> It will be very helpful for fast guest-agent checking for every domain.
> 
> And this one is beyond the scope of the command. The aim of list command
> is to list available domains, not fetch side note info on them.
> 
>> 
>> 4. Please add column Parent and Metadata to virsh snapshot-list. 
>> 
> 
> I'm ambivalent on this one.
> 
> Michal



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