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

[libvirt] [PATCHv2 1/5] qemu: chardev: Extract more information about character devices



Improve the monitor function to also retrieve the guest state of
character device (if provided) so that we can refresh the state of
virtio-serial channels and perhaps react to changes in the state in
future patches.

This patch changes the returned data from qemuMonitorGetChardevInfo to
return a structure containing the pty path and the state for all the
character devices.

The change to the testsuite makes sure that the data is parsed
correctly.
---

Notes:
    Version 2:
    - all devices are now stored in the hash
    - code is optimized according to the review
    - example input code format enhanced to contain new field
    - tests modified to pass

 src/qemu/qemu_monitor.c      | 13 ++++++++++-
 src/qemu/qemu_monitor.h      |  6 ++++++
 src/qemu/qemu_monitor_json.c | 51 +++++++++++++++++++++++++++++---------------
 src/qemu/qemu_monitor_text.c | 17 ++++++++++-----
 src/qemu/qemu_process.c      |  8 +++----
 tests/qemumonitorjsontest.c  | 41 +++++++++++++++++++++++++++++------
 6 files changed, 103 insertions(+), 33 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 926619f..c9c84f9 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2982,6 +2982,17 @@ qemuMonitorQueryRxFilter(qemuMonitorPtr mon, const char *alias,
 }


+static void
+qemuMonitorChardevInfoFree(void *data,
+                           const void *name ATTRIBUTE_UNUSED)
+{
+    qemuMonitorChardevInfoPtr info = data;
+
+    VIR_FREE(info->ptyPath);
+    VIR_FREE(info);
+}
+
+
 int
 qemuMonitorGetChardevInfo(qemuMonitorPtr mon,
                           virHashTablePtr *retinfo)
@@ -2997,7 +3008,7 @@ qemuMonitorGetChardevInfo(qemuMonitorPtr mon,
         goto error;
     }

-    if (!(info = virHashCreate(10, virHashValueFree)))
+    if (!(info = virHashCreate(10, qemuMonitorChardevInfoFree)))
         goto error;

     if (mon->json)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 3078be0..21533a4 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -649,6 +649,12 @@ int qemuMonitorRemoveNetdev(qemuMonitorPtr mon,
 int qemuMonitorQueryRxFilter(qemuMonitorPtr mon, const char *alias,
                              virNetDevRxFilterPtr *filter);

+typedef struct _qemuMonitorChardevInfo qemuMonitorChardevInfo;
+typedef qemuMonitorChardevInfo *qemuMonitorChardevInfoPtr;
+struct _qemuMonitorChardevInfo {
+    char *ptyPath;
+    virDomainChrDeviceState state;
+};
 int qemuMonitorGetChardevInfo(qemuMonitorPtr mon,
                               virHashTablePtr *retinfo);

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 9c8a6fb..6e251b3 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3414,7 +3414,7 @@ qemuMonitorJSONQueryRxFilter(qemuMonitorPtr mon, const char *alias,
  *
  * {"return": [
  *      {"filename": "stdio", "label": "monitor"},
- *      {"filename": "pty:/dev/pts/6", "label": "serial0"},
+ *      {"filename": "pty:/dev/pts/6", "label": "serial0", "frontend-open": true},
  *      {"filename": "pty:/dev/pts/7", "label": "parallel0"}
  * ]}
  *
@@ -3426,6 +3426,7 @@ qemuMonitorJSONExtractChardevInfo(virJSONValuePtr reply,
     virJSONValuePtr data;
     int ret = -1;
     size_t i;
+    qemuMonitorChardevInfoPtr entry = NULL;

     if (!(data = virJSONValueObjectGet(reply, "return"))) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -3440,44 +3441,60 @@ qemuMonitorJSONExtractChardevInfo(virJSONValuePtr reply,
     }

     for (i = 0; i < virJSONValueArraySize(data); i++) {
-        virJSONValuePtr entry = virJSONValueArrayGet(data, i);
+        virJSONValuePtr chardev = virJSONValueArrayGet(data, i);
         const char *type;
-        const char *id;
-        if (!entry) {
+        const char *alias;
+        bool connected;
+
+        if (!chardev) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                            _("character device information was missing array element"));
             goto cleanup;
         }

-        if (!(type = virJSONValueObjectGetString(entry, "filename"))) {
+        if (!(alias = virJSONValueObjectGetString(chardev, "label"))) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("character device information was missing filename"));
+                           _("character device information was missing label"));
             goto cleanup;
         }

-        if (!(id = virJSONValueObjectGetString(entry, "label"))) {
+        if (!(type = virJSONValueObjectGetString(chardev, "filename"))) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                            _("character device information was missing filename"));
             goto cleanup;
         }

-        if (STRPREFIX(type, "pty:")) {
-            char *path;
-            if (VIR_STRDUP(path, type + strlen("pty:")) < 0)
-                goto cleanup;
+        if (VIR_ALLOC(entry) < 0)
+            goto cleanup;

-            if (virHashAddEntry(info, id, path) < 0) {
-                virReportError(VIR_ERR_OPERATION_FAILED,
-                               _("failed to save chardev path '%s'"), path);
-                VIR_FREE(path);
-                goto cleanup;
-            }
+        if (STRPREFIX(type, "pty:") &&
+            VIR_STRDUP(entry->ptyPath, type + strlen("pty:")) < 0)
+            goto cleanup;
+
+        if (virJSONValueObjectGetBoolean(chardev, "frontend-open", &connected) == 0) {
+            if (connected)
+                entry->state = VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED;
+            else
+                entry->state = VIR_DOMAIN_CHR_DEVICE_STATE_DISCONNECTED;
+        }
+
+        if (virHashAddEntry(info, alias, entry) < 0) {
+            virReportError(VIR_ERR_OPERATION_FAILED,
+                           _("failed to add chardev '%s' info"), alias);
+            goto cleanup;
         }
+
+        entry = NULL;
     }

     ret = 0;

  cleanup:
+    if (entry) {
+        VIR_FREE(entry->ptyPath);
+        VIR_FREE(entry);
+    }
+
     return ret;
 }

diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index b099a32..70aeaca 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -2174,6 +2174,7 @@ int qemuMonitorTextGetChardevInfo(qemuMonitorPtr mon,
                                   virHashTablePtr info)
 {
     char *reply = NULL;
+    qemuMonitorChardevInfoPtr entry = NULL;
     int ret = -1;

     if (qemuMonitorHMPCommand(mon, "info chardev", &reply) < 0)
@@ -2218,17 +2219,22 @@ int qemuMonitorTextGetChardevInfo(qemuMonitorPtr mon,

         /* Path is everything after needle to the end of the line */
         *eol = '\0';
-        char *path;
-        if (VIR_STRDUP(path, needle + strlen(NEEDLE)) < 0)
+
+        if (VIR_ALLOC(entry) < 0)
+            goto cleanup;
+
+        if (VIR_STRDUP(entry->ptyPath, needle + strlen(NEEDLE)) < 0)
             goto cleanup;

-        if (virHashAddEntry(info, id, path) < 0) {
+        if (virHashAddEntry(info, id, entry) < 0) {
             virReportError(VIR_ERR_OPERATION_FAILED,
                            _("failed to save chardev path '%s'"),
-                           path);
-            VIR_FREE(path);
+                           entry->ptyPath);
+            VIR_FREE(entry->ptyPath);
             goto cleanup;
         }
+
+        entry = NULL;
 #undef NEEDLE
     }

@@ -2236,6 +2242,7 @@ int qemuMonitorTextGetChardevInfo(qemuMonitorPtr mon,

  cleanup:
     VIR_FREE(reply);
+    VIR_FREE(entry);
     return ret;
 }

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 8655b5c..41fe0d1 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1919,7 +1919,7 @@ qemuProcessLookupPTYs(virDomainDefPtr def,

         if (chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) {
             char id[32];
-            const char *path;
+            qemuMonitorChardevInfoPtr entry;

             if (snprintf(id, sizeof(id), "%s%s",
                          chardevfmt ? "char" : "",
@@ -1930,8 +1930,8 @@ qemuProcessLookupPTYs(virDomainDefPtr def,
                 return -1;
             }

-            path = (const char *) virHashLookup(info, id);
-            if (path == NULL) {
+            entry = virHashLookup(info, id);
+            if (!entry || !entry->ptyPath) {
                 if (chr->source.data.file.path == NULL) {
                     /* neither the log output nor 'info chardev' had a
                      * pty path for this chardev, report an error
@@ -1948,7 +1948,7 @@ qemuProcessLookupPTYs(virDomainDefPtr def,
             }

             VIR_FREE(chr->source.data.file.path);
-            if (VIR_STRDUP(chr->source.data.file.path, path) < 0)
+            if (VIR_STRDUP(chr->source.data.file.path, entry->ptyPath) < 0)
                 return -1;
         }
     }
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index acbb414..5bfcd20 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1759,11 +1759,28 @@ testQemuMonitorJSONqemuMonitorJSONGetSpiceMigrationStatus(const void *data)
 }

 static int
-testHashEqualString(const void *value1, const void *value2)
+testHashEqualChardevInfo(const void *value1, const void *value2)
 {
-    return strcmp(value1, value2);
+    const qemuMonitorChardevInfo *info1 = value1;
+    const qemuMonitorChardevInfo *info2 = value2;
+
+    if (info1->state != info2->state)
+        goto error;
+
+    if (STRNEQ_NULLABLE(info1->ptyPath, info2->ptyPath))
+        goto error;
+
+    return 0;
+
+ error:
+    fprintf(stderr, "\n"
+            "info1->state: %d info2->state: %d\n"
+            "info1->ptyPath: %s info2->ptyPath: %s\n",
+            info1->state, info2->state, info1->ptyPath, info2->ptyPath);
+    return -1;
 }

+
 static int
 testQemuMonitorJSONqemuMonitorJSONGetChardevInfo(const void *data)
 {
@@ -1771,6 +1788,10 @@ testQemuMonitorJSONqemuMonitorJSONGetChardevInfo(const void *data)
     qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
     int ret = -1;
     virHashTablePtr info = NULL, expectedInfo = NULL;
+    qemuMonitorChardevInfo info0 = { NULL, VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT };
+    qemuMonitorChardevInfo info1 = { (char *) "/dev/pts/21", VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED };
+    qemuMonitorChardevInfo info2 = { (char *) "/dev/pts/20", VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT };
+    qemuMonitorChardevInfo info3 = { NULL, VIR_DOMAIN_CHR_DEVICE_STATE_DISCONNECTED };

     if (!test)
         return -1;
@@ -1779,8 +1800,10 @@ testQemuMonitorJSONqemuMonitorJSONGetChardevInfo(const void *data)
         !(expectedInfo = virHashCreate(32, NULL)))
         goto cleanup;

-    if (virHashAddEntry(expectedInfo, "charserial1", (void *) "/dev/pts/21") < 0 ||
-        virHashAddEntry(expectedInfo, "charserial0", (void *) "/dev/pts/20") < 0) {
+    if (virHashAddEntry(expectedInfo, "charserial1", &info1) < 0 ||
+        virHashAddEntry(expectedInfo, "charserial0", &info2) < 0 ||
+        virHashAddEntry(expectedInfo, "charmonitor", &info0) < 0 ||
+        virHashAddEntry(expectedInfo, "charserial2", &info3) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        "Unable to create expectedInfo hash table");
         goto cleanup;
@@ -1791,7 +1814,8 @@ testQemuMonitorJSONqemuMonitorJSONGetChardevInfo(const void *data)
                                "    \"return\": ["
                                "        {"
                                "            \"filename\": \"pty:/dev/pts/21\","
-                               "            \"label\": \"charserial1\""
+                               "            \"label\": \"charserial1\","
+                               "            \"frontend-open\": true"
                                "        },"
                                "        {"
                                "            \"filename\": \"pty:/dev/pts/20\","
@@ -1800,6 +1824,11 @@ testQemuMonitorJSONqemuMonitorJSONGetChardevInfo(const void *data)
                                "        {"
                                "            \"filename\": \"unix:/var/lib/libvirt/qemu/gentoo.monitor,server\","
                                "            \"label\": \"charmonitor\""
+                               "        },"
+                               "        {"
+                               "            \"filename\": \"unix:/path/to/socket,server\","
+                               "            \"label\": \"charserial2\","
+                               "            \"frontend-open\": false"
                                "        }"
                                "    ],"
                                "    \"id\": \"libvirt-15\""
@@ -1810,7 +1839,7 @@ testQemuMonitorJSONqemuMonitorJSONGetChardevInfo(const void *data)
                                       info) < 0)
         goto cleanup;

-    if (!virHashEqual(info, expectedInfo, testHashEqualString)) {
+    if (!virHashEqual(info, expectedInfo, testHashEqualChardevInfo)) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        "Hashtable is different to the expected one");
         goto cleanup;
-- 
2.1.0


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