So much can happen in the fileName field of the VMX that the easiest
thing is to silently report a serial type="null".

This effectively reverts commits de81bdb8d4cd and 62c53db0421a, but
keeps the test files to show the fix is still in place.

There is one instance where an error gets reset, but since that is a
rare case on its own and on top of that does not happen in any of our
long-running daemons with a logfile that might get monitored it should
be fine to leave it there.

Resolves: https://issues.redhat.com/browse/RHEL-32182

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 src/vmx/vmx.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index e5bc2d793c66..227744d06258 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2975,9 +2975,6 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port,
     char fileName_name[48] = "";
     g_autofree char *fileName = NULL;
 
-    char vspc_name[48] = "";
-    g_autofree char *vspc = NULL;
-
     char network_endPoint_name[48] = "";
     g_autofree char *network_endPoint = NULL;
 
@@ -3000,7 +2997,6 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port,
     VMX_BUILD_NAME(startConnected);
     VMX_BUILD_NAME(fileType);
     VMX_BUILD_NAME(fileName);
-    VMX_BUILD_NAME(vspc);
     VMX_BUILD_NAME_EXTRA(network_endPoint, "network.endPoint");
 
     /* vmx:present */
@@ -3030,10 +3026,6 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port,
     if (virVMXGetConfigString(conf, fileName_name, &fileName, true) < 0)
         goto cleanup;
 
-    /* vmx:fileName -> def:data.file.path */
-    if (virVMXGetConfigString(conf, vspc_name, &vspc, true) < 0)
-        goto cleanup;
-
     /* vmx:network.endPoint -> def:data.tcp.listen */
     if (virVMXGetConfigString(conf, network_endPoint_name, &network_endPoint,
                               true) < 0) {
@@ -3065,21 +3057,25 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port,
         (*def)->target.port = port;
         (*def)->source->type = VIR_DOMAIN_CHR_TYPE_PIPE;
         (*def)->source->data.file.path = g_steal_pointer(&fileName);
-    } else if (STRCASEEQ(fileType, "network") && (vspc || !fileName || STREQ(fileName, ""))) {
-        (*def)->target.port = port;
-        (*def)->source->type = VIR_DOMAIN_CHR_TYPE_NULL;
     } else if (STRCASEEQ(fileType, "network")) {
         (*def)->target.port = port;
         (*def)->source->type = VIR_DOMAIN_CHR_TYPE_TCP;
 
-        if (!(parsedUri = virURIParse(fileName)))
-            goto cleanup;
+        if (!(parsedUri = virURIParse(fileName))) {
+            /*
+             * Ignore anything we cannot parse since there are many variations
+             * that could lead to unusable or non-representable serial ports
+             * which are very commonly seen and the main consumer of this driver
+             * (virt-v2v) ignores them anyway, so let's at least not error out.
+             */
+            virResetLastError();
+            (*def)->source->type = VIR_DOMAIN_CHR_TYPE_NULL;
+            return 0;
+        }
 
         if (parsedUri->port == 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("VMX entry '%1$s' doesn't contain a port part"),
-                           fileName_name);
-            goto cleanup;
+            (*def)->source->type = VIR_DOMAIN_CHR_TYPE_NULL;
+            return 0;
         }
 
         (*def)->source->data.tcp.host = g_strdup(parsedUri->server);
-- 
2.45.1

On 7/15/24 16:12, Martin Kletzander wrote:
> So much can happen in the fileName field of the VMX that the easiest
> thing is to silently report a serial type="null".
> 
> This effectively reverts commits de81bdb8d4cd and 62c53db0421a, but
> keeps the test files to show the fix is still in place.
> 
> There is one instance where an error gets reset, but since that is a
> rare case on its own and on top of that does not happen in any of our
> long-running daemons with a logfile that might get monitored it should
> be fine to leave it there.
> 
> Resolves: https://issues.redhat.com/browse/RHEL-32182
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  src/vmx/vmx.c | 30 +++++++++++++-----------------
>  1 file changed, 13 insertions(+), 17 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal