[PATCH v3] qemu: add virQEMUBuildBufferEscapeComma in qemu_command.c

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

 



This patch adds virQEMUBuildBufferEscapeComma to properly
escape commas in user provided data fields for qemu command
line processing.

Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@xxxxxxxxx>
---

Thank you for the helpful feedback and apologies for the delay.

Changes in v3:
virQEMUBuildBufferEscapeComma was applied to:
- src->hosts->socket in qemuBuildNetworkDriveURI
- src->path, src->configFile in qemuBuildNetworkDriveStr
- disk->blkdeviotune.group_name in qemuBuildDiskThrottling
- net->data.socket.address, net->data.socket.localaddr in
  qemuBuildHostNetStr
- dev->data.file.path in qemuBuildChrChardevStr
- graphics->data.spice.rendernode in
  qemuBuildGraphicsSPICECommandLine
- smartcard->data.cert.file[i], smartcard->data.cert.database in
  qemuBuildSmartcardCommandLine

Changes in v2:
virQEMUBuildBufferEscapeComma was applied to:
- info->romfile in qemuBuildRomStr
- disk->vendor, disk->product in qemuBuildDriveDevStr
- fs->src->path in qemuBuildFSStr
- fs->dst in qemuBuildFSDevStr
- connect= in qemuBuildHostNetStr
- fileval handling in qemuBuildChrChardevStr
- TYPE_DEV, TYPE_PIPE handling in qemuBuildChrChardevStr
- cfg->vncTLSx509certdir in qemuBuildGraphicsVNCCommandLine
- cfg->spiceTLSx509certdir in qemuBuildGraphicsSPICECommandLine
- loader->path, loader->nvram usage in
  qemuBuildDomainLoaderCommandLine

Link to v2: https://www.redhat.com/archives/libvir-list/2018-March/msg00965.html


When I tried to change src->path in qemuBuildNetworkDriveStr
for this portion

961             } else if (src->nhosts == 1) {
962                 if (virAsprintf(&ret, "sheepdog:%s:%u:%s",
963                                 src->hosts->name, src->hosts->port,
964                                 src->path) < 0)
965                     goto cleanup;
966             } else {

make check reported the following error.

141) QEMU XML-2-ARGV disk-drive-network-sheepdog                       ...
In '/home/skrtbhtngr/libvirt/tests/qemuxml2argvdata/disk-drive-network-sheepdog.args':
Offset 0
Expect [LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none /usr/bin/qemu-system-i686 -name QEMUGuest1 -S -M pc -m 214 -smp 1,sockets=1,cores=1,threads=1 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 -nographic -nodefaults -chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -usb -drive file=/dev/HostVG/QEMU,,Guest,,,,1,format=raw,if=none,id=drive-ide0-0-0 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -drive file=sheepdog:example.org:6000:image,,with,,commas,format=raw,if=none,id=drive-virtio-disk0 -device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,id=virtio-disk0
]
Actual [LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none /usr/bin/qemu-system-i686 -name QEMUGuest1 -S -M pc -m 214 -smp 1,sockets=1,cores=1,threads=1 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 -nographic -nodefaults -chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -usb -drive file=/dev/HostVG/QEMU,,Guest,,,,1,format=raw,if=none,id=drive-ide0-0-0 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -drive file=sheepdog:example.org:6000:image,,,,with,,,,commas,format=raw,if=none,id=drive-virtio-disk0 -device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,id=virtio-disk0
]
                                                                      ... FAILED


In disk-drive-network-sheepdog.args:
    ...
    -drive file=sheepdog:example.org:6000:image,,with,,commas,format=raw,if=none,\
    ...

I was not quite sure how to handle this part. Adding 
virQEMUBuildBufferEscapeComma there is escaping the twin commas 
in the file name again. I have left that part unchanged.


src/qemu/qemu_command.c | 111 +++++++++++++++++++++++++-----------------------
 1 file changed, 59 insertions(+), 52 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6f76f18ab..26b36551c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -844,14 +844,18 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
                          qemuDomainSecretInfoPtr secinfo)
 {
     virURIPtr uri = NULL;
-    char *ret = NULL;
+    char *ret = NULL, *socket = NULL;
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
 
     if (!(uri = qemuBlockStorageSourceGetURI(src)))
         goto cleanup;
 
-    if (src->hosts->socket &&
-        virAsprintf(&uri->query, "socket=%s", src->hosts->socket) < 0)
-        goto cleanup;
+    if (src->hosts->socket) {
+        virQEMUBuildBufferEscapeComma(&buf, src->hosts->socket);
+        socket = virBufferContentAndReset(&buf);
+        if (virAsprintf(&uri->query, "socket=%s", socket) < 0)
+            goto cleanup;
+    }
 
     if (qemuBuildGeneralSecinfoURI(uri, secinfo) < 0)
         goto cleanup;
@@ -860,6 +864,8 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
 
  cleanup:
     virURIFree(uri);
+    virBufferFreeAndReset(&buf);
+
     return ret;
 }
 
@@ -868,8 +874,9 @@ static char *
 qemuBuildNetworkDriveStr(virStorageSourcePtr src,
                          qemuDomainSecretInfoPtr secinfo)
 {
-    char *ret = NULL;
+    char *ret = NULL, *path = NULL, *file = NULL;
     virBuffer buf = VIR_BUFFER_INITIALIZER;
+    virBuffer bufTemp = VIR_BUFFER_INITIALIZER;
     size_t i;
 
     switch ((virStorageNetProtocol) src->protocol) {
@@ -914,8 +921,10 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
                     goto cleanup;
                 }
 
-                if (src->path)
-                    virBufferAsprintf(&buf, ":exportname=%s", src->path);
+                if (src->path) {
+                    virBufferAddLit(&buf, ":exportname=");
+                    virQEMUBuildBufferEscapeComma(&buf, src->path);
+                }
 
                 if (virBufferCheckError(&buf) < 0)
                     goto cleanup;
@@ -945,7 +954,9 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
             }
 
             if (src->nhosts == 0) {
-                if (virAsprintf(&ret, "sheepdog:%s", src->path) < 0)
+                virQEMUBuildBufferEscapeComma(&bufTemp, src->path);
+                path = virBufferContentAndReset(&bufTemp);
+                if (virAsprintf(&ret, "sheepdog:%s", path) < 0)
                     goto cleanup;
             } else if (src->nhosts == 1) {
                 if (virAsprintf(&ret, "sheepdog:%s:%u:%s",
@@ -967,8 +978,9 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
                                src->path);
                 goto cleanup;
             }
-
-            virBufferStrcat(&buf, "rbd:", src->volume, "/", src->path, NULL);
+            virQEMUBuildBufferEscapeComma(&bufTemp, src->path);
+            path = virBufferContentAndReset(&bufTemp);
+            virBufferStrcat(&buf, "rbd:", src->volume, "/", path, NULL);
 
             if (src->snapshot)
                 virBufferEscape(&buf, '\\', ":", "@%s", src->snapshot);
@@ -994,8 +1006,11 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
                 }
             }
 
-            if (src->configFile)
-                virBufferEscape(&buf, '\\', ":", ":conf=%s", src->configFile);
+            if (src->configFile) {
+                virQEMUBuildBufferEscapeComma(&bufTemp, src->configFile);
+                file = virBufferContentAndReset(&bufTemp);
+                virBufferEscape(&buf, '\\', ":", ":conf=%s", file);
+            }
 
             if (virBufferCheckError(&buf) < 0)
                 goto cleanup;
@@ -1022,6 +1037,7 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
     }
 
  cleanup:
+    virBufferFreeAndReset(&bufTemp);
     virBufferFreeAndReset(&buf);
 
     return ret;
@@ -1630,6 +1646,8 @@ qemuBuildDiskThrottling(virDomainDiskDefPtr disk,
         virBufferAsprintf(buf, ",throttling." _label "=%llu", \
                           disk->blkdeviotune._field); \
     }
+    virBuffer bufTemp = VIR_BUFFER_INITIALIZER;
+    char *name = NULL;
 
     IOTUNE_ADD(total_bytes_sec, "bps-total");
     IOTUNE_ADD(read_bytes_sec, "bps-read");
@@ -1647,8 +1665,9 @@ qemuBuildDiskThrottling(virDomainDiskDefPtr disk,
 
     IOTUNE_ADD(size_iops_sec, "iops-size");
     if (disk->blkdeviotune.group_name) {
-        virBufferEscapeString(buf, ",throttling.group=%s",
-                              disk->blkdeviotune.group_name);
+        virQEMUBuildBufferEscapeComma(&bufTemp, disk->blkdeviotune.group_name);
+        name = virBufferContentAndReset(&bufTemp);
+        virBufferEscapeString(buf, ",throttling.group=%s", name);
     }
 
     IOTUNE_ADD(total_bytes_sec_max_length, "bps-total-max-length");
@@ -1657,6 +1676,8 @@ qemuBuildDiskThrottling(virDomainDiskDefPtr disk,
     IOTUNE_ADD(total_iops_sec_max_length, "iops-total-max-length");
     IOTUNE_ADD(read_iops_sec_max_length, "iops-read-max-length");
     IOTUNE_ADD(write_iops_sec_max_length, "iops-write-max-length");
+
+    virBufferFreeAndReset(&bufTemp);
 #undef IOTUNE_ADD
 }
 
@@ -3651,27 +3672,25 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
         break;
 
     case VIR_DOMAIN_NET_TYPE_SERVER:
-        virBufferAsprintf(&buf, "socket%clisten=%s:%d,",
-                          type_sep,
+        virBufferAsprintf(&buf, "socket%clisten=", type_sep);
+        virQEMUBuildBufferEscapeComma(&buf,
                           net->data.socket.address ? net->data.socket.address
-                          : "",
-                          net->data.socket.port);
+                          : "");
+        virBufferAsprintf(&buf, ":%d,", net->data.socket.port);
         break;
 
     case VIR_DOMAIN_NET_TYPE_MCAST:
-        virBufferAsprintf(&buf, "socket%cmcast=%s:%d,",
-                          type_sep,
-                          net->data.socket.address,
-                          net->data.socket.port);
+        virBufferAsprintf(&buf, "socket%cmcast=", type_sep);
+        virQEMUBuildBufferEscapeComma(&buf, net->data.socket.address);
+        virBufferAsprintf(&buf, ":%d,", net->data.socket.port);
         break;
 
     case VIR_DOMAIN_NET_TYPE_UDP:
-        virBufferAsprintf(&buf, "socket%cudp=%s:%d,localaddr=%s:%d,",
-                          type_sep,
-                          net->data.socket.address,
-                          net->data.socket.port,
-                          net->data.socket.localaddr,
-                          net->data.socket.localport);
+        virBufferAsprintf(&buf, "socket%cudp=", type_sep);
+        virQEMUBuildBufferEscapeComma(&buf, net->data.socket.address);
+        virBufferAsprintf(&buf, ":%d,localaddr=", net->data.socket.port);
+        virQEMUBuildBufferEscapeComma(&buf, net->data.socket.localaddr);
+        virBufferAsprintf(&buf, ":%d,", net->data.socket.localport);
         break;
 
     case VIR_DOMAIN_NET_TYPE_USER:
@@ -4954,9 +4973,10 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
                        bool chardevStdioLogd)
 {
     virBuffer buf = VIR_BUFFER_INITIALIZER;
+    virBuffer bufTemp = VIR_BUFFER_INITIALIZER;
     bool telnet;
     char *charAlias = NULL;
-    char *ret = NULL;
+    char *ret = NULL, *path = NULL;
 
     if (!(charAlias = qemuAliasChardevFromDevAlias(alias)))
         goto cleanup;
@@ -4990,9 +5010,11 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
                            _("append not supported in this QEMU binary"));
             goto cleanup;
         }
+        virQEMUBuildBufferEscapeComma(&bufTemp, dev->data.file.path);
+        path = virBufferContentAndReset(&bufTemp);
         if (qemuBuildChrChardevFileStr(chardevStdioLogd ? logManager : NULL,
                                        cmd, def, &buf,
-                                       "path", dev->data.file.path,
+                                       "path", path,
                                        "append", dev->data.file.append) < 0)
             goto cleanup;
         break;
@@ -8150,8 +8172,8 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
                                _("This QEMU doesn't support spice OpenGL rendernode"));
                 goto error;
             }
-
-            virBufferAsprintf(&opt, "rendernode=%s,", graphics->data.spice.rendernode);
+            virBufferAddLit(&opt, "rendernode=");
+            virQEMUBuildBufferEscapeComma(&opt, graphics->data.spice.rendernode);
         }
     }
 
@@ -8771,7 +8793,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
     virDomainSmartcardDefPtr smartcard;
     char *devstr;
     virBuffer opt = VIR_BUFFER_INITIALIZER;
-    const char *database;
     const char *contAlias = NULL;
 
     if (!def->nsmartcards)
@@ -8814,29 +8835,15 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
 
         virBufferAddLit(&opt, "ccid-card-emulated,backend=certificates");
         for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) {
-            if (strchr(smartcard->data.cert.file[i], ',')) {
-                virBufferFreeAndReset(&opt);
-                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                               _("invalid certificate name: %s"),
-                               smartcard->data.cert.file[i]);
-                return -1;
-            }
-            virBufferAsprintf(&opt, ",cert%zu=%s", i + 1,
-                              smartcard->data.cert.file[i]);
+            virBufferAsprintf(&opt, ",cert%zu=", i + 1);
+            virQEMUBuildBufferEscapeComma(&opt, smartcard->data.cert.file[i]);
         }
         if (smartcard->data.cert.database) {
-            if (strchr(smartcard->data.cert.database, ',')) {
-                virBufferFreeAndReset(&opt);
-                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                               _("invalid database name: %s"),
-                               smartcard->data.cert.database);
-                return -1;
-            }
-            database = smartcard->data.cert.database;
+            virBufferAddLit(&opt, ",db=");
+            virQEMUBuildBufferEscapeComma(&opt, smartcard->data.cert.database);
         } else {
-            database = VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE;
+            virBufferAsprintf(&opt, ",db=%s", VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE);
         }
-        virBufferAsprintf(&opt, ",db=%s", database);
         break;
 
     case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
-- 
2.16.2

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux