1. Patchew
  2. Libvirt
  3. View series

[PATCH] vmx: Check serialX.vspc before serialX.fileName

Martin Kletzander posted 1 patch 1 week, 3 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/dd3dd0cf8457c0ca23c8397c7435505f10cf5c36.1714047174.git.mkletzan@redhat.com
src/vmx/vmx.c                            | 12 +++
tests/vmx2xmldata/esx-in-the-wild-13.vmx | 97 ++++++++++++++++++++++++
tests/vmx2xmldata/esx-in-the-wild-13.xml | 55 ++++++++++++++
tests/vmx2xmltest.c                      |  1 +
4 files changed, 165 insertions(+)
create mode 100644 tests/vmx2xmldata/esx-in-the-wild-13.vmx
create mode 100644 tests/vmx2xmldata/esx-in-the-wild-13.xml
[PATCH] vmx: Check serialX.vspc before serialX.fileName
Posted by Martin Kletzander 1 week, 3 days ago 
When using vSPC (Virtual Serial Port Concentrator) in vSphere the actual
address for it is saved in serialX.vspc in which case the
serialX.fileName is most probably something we can't get any useful
information from and we also fail during the parsing rendering any
dumpxml and similar tries unsuccessful.

Instead of parsing the vspc URL with something along the lines of
`virURIParse(vspc ? vspc : fileName)`, which could lead to us reporting
information that is very prune to misuse (the vSPC seemingly has a
protocol on top of the telnet connection; redefining the domain would
change the behaviour; the URL might have a fragment we are not saving;
etc.) or adding more XML knobs to indicate vSPC usage (which we would
not be able to configure; we'd have to properly error out everywhere;
etc.) let's just report dummy serial port that leads to nowhere.

Resolves: https://issues.redhat.com/browse/RHEL-32182
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 src/vmx/vmx.c                            | 12 +++
 tests/vmx2xmldata/esx-in-the-wild-13.vmx | 97 ++++++++++++++++++++++++
 tests/vmx2xmldata/esx-in-the-wild-13.xml | 55 ++++++++++++++
 tests/vmx2xmltest.c                      |  1 +
 4 files changed, 165 insertions(+)
 create mode 100644 tests/vmx2xmldata/esx-in-the-wild-13.vmx
 create mode 100644 tests/vmx2xmldata/esx-in-the-wild-13.xml

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 5da67aae60d9..32074f62e14c 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2975,6 +2975,9 @@ 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;
 
@@ -2997,6 +3000,7 @@ 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 */
@@ -3026,6 +3030,10 @@ 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) {
@@ -3057,6 +3065,10 @@ 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) {
+        (*def)->target.port = port;
+        (*def)->source->type = VIR_DOMAIN_CHR_TYPE_FILE;
+        (*def)->source->data.file.path = g_strdup("/dev/null");
     } else if (STRCASEEQ(fileType, "network")) {
         (*def)->target.port = port;
         (*def)->source->type = VIR_DOMAIN_CHR_TYPE_TCP;
diff --git a/tests/vmx2xmldata/esx-in-the-wild-13.vmx b/tests/vmx2xmldata/esx-in-the-wild-13.vmx
new file mode 100644
index 000000000000..1016acab28d8
--- /dev/null
+++ b/tests/vmx2xmldata/esx-in-the-wild-13.vmx
@@ -0,0 +1,97 @@
+.encoding = "UTF-8"
+config.version = "8"
+virtualHW.version = "19"
+vmci0.present = "TRUE"
+floppy0.present = "FALSE"
+memSize = "1024"
+tools.upgrade.policy = "manual"
+sched.cpu.units = "mhz"
+vm.createDate = "1704946351823519"
+scsi0.virtualDev = "lsilogic"
+scsi0.present = "TRUE"
+scsi0:0.deviceType = "scsi-hardDisk"
+scsi0:0.fileName = "Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)_2.vmdk"
+sched.scsi0:0.shares = "normal"
+sched.scsi0:0.throughputCap = "off"
+scsi0:0.present = "TRUE"
+ethernet0.virtualDev = "vmxnet3"
+ethernet0.opaqueNetwork.id = "25c9a00e-dc60-4918-89b7-41c951988366"
+ethernet0.opaqueNetwork.type = "nsx.LogicalSwitch"
+ethernet0.shares = "normal"
+ethernet0.addressType = "static"
+ethernet0.address = "fa:16:3e:bb:2c:4a"
+ethernet0.externalId = "4b57523e-35af-4f8d-b050-1a7410e1a307"
+ethernet0.uptCompatibility = "TRUE"
+ethernet0.present = "TRUE"
+ethernet0.networkName = "Test"
+serial0.fileType = "network"
+serial0.fileName = "ZmVybmV0IGdBQUFBQUJrdFotaW8yclpkRXR6N3dBcDdyYkFMaWFUMVd4RENJSHgtUXpkTlMyTzRRejI3V192QVlOVUY3ZU1SOTNHZXJrN1dGb2stS0EybmpwWFQ4NjJNNlgwc2ZDdmNlOE50eFNhcU1XdlNBTmdhazQ1T1J3LUI5OEZsSDdwMDBZa2R6bWt4Y1Ax"
+serial0.vspc = "telnets://10.28.100.26:18979#thumbprint=18:F5:79:E5:73:A5:22:83:C0:57:B9:B4:FA:CE:60:19:F1:12:F5:7B"
+serial0.yieldOnMsrRead = "TRUE"
+serial0.present = "TRUE"
+displayName = "Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)"
+annotation = "name:Test-Mig-VM-1|0Auserid:962314ba515c48388a0e95c0961709ff|0Ausername:admin|0Aprojectid:b06b5f77b6bb442f85b1c67cff980ef9|0Aprojectname:MIS|0Aflavor:name:mig-test-flavor|0Aflavor:memory_mb:1024|0Aflavor:vcpus:1|0Aflavor:ephemeral_gb:0|0Aflavor:root_gb:10|0Aflavor:swap:0|0Aimageid:8b90d6fa-20ab-4adf-8015-aad3dddb246c|0Apackage:20.6.2|0A"
+guestOS = "other-64"
+toolScripts.afterPowerOn = "TRUE"
+toolScripts.afterResume = "TRUE"
+toolScripts.beforeSuspend = "TRUE"
+toolScripts.beforePowerOff = "TRUE"
+tools.syncTime = "FALSE"
+uuid.bios = "42 1e b4 58 54 48 fc 12-20 74 83 d5 e4 19 e1 38"
+vc.uuid = "01 ce 57 d0 4e 20 41 a5-8b 6c bc bf 49 a0 32 ec"
+sched.cpu.latencySensitivity = "normal"
+tools.guest.desktop.autolock = "FALSE"
+nvram = "Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec).nvram"
+svga.present = "TRUE"
+pciBridge0.present = "TRUE"
+pciBridge4.present = "TRUE"
+pciBridge4.virtualDev = "pcieRootPort"
+pciBridge4.functions = "8"
+pciBridge5.present = "TRUE"
+pciBridge5.virtualDev = "pcieRootPort"
+pciBridge5.functions = "8"
+pciBridge6.present = "TRUE"
+pciBridge6.virtualDev = "pcieRootPort"
+pciBridge6.functions = "8"
+pciBridge7.present = "TRUE"
+pciBridge7.virtualDev = "pcieRootPort"
+pciBridge7.functions = "8"
+hpet0.present = "TRUE"
+vio.imageId = "8b90d6fa-20ab-4adf-8015-aad3dddb246c"
+nvp.vm-uuid = "01ce57d0-4e20-41a5-8b6c-bcbf49a032ec"
+nvp.iface-id.0 = "4b57523e-35af-4f8d-b050-1a7410e1a307"
+smbios.assetTag = "OpenTelekomCloud"
+root-disk-uuid = "6000C294-db10-e2c6-963f-e4371d3605e8"
+numa.autosize.cookie = "10012"
+numa.autosize.vcpu.maxPerVirtualNode = "1"
+pciBridge0.pciSlotNumber = "17"
+pciBridge4.pciSlotNumber = "21"
+pciBridge5.pciSlotNumber = "22"
+pciBridge6.pciSlotNumber = "23"
+pciBridge7.pciSlotNumber = "24"
+scsi0.pciSlotNumber = "16"
+ethernet0.pciSlotNumber = "160"
+vmotion.checkpointFBSize = "4194304"
+vmotion.checkpointSVGAPrimarySize = "4194304"
+vmotion.svga.mobMaxSize = "4194304"
+vmotion.svga.graphicsMemoryKB = "4096"
+monitor.phys_bits_used = "45"
+softPowerOff = "FALSE"
+ctkEnabled = "TRUE"
+scsi0:0.ctkEnabled = "TRUE"
+svga.guestBackedPrimaryAware = "TRUE"
+viv.moid = "bd916f5e-a90b-46db-98e8-ce974fef34f4:vm-29145:1NineJhqocLZEZ6JOxn/hLdjlihTelLVQGc8mmAj3nA="
+migrate.hostLog = "Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)-7cedc035.hlog"
+sched.cpu.min = "0"
+sched.cpu.shares = "normal"
+sched.mem.min = "0"
+sched.mem.minSize = "0"
+sched.mem.shares = "normal"
+migrate.encryptionMode = "opportunistic"
+ftcpt.ftEncryptionMode = "ftEncryptionOpportunistic"
+sched.swap.derivedName = "/vmfs/volumes/642eff9a-0df93507-cca5-5c6f69c57c70/Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)/Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)-69fcc9f7.vswp"
+uuid.location = "56 4d c2 6b 03 eb b2 44-eb 36 8a 83 de 3f bd b6"
+scsi0:0.redo = ""
+vmci0.id = "-468065992"
+cleanShutdown = "TRUE"
+vmxstats.filename = "Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec).scoreboard"
diff --git a/tests/vmx2xmldata/esx-in-the-wild-13.xml b/tests/vmx2xmldata/esx-in-the-wild-13.xml
new file mode 100644
index 000000000000..689282747779
--- /dev/null
+++ b/tests/vmx2xmldata/esx-in-the-wild-13.xml
@@ -0,0 +1,55 @@
+<domain type='vmware'>
+  <name>Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)</name>
+  <uuid>421eb458-5448-fc12-2074-83d5e419e138</uuid>
+  <description>name:Test-Mig-VM-1
+userid:962314ba515c48388a0e95c0961709ff
+username:admin
+projectid:b06b5f77b6bb442f85b1c67cff980ef9
+projectname:MIS
+flavor:name:mig-test-flavor
+flavor:memory_mb:1024
+flavor:vcpus:1
+flavor:ephemeral_gb:0
+flavor:root_gb:10
+flavor:swap:0
+imageid:8b90d6fa-20ab-4adf-8015-aad3dddb246c
+package:20.6.2
+</description>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <cputune>
+    <shares>1000</shares>
+  </cputune>
+  <os>
+    <type arch='x86_64'>hvm</type>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <disk type='file' device='disk'>
+      <source file='[datastore] directory/Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)_2.vmdk'/>
+      <target dev='sda' bus='scsi'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='scsi' index='0' model='lsilogic'/>
+    <interface type='bridge'>
+      <mac address='fa:16:3e:bb:2c:4a' type='static'/>
+      <source bridge='Test'/>
+      <model type='vmxnet3'/>
+    </interface>
+    <serial type='file'>
+      <source path='/dev/null'/>
+      <target port='0'/>
+    </serial>
+    <console type='file'>
+      <source path='/dev/null'/>
+      <target type='serial' port='0'/>
+    </console>
+    <video>
+      <model type='vmvga' vram='4096' primary='yes'/>
+    </video>
+  </devices>
+</domain>
diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
index 785eee6d3002..0fb5f13f72ce 100644
--- a/tests/vmx2xmltest.c
+++ b/tests/vmx2xmltest.c
@@ -263,6 +263,7 @@ mymain(void)
     DO_TEST("esx-in-the-wild-10");
     DO_TEST("esx-in-the-wild-11");
     DO_TEST("esx-in-the-wild-12");
+    DO_TEST("esx-in-the-wild-13");
 
     DO_TEST("gsx-in-the-wild-1");
     DO_TEST("gsx-in-the-wild-2");
-- 
2.44.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] vmx: Check serialX.vspc before serialX.fileName
Posted by Richard W.M. Jones 1 week, 2 days ago 
On Thu, Apr 25, 2024 at 02:12:54PM +0200, Martin Kletzander wrote:
> When using vSPC (Virtual Serial Port Concentrator) in vSphere the actual
> address for it is saved in serialX.vspc in which case the
> serialX.fileName is most probably something we can't get any useful
> information from and we also fail during the parsing rendering any
> dumpxml and similar tries unsuccessful.
> 
> Instead of parsing the vspc URL with something along the lines of
> `virURIParse(vspc ? vspc : fileName)`, which could lead to us reporting
> information that is very prune to misuse (the vSPC seemingly has a
> protocol on top of the telnet connection; redefining the domain would
> change the behaviour; the URL might have a fragment we are not saving;
> etc.) or adding more XML knobs to indicate vSPC usage (which we would
> not be able to configure; we'd have to properly error out everywhere;
> etc.) let's just report dummy serial port that leads to nowhere.
> 
> Resolves: https://issues.redhat.com/browse/RHEL-32182
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  src/vmx/vmx.c                            | 12 +++
>  tests/vmx2xmldata/esx-in-the-wild-13.vmx | 97 ++++++++++++++++++++++++
>  tests/vmx2xmldata/esx-in-the-wild-13.xml | 55 ++++++++++++++
>  tests/vmx2xmltest.c                      |  1 +
>  4 files changed, 165 insertions(+)
>  create mode 100644 tests/vmx2xmldata/esx-in-the-wild-13.vmx
>  create mode 100644 tests/vmx2xmldata/esx-in-the-wild-13.xml
> 
> diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
> index 5da67aae60d9..32074f62e14c 100644
> --- a/src/vmx/vmx.c
> +++ b/src/vmx/vmx.c
> @@ -2975,6 +2975,9 @@ 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;
>  
> @@ -2997,6 +3000,7 @@ 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 */
> @@ -3026,6 +3030,10 @@ 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) {
> @@ -3057,6 +3065,10 @@ 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) {
> +        (*def)->target.port = port;
> +        (*def)->source->type = VIR_DOMAIN_CHR_TYPE_FILE;
> +        (*def)->source->data.file.path = g_strdup("/dev/null");
>      } else if (STRCASEEQ(fileType, "network")) {
>          (*def)->target.port = port;
>          (*def)->source->type = VIR_DOMAIN_CHR_TYPE_TCP;
> diff --git a/tests/vmx2xmldata/esx-in-the-wild-13.vmx b/tests/vmx2xmldata/esx-in-the-wild-13.vmx
> new file mode 100644
> index 000000000000..1016acab28d8
> --- /dev/null
> +++ b/tests/vmx2xmldata/esx-in-the-wild-13.vmx
> @@ -0,0 +1,97 @@
> +.encoding = "UTF-8"
> +config.version = "8"
> +virtualHW.version = "19"
> +vmci0.present = "TRUE"
> +floppy0.present = "FALSE"
> +memSize = "1024"
> +tools.upgrade.policy = "manual"
> +sched.cpu.units = "mhz"
> +vm.createDate = "1704946351823519"
> +scsi0.virtualDev = "lsilogic"
> +scsi0.present = "TRUE"
> +scsi0:0.deviceType = "scsi-hardDisk"
> +scsi0:0.fileName = "Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)_2.vmdk"
> +sched.scsi0:0.shares = "normal"
> +sched.scsi0:0.throughputCap = "off"
> +scsi0:0.present = "TRUE"
> +ethernet0.virtualDev = "vmxnet3"
> +ethernet0.opaqueNetwork.id = "25c9a00e-dc60-4918-89b7-41c951988366"
> +ethernet0.opaqueNetwork.type = "nsx.LogicalSwitch"
> +ethernet0.shares = "normal"
> +ethernet0.addressType = "static"
> +ethernet0.address = "fa:16:3e:bb:2c:4a"
> +ethernet0.externalId = "4b57523e-35af-4f8d-b050-1a7410e1a307"
> +ethernet0.uptCompatibility = "TRUE"
> +ethernet0.present = "TRUE"
> +ethernet0.networkName = "Test"
> +serial0.fileType = "network"
> +serial0.fileName = "ZmVybmV0IGdBQUFBQUJrdFotaW8yclpkRXR6N3dBcDdyYkFMaWFUMVd4RENJSHgtUXpkTlMyTzRRejI3V192QVlOVUY3ZU1SOTNHZXJrN1dGb2stS0EybmpwWFQ4NjJNNlgwc2ZDdmNlOE50eFNhcU1XdlNBTmdhazQ1T1J3LUI5OEZsSDdwMDBZa2R6bWt4Y1Ax"
> +serial0.vspc = "telnets://10.28.100.26:18979#thumbprint=18:F5:79:E5:73:A5:22:83:C0:57:B9:B4:FA:CE:60:19:F1:12:F5:7B"
> +serial0.yieldOnMsrRead = "TRUE"
> +serial0.present = "TRUE"
> +displayName = "Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)"
> +annotation = "name:Test-Mig-VM-1|0Auserid:962314ba515c48388a0e95c0961709ff|0Ausername:admin|0Aprojectid:b06b5f77b6bb442f85b1c67cff980ef9|0Aprojectname:MIS|0Aflavor:name:mig-test-flavor|0Aflavor:memory_mb:1024|0Aflavor:vcpus:1|0Aflavor:ephemeral_gb:0|0Aflavor:root_gb:10|0Aflavor:swap:0|0Aimageid:8b90d6fa-20ab-4adf-8015-aad3dddb246c|0Apackage:20.6.2|0A"
> +guestOS = "other-64"
> +toolScripts.afterPowerOn = "TRUE"
> +toolScripts.afterResume = "TRUE"
> +toolScripts.beforeSuspend = "TRUE"
> +toolScripts.beforePowerOff = "TRUE"
> +tools.syncTime = "FALSE"
> +uuid.bios = "42 1e b4 58 54 48 fc 12-20 74 83 d5 e4 19 e1 38"
> +vc.uuid = "01 ce 57 d0 4e 20 41 a5-8b 6c bc bf 49 a0 32 ec"
> +sched.cpu.latencySensitivity = "normal"
> +tools.guest.desktop.autolock = "FALSE"
> +nvram = "Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec).nvram"
> +svga.present = "TRUE"
> +pciBridge0.present = "TRUE"
> +pciBridge4.present = "TRUE"
> +pciBridge4.virtualDev = "pcieRootPort"
> +pciBridge4.functions = "8"
> +pciBridge5.present = "TRUE"
> +pciBridge5.virtualDev = "pcieRootPort"
> +pciBridge5.functions = "8"
> +pciBridge6.present = "TRUE"
> +pciBridge6.virtualDev = "pcieRootPort"
> +pciBridge6.functions = "8"
> +pciBridge7.present = "TRUE"
> +pciBridge7.virtualDev = "pcieRootPort"
> +pciBridge7.functions = "8"
> +hpet0.present = "TRUE"
> +vio.imageId = "8b90d6fa-20ab-4adf-8015-aad3dddb246c"
> +nvp.vm-uuid = "01ce57d0-4e20-41a5-8b6c-bcbf49a032ec"
> +nvp.iface-id.0 = "4b57523e-35af-4f8d-b050-1a7410e1a307"
> +smbios.assetTag = "OpenTelekomCloud"
> +root-disk-uuid = "6000C294-db10-e2c6-963f-e4371d3605e8"
> +numa.autosize.cookie = "10012"
> +numa.autosize.vcpu.maxPerVirtualNode = "1"
> +pciBridge0.pciSlotNumber = "17"
> +pciBridge4.pciSlotNumber = "21"
> +pciBridge5.pciSlotNumber = "22"
> +pciBridge6.pciSlotNumber = "23"
> +pciBridge7.pciSlotNumber = "24"
> +scsi0.pciSlotNumber = "16"
> +ethernet0.pciSlotNumber = "160"
> +vmotion.checkpointFBSize = "4194304"
> +vmotion.checkpointSVGAPrimarySize = "4194304"
> +vmotion.svga.mobMaxSize = "4194304"
> +vmotion.svga.graphicsMemoryKB = "4096"
> +monitor.phys_bits_used = "45"
> +softPowerOff = "FALSE"
> +ctkEnabled = "TRUE"
> +scsi0:0.ctkEnabled = "TRUE"
> +svga.guestBackedPrimaryAware = "TRUE"
> +viv.moid = "bd916f5e-a90b-46db-98e8-ce974fef34f4:vm-29145:1NineJhqocLZEZ6JOxn/hLdjlihTelLVQGc8mmAj3nA="
> +migrate.hostLog = "Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)-7cedc035.hlog"
> +sched.cpu.min = "0"
> +sched.cpu.shares = "normal"
> +sched.mem.min = "0"
> +sched.mem.minSize = "0"
> +sched.mem.shares = "normal"
> +migrate.encryptionMode = "opportunistic"
> +ftcpt.ftEncryptionMode = "ftEncryptionOpportunistic"
> +sched.swap.derivedName = "/vmfs/volumes/642eff9a-0df93507-cca5-5c6f69c57c70/Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)/Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)-69fcc9f7.vswp"
> +uuid.location = "56 4d c2 6b 03 eb b2 44-eb 36 8a 83 de 3f bd b6"
> +scsi0:0.redo = ""
> +vmci0.id = "-468065992"
> +cleanShutdown = "TRUE"
> +vmxstats.filename = "Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec).scoreboard"
> diff --git a/tests/vmx2xmldata/esx-in-the-wild-13.xml b/tests/vmx2xmldata/esx-in-the-wild-13.xml
> new file mode 100644
> index 000000000000..689282747779
> --- /dev/null
> +++ b/tests/vmx2xmldata/esx-in-the-wild-13.xml
> @@ -0,0 +1,55 @@
> +<domain type='vmware'>
> +  <name>Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)</name>
> +  <uuid>421eb458-5448-fc12-2074-83d5e419e138</uuid>
> +  <description>name:Test-Mig-VM-1
> +userid:962314ba515c48388a0e95c0961709ff
> +username:admin
> +projectid:b06b5f77b6bb442f85b1c67cff980ef9
> +projectname:MIS
> +flavor:name:mig-test-flavor
> +flavor:memory_mb:1024
> +flavor:vcpus:1
> +flavor:ephemeral_gb:0
> +flavor:root_gb:10
> +flavor:swap:0
> +imageid:8b90d6fa-20ab-4adf-8015-aad3dddb246c
> +package:20.6.2
> +</description>
> +  <memory unit='KiB'>1048576</memory>
> +  <currentMemory unit='KiB'>1048576</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <cputune>
> +    <shares>1000</shares>
> +  </cputune>
> +  <os>
> +    <type arch='x86_64'>hvm</type>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <disk type='file' device='disk'>
> +      <source file='[datastore] directory/Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)_2.vmdk'/>
> +      <target dev='sda' bus='scsi'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <controller type='scsi' index='0' model='lsilogic'/>
> +    <interface type='bridge'>
> +      <mac address='fa:16:3e:bb:2c:4a' type='static'/>
> +      <source bridge='Test'/>
> +      <model type='vmxnet3'/>
> +    </interface>
> +    <serial type='file'>
> +      <source path='/dev/null'/>
> +      <target port='0'/>
> +    </serial>
> +    <console type='file'>
> +      <source path='/dev/null'/>
> +      <target type='serial' port='0'/>
> +    </console>
> +    <video>
> +      <model type='vmvga' vram='4096' primary='yes'/>
> +    </video>
> +  </devices>
> +</domain>
> diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
> index 785eee6d3002..0fb5f13f72ce 100644
> --- a/tests/vmx2xmltest.c
> +++ b/tests/vmx2xmltest.c
> @@ -263,6 +263,7 @@ mymain(void)
>      DO_TEST("esx-in-the-wild-10");
>      DO_TEST("esx-in-the-wild-11");
>      DO_TEST("esx-in-the-wild-12");
> +    DO_TEST("esx-in-the-wild-13");
>  
>      DO_TEST("gsx-in-the-wild-1");
>      DO_TEST("gsx-in-the-wild-2");
> -- 

From the point of view of virt-v2v:

ACK

I don't care if the path is /dev/null or nothing, as virt-v2v will
ignore the serial port.  Most important thing is not to fail.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] vmx: Check serialX.vspc before serialX.fileName
Posted by Martin Kletzander 1 week, 2 days ago 
On Fri, Apr 26, 2024 at 11:47:36AM +0100, Richard W.M. Jones wrote:
>On Thu, Apr 25, 2024 at 02:12:54PM +0200, Martin Kletzander wrote:
>> When using vSPC (Virtual Serial Port Concentrator) in vSphere the actual
>> address for it is saved in serialX.vspc in which case the
>> serialX.fileName is most probably something we can't get any useful
>> information from and we also fail during the parsing rendering any
>> dumpxml and similar tries unsuccessful.
>>
>> Instead of parsing the vspc URL with something along the lines of
>> `virURIParse(vspc ? vspc : fileName)`, which could lead to us reporting
>> information that is very prune to misuse (the vSPC seemingly has a
>> protocol on top of the telnet connection; redefining the domain would
>> change the behaviour; the URL might have a fragment we are not saving;
>> etc.) or adding more XML knobs to indicate vSPC usage (which we would
>> not be able to configure; we'd have to properly error out everywhere;
>> etc.) let's just report dummy serial port that leads to nowhere.
>>
>> Resolves: https://issues.redhat.com/browse/RHEL-32182
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---
>>  src/vmx/vmx.c                            | 12 +++
>>  tests/vmx2xmldata/esx-in-the-wild-13.vmx | 97 ++++++++++++++++++++++++
>>  tests/vmx2xmldata/esx-in-the-wild-13.xml | 55 ++++++++++++++
>>  tests/vmx2xmltest.c                      |  1 +
>>  4 files changed, 165 insertions(+)
>>  create mode 100644 tests/vmx2xmldata/esx-in-the-wild-13.vmx
>>  create mode 100644 tests/vmx2xmldata/esx-in-the-wild-13.xml
>>
>> diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
>> index 5da67aae60d9..32074f62e14c 100644
>> --- a/src/vmx/vmx.c
>> +++ b/src/vmx/vmx.c
>> @@ -2975,6 +2975,9 @@ 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;
>>
>> @@ -2997,6 +3000,7 @@ 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 */
>> @@ -3026,6 +3030,10 @@ 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) {
>> @@ -3057,6 +3065,10 @@ 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) {
>> +        (*def)->target.port = port;
>> +        (*def)->source->type = VIR_DOMAIN_CHR_TYPE_FILE;
>> +        (*def)->source->data.file.path = g_strdup("/dev/null");
>>      } else if (STRCASEEQ(fileType, "network")) {
>>          (*def)->target.port = port;
>>          (*def)->source->type = VIR_DOMAIN_CHR_TYPE_TCP;
>> diff --git a/tests/vmx2xmldata/esx-in-the-wild-13.vmx b/tests/vmx2xmldata/esx-in-the-wild-13.vmx
>> new file mode 100644
>> index 000000000000..1016acab28d8
>> --- /dev/null
>> +++ b/tests/vmx2xmldata/esx-in-the-wild-13.vmx
>> @@ -0,0 +1,97 @@
>> +.encoding = "UTF-8"
>> +config.version = "8"
>> +virtualHW.version = "19"
>> +vmci0.present = "TRUE"
>> +floppy0.present = "FALSE"
>> +memSize = "1024"
>> +tools.upgrade.policy = "manual"
>> +sched.cpu.units = "mhz"
>> +vm.createDate = "1704946351823519"
>> +scsi0.virtualDev = "lsilogic"
>> +scsi0.present = "TRUE"
>> +scsi0:0.deviceType = "scsi-hardDisk"
>> +scsi0:0.fileName = "Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)_2.vmdk"
>> +sched.scsi0:0.shares = "normal"
>> +sched.scsi0:0.throughputCap = "off"
>> +scsi0:0.present = "TRUE"
>> +ethernet0.virtualDev = "vmxnet3"
>> +ethernet0.opaqueNetwork.id = "25c9a00e-dc60-4918-89b7-41c951988366"
>> +ethernet0.opaqueNetwork.type = "nsx.LogicalSwitch"
>> +ethernet0.shares = "normal"
>> +ethernet0.addressType = "static"
>> +ethernet0.address = "fa:16:3e:bb:2c:4a"
>> +ethernet0.externalId = "4b57523e-35af-4f8d-b050-1a7410e1a307"
>> +ethernet0.uptCompatibility = "TRUE"
>> +ethernet0.present = "TRUE"
>> +ethernet0.networkName = "Test"
>> +serial0.fileType = "network"
>> +serial0.fileName = "ZmVybmV0IGdBQUFBQUJrdFotaW8yclpkRXR6N3dBcDdyYkFMaWFUMVd4RENJSHgtUXpkTlMyTzRRejI3V192QVlOVUY3ZU1SOTNHZXJrN1dGb2stS0EybmpwWFQ4NjJNNlgwc2ZDdmNlOE50eFNhcU1XdlNBTmdhazQ1T1J3LUI5OEZsSDdwMDBZa2R6bWt4Y1Ax"
>> +serial0.vspc = "telnets://10.28.100.26:18979#thumbprint=18:F5:79:E5:73:A5:22:83:C0:57:B9:B4:FA:CE:60:19:F1:12:F5:7B"
>> +serial0.yieldOnMsrRead = "TRUE"
>> +serial0.present = "TRUE"
>> +displayName = "Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)"
>> +annotation = "name:Test-Mig-VM-1|0Auserid:962314ba515c48388a0e95c0961709ff|0Ausername:admin|0Aprojectid:b06b5f77b6bb442f85b1c67cff980ef9|0Aprojectname:MIS|0Aflavor:name:mig-test-flavor|0Aflavor:memory_mb:1024|0Aflavor:vcpus:1|0Aflavor:ephemeral_gb:0|0Aflavor:root_gb:10|0Aflavor:swap:0|0Aimageid:8b90d6fa-20ab-4adf-8015-aad3dddb246c|0Apackage:20.6.2|0A"
>> +guestOS = "other-64"
>> +toolScripts.afterPowerOn = "TRUE"
>> +toolScripts.afterResume = "TRUE"
>> +toolScripts.beforeSuspend = "TRUE"
>> +toolScripts.beforePowerOff = "TRUE"
>> +tools.syncTime = "FALSE"
>> +uuid.bios = "42 1e b4 58 54 48 fc 12-20 74 83 d5 e4 19 e1 38"
>> +vc.uuid = "01 ce 57 d0 4e 20 41 a5-8b 6c bc bf 49 a0 32 ec"
>> +sched.cpu.latencySensitivity = "normal"
>> +tools.guest.desktop.autolock = "FALSE"
>> +nvram = "Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec).nvram"
>> +svga.present = "TRUE"
>> +pciBridge0.present = "TRUE"
>> +pciBridge4.present = "TRUE"
>> +pciBridge4.virtualDev = "pcieRootPort"
>> +pciBridge4.functions = "8"
>> +pciBridge5.present = "TRUE"
>> +pciBridge5.virtualDev = "pcieRootPort"
>> +pciBridge5.functions = "8"
>> +pciBridge6.present = "TRUE"
>> +pciBridge6.virtualDev = "pcieRootPort"
>> +pciBridge6.functions = "8"
>> +pciBridge7.present = "TRUE"
>> +pciBridge7.virtualDev = "pcieRootPort"
>> +pciBridge7.functions = "8"
>> +hpet0.present = "TRUE"
>> +vio.imageId = "8b90d6fa-20ab-4adf-8015-aad3dddb246c"
>> +nvp.vm-uuid = "01ce57d0-4e20-41a5-8b6c-bcbf49a032ec"
>> +nvp.iface-id.0 = "4b57523e-35af-4f8d-b050-1a7410e1a307"
>> +smbios.assetTag = "OpenTelekomCloud"
>> +root-disk-uuid = "6000C294-db10-e2c6-963f-e4371d3605e8"
>> +numa.autosize.cookie = "10012"
>> +numa.autosize.vcpu.maxPerVirtualNode = "1"
>> +pciBridge0.pciSlotNumber = "17"
>> +pciBridge4.pciSlotNumber = "21"
>> +pciBridge5.pciSlotNumber = "22"
>> +pciBridge6.pciSlotNumber = "23"
>> +pciBridge7.pciSlotNumber = "24"
>> +scsi0.pciSlotNumber = "16"
>> +ethernet0.pciSlotNumber = "160"
>> +vmotion.checkpointFBSize = "4194304"
>> +vmotion.checkpointSVGAPrimarySize = "4194304"
>> +vmotion.svga.mobMaxSize = "4194304"
>> +vmotion.svga.graphicsMemoryKB = "4096"
>> +monitor.phys_bits_used = "45"
>> +softPowerOff = "FALSE"
>> +ctkEnabled = "TRUE"
>> +scsi0:0.ctkEnabled = "TRUE"
>> +svga.guestBackedPrimaryAware = "TRUE"
>> +viv.moid = "bd916f5e-a90b-46db-98e8-ce974fef34f4:vm-29145:1NineJhqocLZEZ6JOxn/hLdjlihTelLVQGc8mmAj3nA="
>> +migrate.hostLog = "Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)-7cedc035.hlog"
>> +sched.cpu.min = "0"
>> +sched.cpu.shares = "normal"
>> +sched.mem.min = "0"
>> +sched.mem.minSize = "0"
>> +sched.mem.shares = "normal"
>> +migrate.encryptionMode = "opportunistic"
>> +ftcpt.ftEncryptionMode = "ftEncryptionOpportunistic"
>> +sched.swap.derivedName = "/vmfs/volumes/642eff9a-0df93507-cca5-5c6f69c57c70/Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)/Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)-69fcc9f7.vswp"
>> +uuid.location = "56 4d c2 6b 03 eb b2 44-eb 36 8a 83 de 3f bd b6"
>> +scsi0:0.redo = ""
>> +vmci0.id = "-468065992"
>> +cleanShutdown = "TRUE"
>> +vmxstats.filename = "Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec).scoreboard"
>> diff --git a/tests/vmx2xmldata/esx-in-the-wild-13.xml b/tests/vmx2xmldata/esx-in-the-wild-13.xml
>> new file mode 100644
>> index 000000000000..689282747779
>> --- /dev/null
>> +++ b/tests/vmx2xmldata/esx-in-the-wild-13.xml
>> @@ -0,0 +1,55 @@
>> +<domain type='vmware'>
>> +  <name>Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)</name>
>> +  <uuid>421eb458-5448-fc12-2074-83d5e419e138</uuid>
>> +  <description>name:Test-Mig-VM-1
>> +userid:962314ba515c48388a0e95c0961709ff
>> +username:admin
>> +projectid:b06b5f77b6bb442f85b1c67cff980ef9
>> +projectname:MIS
>> +flavor:name:mig-test-flavor
>> +flavor:memory_mb:1024
>> +flavor:vcpus:1
>> +flavor:ephemeral_gb:0
>> +flavor:root_gb:10
>> +flavor:swap:0
>> +imageid:8b90d6fa-20ab-4adf-8015-aad3dddb246c
>> +package:20.6.2
>> +</description>
>> +  <memory unit='KiB'>1048576</memory>
>> +  <currentMemory unit='KiB'>1048576</currentMemory>
>> +  <vcpu placement='static'>1</vcpu>
>> +  <cputune>
>> +    <shares>1000</shares>
>> +  </cputune>
>> +  <os>
>> +    <type arch='x86_64'>hvm</type>
>> +  </os>
>> +  <clock offset='utc'/>
>> +  <on_poweroff>destroy</on_poweroff>
>> +  <on_reboot>restart</on_reboot>
>> +  <on_crash>destroy</on_crash>
>> +  <devices>
>> +    <disk type='file' device='disk'>
>> +      <source file='[datastore] directory/Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)_2.vmdk'/>
>> +      <target dev='sda' bus='scsi'/>
>> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
>> +    </disk>
>> +    <controller type='scsi' index='0' model='lsilogic'/>
>> +    <interface type='bridge'>
>> +      <mac address='fa:16:3e:bb:2c:4a' type='static'/>
>> +      <source bridge='Test'/>
>> +      <model type='vmxnet3'/>
>> +    </interface>
>> +    <serial type='file'>
>> +      <source path='/dev/null'/>
>> +      <target port='0'/>
>> +    </serial>
>> +    <console type='file'>
>> +      <source path='/dev/null'/>
>> +      <target type='serial' port='0'/>
>> +    </console>
>> +    <video>
>> +      <model type='vmvga' vram='4096' primary='yes'/>
>> +    </video>
>> +  </devices>
>> +</domain>
>> diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
>> index 785eee6d3002..0fb5f13f72ce 100644
>> --- a/tests/vmx2xmltest.c
>> +++ b/tests/vmx2xmltest.c
>> @@ -263,6 +263,7 @@ mymain(void)
>>      DO_TEST("esx-in-the-wild-10");
>>      DO_TEST("esx-in-the-wild-11");
>>      DO_TEST("esx-in-the-wild-12");
>> +    DO_TEST("esx-in-the-wild-13");
>>
>>      DO_TEST("gsx-in-the-wild-1");
>>      DO_TEST("gsx-in-the-wild-2");
>> --
>
>From the point of view of virt-v2v:
>
>ACK
>
>I don't care if the path is /dev/null or nothing, as virt-v2v will
>ignore the serial port.  Most important thing is not to fail.
>

So type="null" it is, thanks for the info.

>Rich.
>
>-- 
>Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
>Read my programming and virtualization blog: http://rwmj.wordpress.com
>libguestfs lets you edit virtual machines.  Supports shell scripting,
>bindings from many languages.  http://libguestfs.org
>
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] vmx: Check serialX.vspc before serialX.fileName
Posted by Peter Krempa 1 week, 3 days ago 
On Thu, Apr 25, 2024 at 14:12:54 +0200, Martin Kletzander wrote:
> When using vSPC (Virtual Serial Port Concentrator) in vSphere the actual
> address for it is saved in serialX.vspc in which case the
> serialX.fileName is most probably something we can't get any useful
> information from and we also fail during the parsing rendering any
> dumpxml and similar tries unsuccessful.
> 
> Instead of parsing the vspc URL with something along the lines of
> `virURIParse(vspc ? vspc : fileName)`, which could lead to us reporting
> information that is very prune to misuse (the vSPC seemingly has a
> protocol on top of the telnet connection; redefining the domain would
> change the behaviour; the URL might have a fragment we are not saving;
> etc.) or adding more XML knobs to indicate vSPC usage (which we would
> not be able to configure; we'd have to properly error out everywhere;
> etc.) let's just report dummy serial port that leads to nowhere.
> 
> Resolves: https://issues.redhat.com/browse/RHEL-32182
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  src/vmx/vmx.c                            | 12 +++
>  tests/vmx2xmldata/esx-in-the-wild-13.vmx | 97 ++++++++++++++++++++++++
>  tests/vmx2xmldata/esx-in-the-wild-13.xml | 55 ++++++++++++++
>  tests/vmx2xmltest.c                      |  1 +
>  4 files changed, 165 insertions(+)
>  create mode 100644 tests/vmx2xmldata/esx-in-the-wild-13.vmx
>  create mode 100644 tests/vmx2xmldata/esx-in-the-wild-13.xml
> 
> diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
> index 5da67aae60d9..32074f62e14c 100644
> --- a/src/vmx/vmx.c
> +++ b/src/vmx/vmx.c
> @@ -2975,6 +2975,9 @@ 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;
>  
> @@ -2997,6 +3000,7 @@ 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 */
> @@ -3026,6 +3030,10 @@ 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) {
> @@ -3057,6 +3065,10 @@ 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) {
> +        (*def)->target.port = port;
> +        (*def)->source->type = VIR_DOMAIN_CHR_TYPE_FILE;

Wouldn't a _TYPE_NULL be more reasonable in this case? Or is the
intention to actually have a path?
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] vmx: Check serialX.vspc before serialX.fileName
Posted by Martin Kletzander 1 week, 2 days ago 
On Thu, Apr 25, 2024 at 05:30:24PM +0200, Peter Krempa wrote:
>On Thu, Apr 25, 2024 at 14:12:54 +0200, Martin Kletzander wrote:
>> When using vSPC (Virtual Serial Port Concentrator) in vSphere the actual
>> address for it is saved in serialX.vspc in which case the
>> serialX.fileName is most probably something we can't get any useful
>> information from and we also fail during the parsing rendering any
>> dumpxml and similar tries unsuccessful.
>>
>> Instead of parsing the vspc URL with something along the lines of
>> `virURIParse(vspc ? vspc : fileName)`, which could lead to us reporting
>> information that is very prune to misuse (the vSPC seemingly has a
>> protocol on top of the telnet connection; redefining the domain would
>> change the behaviour; the URL might have a fragment we are not saving;
>> etc.) or adding more XML knobs to indicate vSPC usage (which we would
>> not be able to configure; we'd have to properly error out everywhere;
>> etc.) let's just report dummy serial port that leads to nowhere.
>>
>> Resolves: https://issues.redhat.com/browse/RHEL-32182
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---
>>  src/vmx/vmx.c                            | 12 +++
>>  tests/vmx2xmldata/esx-in-the-wild-13.vmx | 97 ++++++++++++++++++++++++
>>  tests/vmx2xmldata/esx-in-the-wild-13.xml | 55 ++++++++++++++
>>  tests/vmx2xmltest.c                      |  1 +
>>  4 files changed, 165 insertions(+)
>>  create mode 100644 tests/vmx2xmldata/esx-in-the-wild-13.vmx
>>  create mode 100644 tests/vmx2xmldata/esx-in-the-wild-13.xml
>>
>> diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
>> index 5da67aae60d9..32074f62e14c 100644
>> --- a/src/vmx/vmx.c
>> +++ b/src/vmx/vmx.c
>> @@ -2975,6 +2975,9 @@ 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;
>>
>> @@ -2997,6 +3000,7 @@ 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 */
>> @@ -3026,6 +3030,10 @@ 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) {
>> @@ -3057,6 +3065,10 @@ 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) {
>> +        (*def)->target.port = port;
>> +        (*def)->source->type = VIR_DOMAIN_CHR_TYPE_FILE;
>
>Wouldn't a _TYPE_NULL be more reasonable in this case? Or is the
>intention to actually have a path?
>

It is not the intention, I simply haven't thought of a NULL type.  I'll
try to figure out what's the best way of keeping the serial in a way
that it does work seamlessly with the main (only?) consumer - virt-v2v.
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] vmx: Check serialX.vspc before serialX.fileName
Posted by Martin Kletzander 1 week, 2 days ago 
On Fri, Apr 26, 2024 at 10:34:46AM +0200, Martin Kletzander wrote:
>On Thu, Apr 25, 2024 at 05:30:24PM +0200, Peter Krempa wrote:
>>On Thu, Apr 25, 2024 at 14:12:54 +0200, Martin Kletzander wrote:
>>> When using vSPC (Virtual Serial Port Concentrator) in vSphere the actual
>>> address for it is saved in serialX.vspc in which case the
>>> serialX.fileName is most probably something we can't get any useful
>>> information from and we also fail during the parsing rendering any
>>> dumpxml and similar tries unsuccessful.
>>>
>>> Instead of parsing the vspc URL with something along the lines of
>>> `virURIParse(vspc ? vspc : fileName)`, which could lead to us reporting
>>> information that is very prune to misuse (the vSPC seemingly has a
>>> protocol on top of the telnet connection; redefining the domain would
>>> change the behaviour; the URL might have a fragment we are not saving;
>>> etc.) or adding more XML knobs to indicate vSPC usage (which we would
>>> not be able to configure; we'd have to properly error out everywhere;
>>> etc.) let's just report dummy serial port that leads to nowhere.
>>>
>>> Resolves: https://issues.redhat.com/browse/RHEL-32182
>>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>>> ---
>>>  src/vmx/vmx.c                            | 12 +++
>>>  tests/vmx2xmldata/esx-in-the-wild-13.vmx | 97 ++++++++++++++++++++++++
>>>  tests/vmx2xmldata/esx-in-the-wild-13.xml | 55 ++++++++++++++
>>>  tests/vmx2xmltest.c                      |  1 +
>>>  4 files changed, 165 insertions(+)
>>>  create mode 100644 tests/vmx2xmldata/esx-in-the-wild-13.vmx
>>>  create mode 100644 tests/vmx2xmldata/esx-in-the-wild-13.xml
>>>
>>> diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
>>> index 5da67aae60d9..32074f62e14c 100644
>>> --- a/src/vmx/vmx.c
>>> +++ b/src/vmx/vmx.c
>>> @@ -2975,6 +2975,9 @@ 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;
>>>
>>> @@ -2997,6 +3000,7 @@ 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 */
>>> @@ -3026,6 +3030,10 @@ 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) {
>>> @@ -3057,6 +3065,10 @@ 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) {
>>> +        (*def)->target.port = port;
>>> +        (*def)->source->type = VIR_DOMAIN_CHR_TYPE_FILE;
>>
>>Wouldn't a _TYPE_NULL be more reasonable in this case? Or is the
>>intention to actually have a path?
>>
>
>It is not the intention, I simply haven't thought of a NULL type.  I'll
>try to figure out what's the best way of keeping the serial in a way
>that it does work seamlessly with the main (only?) consumer - virt-v2v.

So based on Rich's response type="null" is fine too, so I changed it.
I'll wait after the release to push it since this is not strictly a
bugfix.
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] vmx: Check serialX.vspc before serialX.fileName
Posted by Michal Prívozník 1 week, 3 days ago 
On 4/25/24 14:12, Martin Kletzander wrote:
> When using vSPC (Virtual Serial Port Concentrator) in vSphere the actual
> address for it is saved in serialX.vspc in which case the
> serialX.fileName is most probably something we can't get any useful
> information from and we also fail during the parsing rendering any
> dumpxml and similar tries unsuccessful.
> 
> Instead of parsing the vspc URL with something along the lines of
> `virURIParse(vspc ? vspc : fileName)`, which could lead to us reporting
> information that is very prune to misuse (the vSPC seemingly has a
> protocol on top of the telnet connection; redefining the domain would
> change the behaviour; the URL might have a fragment we are not saving;
> etc.) or adding more XML knobs to indicate vSPC usage (which we would
> not be able to configure; we'd have to properly error out everywhere;
> etc.) let's just report dummy serial port that leads to nowhere.
> 
> Resolves: https://issues.redhat.com/browse/RHEL-32182
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  src/vmx/vmx.c                            | 12 +++
>  tests/vmx2xmldata/esx-in-the-wild-13.vmx | 97 ++++++++++++++++++++++++
>  tests/vmx2xmldata/esx-in-the-wild-13.xml | 55 ++++++++++++++
>  tests/vmx2xmltest.c                      |  1 +
>  4 files changed, 165 insertions(+)
>  create mode 100644 tests/vmx2xmldata/esx-in-the-wild-13.vmx
>  create mode 100644 tests/vmx2xmldata/esx-in-the-wild-13.xml

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

Michal
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.